Adding support for multitag #71
Conversation
jupierce
force-pushed
the
addmultitag
branch
3 times, most recently
from
1b6e932 to
59849c1
Compare
| } | ||
|
|
||
| IImageStream destIS = null; | ||
| for ( int retries = 10; retries > 0; retries-- ) { |
There was a problem hiding this comment.
curious on the 10 retries here .... unlike some of the pod related operations discussed in the oc exec pull, I've not encountered similar timing issues with tagging images in the past. Did it seem like it stemmed from the multi-tagging, where the code was creating multiple tags in rapid succession?
There was a problem hiding this comment.
Precisely that. Modifying the resource repeatedly caused a 403 unless I pulled the resource again.
There was a problem hiding this comment.
Gotcha - thanks for the confirm.
| return false; | ||
| } | ||
|
|
||
| for ( int retries2 = 10; retries2 > 0; retries2-- ) { |
There was a problem hiding this comment.
ah .. you just added this one while I was reviewing .... same curiosity on what exactly you are seeing in your testing and if the theory I posted in the comment above is on the right track or not
There was a problem hiding this comment.
That one was added more out of caution. I did not actually witness it fail. I'm fine to remove it if you think it unnecessary.
There was a problem hiding this comment.
Yeah a get should be OK vs. the update ... let's remove / have a little less clutter.
| } | ||
|
|
||
| IImageStream destIS = null; | ||
| for ( int retries = 10; retries > 0; retries-- ) { |
There was a problem hiding this comment.
Gotcha - thanks for the confirm.
| return false; | ||
| } | ||
|
|
||
| for ( int retries2 = 10; retries2 > 0; retries2-- ) { |
There was a problem hiding this comment.
Yeah a get should be OK vs. the update ... let's remove / have a little less clutter.
|
Oh, just noticed the default test failure after my review comment. We'll need to look into that as well prior to merging. |
|
Looks like the extended test set up failed @jupierce - see if this is a known flake or not in openshift/origin |
|
@gabemontero I hand tested multi-project/multi-token configs. Looks good so far, but will need to implement extended tests to be sure those work on new 1->N and N->N variants. |
|
@jupierce roger that - sounds good. |
|
[test] |
|
rebasing for conflict |
|
@gabemontero PR test flake has been addressed. |
|
Cool - thanks for the confirm @jupierce On Thu, Oct 6, 2016 at 11:12 AM, Justin Pierce notifications@github.com
|
Adding exec function for card:
https://trello.com/c/Yl4qomsw/878-5-additional-jenkins-plugin-enhancement-requests-from-github
ptal @gabemontero @bparees
Enabling the following forms: