You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@mesos.apache.org by "Armand Grillet (JIRA)" <ji...@apache.org> on 2018/11/22 16:34:00 UTC

[jira] [Commented] (MESOS-9398) post-reviews.py fails to update an existing chain.

    [ https://issues.apache.org/jira/browse/MESOS-9398?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16696101#comment-16696101 ] 

Armand Grillet commented on MESOS-9398:
---------------------------------------

I have faced the same issue and was not able to fix the situation by cherry-picking.

{noformat}
➜  apache-mesos (MESOS-9399) ✔ support/post-reviews.py
Running 'rbt post' across all of ...
14f2ddf1283016b6f4aedbd26bf2ad98b2eb4ed5 - (HEAD -> MESOS-9399) Added '--all' flag to 'mesos task list'. (8 seconds ago)
92f74aa144e20c4fa3568aacc66b656ca82c4379 - Replaced CLI test helper function 'running_tasks' by 'wait_for_task'. (8 seconds ago)
8d814a992faac40de7d7869901f872788fcde7f4 - Fixed name of task created when running mesos-cli-tests. (8 seconds ago)
b9b2080cac081f1f0a4a232f9e88435844107edf - Updated 'mesos task list' to only display running tasks. (20 seconds ago)
Updating diff of:
b9b2080cac081f1f0a4a232f9e88435844107edf - Updated 'mesos task list' to only display running tasks. (20 seconds ago)
Press enter to continue or 'Ctrl-C' to skip.

Failed to execute: 'rbt post --markdown --tracking-branch=master --review-request-id=69394 fcb8e2abeef52042a7f40312c461cab716c0bf3c 1a418847eb152875f1c579e45f6f35733532e3bb':
ERROR: Error validating diff

fatal: git cat-file: could not get object info
 (HTTP 400, API Error 224)

➜  apache-mesos (MESOS-9399) ✔ git checkout master

➜  apache-mesos (master) ✔ git checkout -b MESOS-9399-2

➜  apache-mesos (MESOS-9399-2) ✔ git cherry-pick b9b2080cac081f1f0a4a232f9e88435844107edf
[MESOS-9399-2 015168383] Updated 'mesos task list' to only display running tasks.
 Date: Mon Nov 19 14:05:17 2018 +0100
 2 files changed, 10 insertions(+), 7 deletions(-)

➜  apache-mesos (MESOS-9399-2) ✔ support/post-reviews.py
Running 'rbt post' across all of ...
0151683836474223c0d76d779c9e451ba6fdbd88 - (HEAD -> MESOS-9399-2) Updated 'mesos task list' to only display running tasks. (6 seconds ago)
Updating diff of:
0151683836474223c0d76d779c9e451ba6fdbd88 - (HEAD -> MESOS-9399-2) Updated 'mesos task list' to only display running tasks. (6 seconds ago)
Press enter to continue or 'Ctrl-C' to skip.

Failed to execute: 'rbt post --markdown --tracking-branch=master --review-request-id=69394 fcb8e2abeef52042a7f40312c461cab716c0bf3c 954f9b3cf58aed47f1ea58e57e33491ea9d46a2b':
ERROR: Error validating diff

fatal: git cat-file: could not get object info
 (HTTP 400, API Error 224)

➜  apache-mesos (master) ✔ rbt post --markdown --tracking-branch=master --review-request-id=69394 fcb8e2abeef52042a7f40312c461cab716c0bf3c 0151683836474223c0d76d779c9e451ba6fdbd88
ERROR: Error validating diff

fatal: git cat-file: could not get object info
 (HTTP 400, API Error 224)
{noformat}

We can sometimes see the same issue with ReviewBot, like in https://reviews.apache.org/r/69426/

> 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 <to...@me.com>
> Date:   Sun Nov 18 16:56:21 2018 +0100
>     Added test for ACCESS_MESOS_LOG authorization.
> commit 7b02bac16d74a80fc72cd02514fb115a9084cf87
> Author: Till Toenshoff <to...@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 <to...@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 <to...@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 <to...@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 <to...@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)