-
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 #12982
Bug 1421643 - Use existing openshift/origin image instead of new openshift/diagnostics-deployer #12982
Conversation
@openshift/networking @sdodson PTAL |
weird that this even works, but lgtm I guess |
lgtm |
@smarterclayton can you please review/merge? |
Outside the script looks fine. Lgtm as well [merge] thanks. |
[Test]ing while waiting on the merge queue |
Unrelated errors: re[test] |
pkg/diagnostics/network/objects.go
Outdated
@@ -13,7 +13,7 @@ import ( | |||
) | |||
|
|||
const ( | |||
diagnosticsImage = "openshift/diagnostics-deployer" | |||
diagnosticsImage = "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.
Can this be updated to use the image format string so that we don't have to patch 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.
I don't see existing image format string for 'openshift/origin', there is openshift/origin-${component}:${version} for system components but that doesn't match with what we need.
Supporting this as config involves creating Network diags section with an option in master-config and this needs to be propagated all the way to GetNetworkDiagnosticsPod().
Is it easy to patch OCP instead?
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.
Is it appropriate to use the image format string and strip the hyphen? That's the image name we'd want 'openshift/origin' and 'openshift3/ose'
…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).
41ceb6d
to
37dccba
Compare
Added a new commit that makes network diagnostics pod image configurable (default: openshift/origin) |
Looks like this fails on verifying completions. Maybe other things... Please leave a note when you fix the verifications. |
Updated auto generated docs/bash completions |
[test] |
Evaluated for origin test up to cdc8ab9 |
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/364/) (Base Commit: 3f36d2e) |
[merge] |
1 similar comment
[merge] |
[merge] dirty git repo flake
On Feb 21, 2017, at 7:45 PM, OpenShift Bot <[email protected]> wrote:
continuous-integration/openshift-jenkins/merge FAILURE (
https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/409/)
(Base Commit: 90aaad3
<90aaad3>
)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#12982 (comment)>,
or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p5Vn4ZChgavvGIuHDkr8B0hADTggks5re4UqgaJpZM4MCg8t>
.
|
[merge] is this some sick joke at this point? |
The merge queue hates this PR.
…On Wed, Feb 22, 2017 at 11:49 AM, OpenShift Bot ***@***.***> wrote:
continuous-integration/openshift-jenkins/merge Running (
https://ci.openshift.redhat.com/jenkins/job/test_pull_
requests_origin_future/442/) (Base Commit: bc8f11b
<bc8f11b>
)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#12982 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p6EeM-GwKgqLaUxDHnNjB6WenNFbks5rfGcfgaJpZM4MCg8t>
.
|
[merge] if this fails, i'm just going to hit the big green button on all the PRs that are killing me. |
As steves email noted we may have a problem with Jenkins. If necessary
we'll drain the queue manually.
On Feb 22, 2017, at 1:53 PM, Eric Paris <[email protected]> wrote:
[merge] if this fails, i'm just going to hit the big green button on all
the PRs that are killing me.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#12982 (comment)>,
or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p9_fMYzaPXiUX87kOqqUvoybp_bLks5rfIQTgaJpZM4MCg8t>
.
|
Evaluated for origin merge up to cdc8ab9 |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/464/) (Base Commit: 2ba2ff0) (Image: devenv-rhel7_5959) |
Flake #13068
Merging by hand.
…On Wed, Feb 22, 2017 at 7:05 PM, OpenShift Bot ***@***.***> wrote:
continuous-integration/openshift-jenkins/merge Running (
https://ci.openshift.redhat.com/jenkins/job/test_pull_
requests_origin_future/464/) (Base Commit: 2ba2ff0
<2ba2ff0>
)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#12982 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p-dFemrrAmBWl5YEVRcoi-pFrF4gks5rfM1YgaJpZM4MCg8t>
.
|
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).