Till Toenshoff created MESOS-9398:
-------------------------------------
Summary: postreviews.py references outdated commit SHAs.
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
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 how post-reviews.py is referencing a commit that definitely is not part of
my current branch. In fact it is an old version of the very commit I am trying
to push:
```
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
[...]
```
Admittedly my workflow to reach this point was complex and involved - however,
this behaviour is wrong and I wonder how on earth post-reviews.py would get to
the conclusion that it was a good idea to base the commit to post on a version
of that same commit.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
