-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Bug 1684670: data/data/bootstrap: add kube-client-agent-image flag #1401
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
Conversation
* adds new flag and populate with release image metadata Signed-off-by: Sam Batschelet <[email protected]>
|
/hold |
|
/hold cancel |
|
/retest |
|
/cc @wking @abhinavdahiya PTAL openshift/machine-config-operator#538 has merged allowing this to merge to complete chicken and egg. |
|
@hexfusion: GitHub didn't allow me to request PR reviews from the following users: PTAL. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
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.
Why this? Can we drop this commit until we move the etcd signer into the release image references?
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.
Why this? Can we drop this commit until we move the etcd signer into the release image references?
Sure I just figured it was better to not have the non RHEL image, will update.
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.
Hmm, now that you've dropped this, is it still fixing rhbz#1684670 ? I think we need openshift/origin#22325 so we can drop etcdCertSignerImage completely.
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.
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.
So if that if signer-server is refed in origin we can just pull the sha like the others or?
podman run --quiet --rm ${release} image kube-etcd-signer-server
Wouldn't this work now?
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.
guess not
podman run --quiet --rm registry.svc.ci.openshift.org/ci-op-mnr8nyp6/release:latest image kube-etcd-signer-server
[...]
1 image.go:36] error: error: Unknown name requested, could not find kube-etcd-signer-server in UpdatePayload
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.
So if that if signer-server is refed in origin we can just pull the sha like the others or?
podman run --quiet --rm ${release} image kube-etcd-signer-serverWouldn't this work now?
only images required by operators and some hard coded defaults are available in the release image.
[from this CI run release image]
oc adm release info registry.svc.ci.openshift.org/ci-op-mnr8nyp6/release:latest --image-for=kube-etcd-signer-server error: no image tag "kube-etcd-signer-server" exists in the release image registry.svc.ci.openshift.org/ci-op-mnr8nyp6/release:latestThere 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.
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 see so with openshift/origin#22325 updated per your comment we will want
ETCD_CERT_SIGNER_IMAGE=$(podman run --quiet --rm ${release} image kube-etcd-signer-server) ? and remove etcdCertSignerImage ref?
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.
|
@abhinavdahiya @wking https://bugzilla.redhat.com/show_bug.cgi?id=1684670 was for kube-client-agent can we merge this PR to iterate on this? |
|
/test e2e-aws |
|
/lgtm
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, hexfusion The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Ah, sorry, I was confused and removed the |
|
Heh, and as soon as I ask for help, it starts working again :p. Sorry for the noise |
|
Oops 😇 |
depends on: openshift/machine-config-operator#538
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1684670