-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Bug 1421643 - Use existing openshift/origin image instead of new openshift/diagnostics-deployer #13062
Bug 1421643 - Use existing openshift/origin image instead of new openshift/diagnostics-deployer #13062
Conversation
[test] |
@@ -31,6 +31,8 @@ const ( | |||
NetworkDiagNodeLogDirPrefix = "/nodes" | |||
NetworkDiagMasterLogDirPrefix = "/master" | |||
NetworkDiagPodLogDirPrefix = "/pods" | |||
|
|||
NetworkDiagDefaultPodImage = "openshift/origin" |
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.
Any reason this should not use variable.DefaultImagePrefix
?
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.
Does diagnostics accept that flag?
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.
Actually, it does need to use the defaults correctly since enterprise would then fail to pull diagnostics. It can be a follow up issue but would need to be fixed for 1.5
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.
We are not dependent on exact image, any image that provides shell functionality will work (openshift/origin, openshift3/origin, rhel7). We have embedded the actual script in the pod definition as clayton suggested.
Yes, user can override the image. Diagnostics takes a config flag oadm diagnostics NetworkCheck --network-pod-image=openshift3/origin
I see your point. openshift/origin will not be available in OCP registry and if the user didn't use the pod image override flag then this will not work. I'm assuming variable.DefaultImagePrefix
will be openshift/origin for origin release and openshift3/origin for OCP?
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.
Yeah, it should be.
[merge] |
Hold on, before merge -- I have an outstanding question that is important esp. for OCP |
…shift/diagnostics-deployer Any new image like 'openshift/diagnostics-deployer' incurs build/lifecycle costs to maintian and diagnostics-deployer image has only small block of shell code. To alleviate this problem, now the script is embedded into the pod definition and openshift/origin is used as diagnostics deployer image. On dev machines, currently openshift/origin is close to 800MB but we expect the size to be under 200MB when it is released (compressed, debug headers removed).
…twork diagnostic image. DefaultImagePrefix will have proper default value based on origin or OCP environment.
f32c3cd
to
a3cbf68
Compare
Rebased and cherry-picked #13107 [test] |
Evaluated for origin test up to a3cbf68 |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/558/) (Base Commit: 4cc8555) |
@openshift-bot, the last build failed from the following flakes:
re[merge] |
@openshift-bot, the last build failed from the following flakes: re[merge] |
Evaluated for origin merge up to a3cbf68 |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/577/) (Base Commit: 3ac8799) |
Backporting #12982 to release-1.5