-
Notifications
You must be signed in to change notification settings - Fork 82
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
add image refs, env vars, so we can get jenkins images from payload #85
add image refs, env vars, so we can get jenkins images from payload #85
Conversation
@gabemontero it looks correct to me. |
/retest |
ah ... I need to wait until the mirroring for jenkins-agent-base completes:
|
f8dbc2a
to
8d4ffb4
Compare
updated for openshift/release#2678 @bparees |
8d4ffb4
to
78430a6
Compare
78430a6
to
1630ed5
Compare
image refs/envs look ok to me again |
@gabemontero i'd be ok merging this now so we can start seeing if the env vars show up properly on the samples operator. May make it easier for you to iterate on the next steps later when the cluster is populating the payload correctly out of the box. |
but i think we are supposed to wait on this until the OCP side of the payload is ready which it is not yet. /hold |
discussion of the next step to get it into the OCP payload: |
Just logged into api.ci:
Not sure how to decipher the going to retest and see |
/retest |
the test won't tell you anything because it doesn't build an OCP payload. the |
I do not see the jenkins images in the ocp imagestream, so you'll have to bug @joeldavis84 or @tbielawa to see what we've missed in terms of getting the new jenkins images built+published for ocp. |
/retest |
The actual image job failure is:
notice the All the references in this PR are of the In fact, there are no references to Perhaps something was set up in dist git / osbs land when the name was @joeldavis84 @tbielawa does that provide any clues? Or maybe that is a red herring .... |
@bparees yeah, I see those ist's now in the ocp namespace (appear to have been there since Jan 24:
I would think the naming issue I noted above is the smoking guy currently. |
it's not just that they need to be in the ocp namespace, they need to be part of the "4.0" imagestream.
if you look in that imagestream (named "4.0") you will see tags for all the other OCP payload images. It is supposed to be the equivalent to the "origin-v4.0" imagestream in the "openshift" namespace:
|
1630ed5
to
8b4998a
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gabemontero The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 similar comment
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gabemontero The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
In the failed image eco run, the |
from @bparees on slack: things look good re: ocp ... needed to look at |
/retest |
8b4998a
to
f6a4b83
Compare
@bparees this still needs to hold until the other PRs merge, but this is reviewable in that a) the changes do not collide with #81 ptal |
f6a4b83
to
fabbd3b
Compare
case imagestream.Name == "jenkins-agent-maven": | ||
imagestream = tagInPayload("v4.0", "IMAGE_AGENT_MAVEN", imagestream) | ||
case imagestream.Name == "jenkins-agent-nodejs": | ||
imagestream = tagInPayload("v4.0", "IMAGE_AGENT_NODEJS", imagestream) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i debated whether to put the "v4.0" tag in the imagestream or just use latest...prior experience told me we'll probably want the flexibility of introducing a new version at some point, but it's something to think about as we move forward, we can revisit this choice up until GA.
@gabemontero this looks fine... i'm not actually sure there's any reason to hold it? All this logic should be ok on its own right? obviously the agent related bits won't do anything, but this will get us using the master image from the payload, right? (maybe this PR should also remove the jenkins specific centos logic you added?) |
/hold cancel |
you convinced me @bparees .... placed on the merge queue |
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest |
/retest Please review the full test history for this PR and help us cut down flakes. |
/assign @bparees
ptal to review at least the yaml/ref stuff
in theory we could just merge this, but ideally I can wire in the processing of the new env's in the samples operator code with this PR as well
I'd like to start on those after #81