[
https://issues.apache.org/jira/browse/MESOS-9398?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16697587#comment-16697587
]
Till Toenshoff commented on MESOS-9398:
---------------------------------------
[~ArmandGrillet] if cherry-picking did not help, then it seems likely that your
local master was not up-to-date or your feature branch was not rebased on an
up-to-date master.
> post-reviews.py fails to update an existing chain.
> --------------------------------------------------
>
> Key: MESOS-9398
> URL: https://issues.apache.org/jira/browse/MESOS-9398
> Project: Mesos
> Issue Type: Bug
> Affects Versions: 1.8.0
> Reporter: Till Toenshoff
> Assignee: Armand Grillet
> Priority: Major
> Labels: reviewboard
>
> My current branch, all commits beyond master's tipp are:
> {noformat}
> commit fbf64ee88c41d7ce1a964197a5fd8900a3157fad (HEAD ->
> ci/till/master-authz-fix-3, origin/ci/till/master-authz-fix-3, backup)
> Author: Till Toenshoff <toensh...@me.com>
> Date: Sun Nov 18 16:56:21 2018 +0100
> Added test for ACCESS_MESOS_LOG authorization.
> commit 7b02bac16d74a80fc72cd02514fb115a9084cf87
> Author: Till Toenshoff <toensh...@me.com>
> Date: Sun Nov 18 18:02:54 2018 +0100
> Refactored createSubject and authorizeLogAccess to common/authorization.
> Moves 'createSubject' out of common/http into common/authorization.
> Removes duplicate 'authorizeLogAccess' out of master.cpp and slave.cpp.
> Introduces 'authorizeLogAccess' within common/authorization.
> commit e4ef89597d8ba99b060ede5d64388041edc6e3d0
> Author: Till Toenshoff <toensh...@me.com>
> Date: Sun Nov 18 18:00:47 2018 +0100
> Introduced common/authorization and refactored collectAuthorizations.
> Adds a new collection of authorization specific helper/s to reduce code
> duplication and increase efficient test coverage.
> Moves the newly introduced 'collectAuthorizations' helper into this new
> authorization source unit.
> commit 4180d433893ec2940c74556b959e9444cef9ad02
> Author: Till Toenshoff <toensh...@me.com>
> Date: Sat Nov 17 23:39:35 2018 +0100
> Added collectAuthorizations helper to master.hpp.
> Adds the helper function 'collectAuthorizations' to master.hpp. This
> function allows for a simple way to collect authorization futures and
> only if all supplied futures result in an approved authorization will
> the returned future return true.
> All identified areas that were formally triggering MESOS-9317 are
> being updated to make use of this new helper.
> A helper function has been chosen and preferred over copying this
> pattern into the areas that needed a fix to allow for an efficient and
> complete test coverage.
> Additionally we are adding a test validating that new helper.
> Review: https://reviews.apache.org/r/69369/
> commit 7c3e73231ec8eb663299dc868f75aea51f2daae7
> Author: Till Toenshoff <toensh...@me.com>
> Date: Sat Nov 17 23:39:23 2018 +0100
> Added test reproducing crash on authorization failure.
> This test reproduces the scenario as described in MESOS-9317. The test
> attempts to create a persistent volume by a web request to the
> authorized V1 operator endpoint. The test assures that the underlying
> authorization request fails as it can in production due to failures in
> the authorization backend.
> Without fixing MESOS-9317, this test crashes the master process as the
> code-path involved will attempt to access the contents of the awaited
> future even though the future had failed.
> Review: https://reviews.apache.org/r/69368/
> {noformat}
> When trying to use post-reviews.py, things appear rather quirky...
> step 1:
> {noformat}
> $ ./support/post-reviews.py
> Running 'rbt post' across all of ...
> fbf64ee88c41d7ce1a964197a5fd8900a3157fad - (HEAD ->
> ci/till/master-authz-fix-3, origin/ci/till/master-authz-fix-3, backup) Added
> test for ACCESS_MESOS_LOG authorization. (31 minutes ago)
> 7b02bac16d74a80fc72cd02514fb115a9084cf87 - Refactored createSubject and
> authorizeLogAccess to common/authorization. (31 minutes ago)
> e4ef89597d8ba99b060ede5d64388041edc6e3d0 - Introduced common/authorization
> and refactored collectAuthorizations. (31 minutes ago)
> 4180d433893ec2940c74556b959e9444cef9ad02 - Added collectAuthorizations helper
> to master.hpp. (31 minutes ago)
> 7c3e73231ec8eb663299dc868f75aea51f2daae7 - Added test reproducing crash on
> authorization failure. (31 minutes ago)
> Updating diff of:
> 7c3e73231ec8eb663299dc868f75aea51f2daae7 - Added test reproducing crash on
> authorization failure. (31 minutes ago)
> Press enter to continue or 'Ctrl-C' to skip.
> {noformat}
> Pressing 'enter', awaiting step 2:
> {noformat}
> Review request #69368 posted.
> https://reviews.apache.org/r/69368/
> https://reviews.apache.org/r/69368/diff/
> Updating diff of:
> 4180d433893ec2940c74556b959e9444cef9ad02 - Added collectAuthorizations helper
> to master.hpp. (32 minutes ago)
> ... with parent diff created from:
> 7c3e73231ec8eb663299dc868f75aea51f2daae7 - Added test reproducing crash on
> authorization failure. (33 minutes ago)
> {noformat}
> Pressing 'enter', awaiting step 3:
> {noformat}
> Failed to execute: 'rbt post --markdown --tracking-branch=master
> --review-request-id=69369 --depends-on=69368
> 7c3e73231ec8eb663299dc868f75aea51f2daae7
> 06ac431bf89479208e042bbdab50cfce0eb3e572':
> ERROR: Error validating diff
> fatal: git cat-file: could not get object info
> (HTTP 400, API Error 224)
> {noformat}
> Note that post-reviews.py creates a temporary branch and cherry-picks onto it
> - so the SHA doesn't match my original one but the commit seems fine and
> matching my expectation.
> {noformat}
> commit 06ac431bf89479208e042bbdab50cfce0eb3e572
> Author: Till Toenshoff <toensh...@me.com>
> Date: Sat Nov 17 23:39:35 2018 +0100
> Added collectAuthorizations helper to master.hpp.
> Adds the helper function 'collectAuthorizations' to master.hpp. This
> function allows for a simple way to collect authorization futures and
> only if all supplied futures result in an approved authorization will
> the returned future return true.
> All identified areas that were formally triggering MESOS-9317 are
> being updated to make use of this new helper.
> A helper function has been chosen and preferred over copying this
> pattern into the areas that needed a fix to allow for an efficient and
> complete test coverage.
> Additionally we are adding a test validating that new helper.
> diff --git a/src/master/master.cpp b/src/master/master.cpp
> [...]
> {noformat}
> Admittedly my workflow to reach this point was complex and involved -
> however, this behaviour seems wrong and any rebasing attempts have not
> corrected this.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
