Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pullthrough serve blob #9796

Merged

Conversation

miminar
Copy link

@miminar miminar commented Jul 12, 2016

Don't attempt to copy any data on a HEAD.

Closes #8613

@miminar
Copy link
Author

miminar commented Jul 12, 2016

Tested manually with tcpdump.

@miminar
Copy link
Author

miminar commented Jul 12, 2016

Looks like flake #9775

Running test/end-to-end/core.sh:29: executing 'oc get -n test pods' expecting any result and text 'frontend.+Running'; re-trying every 0.2s until completion or 120.000s...
FAILURE after 119.691s: test/end-to-end/core.sh:29: executing 'oc get -n test pods' expecting any result and text 'frontend.+Running'; re-trying every 0.2s until completion or 120.000s: the command timed out

[test]

@miminar
Copy link
Author

miminar commented Jul 13, 2016

@legionus PTAL

@legionus
Copy link
Contributor

@miminar Can you add integration test for that ?

@miminar
Copy link
Author

miminar commented Jul 13, 2016

@legionus would a unit test do?

@legionus
Copy link
Contributor

I think you can use any test that checks for this.

@mfojtik
Copy link
Contributor

mfojtik commented Jul 29, 2016

@miminar do we need to backport this?

@miminar
Copy link
Author

miminar commented Jul 29, 2016

@mfojtik it would be nice. Once merged, I'll do the backport. I haven't yet got the chance to finish the tests though.

@miminar
Copy link
Author

miminar commented Aug 5, 2016

@legionus, @soltysh: unit tests added.

@miminar
Copy link
Author

miminar commented Aug 5, 2016

@smarterclayton fyi

@legionus
Copy link
Contributor

legionus commented Aug 5, 2016

LGTM

@mfojtik We need to backport it.

@soltysh
Copy link
Member

soltysh commented Aug 5, 2016

LGTM

@liggitt
Copy link
Contributor

liggitt commented Aug 5, 2016

let's get the blob ACL PR in first

@miminar
Copy link
Author

miminar commented Aug 5, 2016

Yeah, it will conflict for sure.

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 8, 2016
@miminar miminar force-pushed the pullthrough-no-copy-on-head-8613 branch from 1eece67 to c40ffaa Compare August 8, 2016 06:36
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 8, 2016
@miminar miminar force-pushed the pullthrough-no-copy-on-head-8613 branch from c40ffaa to b70c24c Compare August 8, 2016 09:20
@miminar
Copy link
Author

miminar commented Aug 8, 2016

@miminar miminar force-pushed the pullthrough-no-copy-on-head-8613 branch from b70c24c to 8113c24 Compare August 8, 2016 12:39
@miminar
Copy link
Author

miminar commented Aug 8, 2016

I just don't know, how to defeat "The Origin test job could not be run again for this pull request." message without altering commits. In the end, I gave up and re-signed the last commit to move forward.

@stevekuznetsov
Copy link
Contributor

@miminar as the bot requests, you need to have the failed Jenkins job link. The sentence telling you that this link is necessary also has a hyperlink to the failed job it is searching for. This job is the job that was triggered by the bot, so it will be test_pr_origin, not one of the sub-jobs like test_pull_requests_origin_networking. This is a required feature as you could have both failed merge and test jobs in the PR at the same time. The bot needs to know for which failed job you are declaring flakes to have been found.

@miminar
Copy link
Author

miminar commented Aug 8, 2016

@stevekuznetsov thanks! That's 1:0 for the bot. I'm sure I'll win next time.

@miminar
Copy link
Author

miminar commented Aug 8, 2016

Flake #10274. Job https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/7632/
And flake #9490. re-[test]

@stevekuznetsov
Copy link
Contributor

@miminar I'm actually impressed, with your newest comment you found two other bugs in the bot ... I didn't even realize the first way you linked to an issue was a valid thing ... give me a moment.

@miminar
Copy link
Author

miminar commented Aug 8, 2016

I'm always glad to help. 😄

Michal Minář added 2 commits August 14, 2016 12:05
Signed-off-by: Michal Minář <miminar@redhat.com>
Signed-off-by: Michal Minář <miminar@redhat.com>
@miminar miminar force-pushed the pullthrough-no-copy-on-head-8613 branch from 8113c24 to 7d4051e Compare August 14, 2016 10:05
Don't attempt to copy any data on a HEAD.

Signed-off-by: Michal Minar <miminar@redhat.com>
@miminar miminar force-pushed the pullthrough-no-copy-on-head-8613 branch from 7d4051e to 1245726 Compare August 15, 2016 07:04
@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 1245726

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/7902/)

@miminar
Copy link
Author

miminar commented Aug 15, 2016

@mfojtik can you please merge?

@mfojtik
Copy link
Contributor

mfojtik commented Aug 15, 2016

[merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented Aug 15, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/7919/) (Image: devenv-rhel7_4829)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 1245726

@openshift-bot openshift-bot merged commit b6b0bae into openshift:master Aug 15, 2016
@miminar miminar deleted the pullthrough-no-copy-on-head-8613 branch August 17, 2016 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants