-
Notifications
You must be signed in to change notification settings - Fork 43
manifests/image-references: add kube-etcd-signer-server #22
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
Signed-off-by: Sam Batschelet <[email protected]>
|
/test e2e-aws |
|
we are going to give this another go PTAL /cc @smarterclayton |
|
/hold cancel |
|
/test e2e-aws |
|
@sttts requested we move this to a more appropriate place. Where is this value actually used? Machine-config-operator? If so it belongs there. |
|
@smarterclayton the image is used by bootstrap node via installer https://github.com/openshift/installer/blob/af56e2db2e2601156a1ebe56269c7deb67f3c725/data/data/bootstrap/files/usr/local/bin/bootkube.sh.template#L225. The endpoint it creates is consumed by etcd init https://github.com/openshift/machine-config-operator/blob/8381bc2c071125feb44c8b3bee25a8453cc0efe5/templates/master/00-master/_base/files/etc-kubernetes-manifests-etcd-member.yaml#L42 |
|
I am sure you know this but it is unclear to me where that makes it land. |
|
@abhinavdahiya if it's consumed by something in MCO I think the best place to take the reference is there. This code knows nothing about specific components, and even though in the future I could see us moving bootkube.sh to functionality in this repo at the current time it's a thin shim deliberately. Opinions? |
The MCO consumes the the |
|
Is bootkube the component that creates the reference? |
Hmm 😕 what |
|
The reference to the image |
from @hexfusion comment
the bootkube.sh service on the bootstrap node calls the reference. |
|
Why doesn't installer take the reference? |
cluster-bootstrap/manifests/image-references Lines 5 to 8 in fb8d5d4
kube-etcd-signer-server doesn't feel any different.
IMO keeping the image requirements for bootstrap node here |
|
@smarterclayton @abhinavdahiya have we come to an agreement?:) This is now a blocker for etcd metrics. |
|
/retest |
|
@sttts would you mind chiming in on this I know you were against this initially. |
|
@hexfusion dependencies for bootkube.sh do belong where bootkube.sh is stored. Today, cluster-bootstrap knows nothing about the bootkube.sh bootstrapping process. Any etcd dependency here is on the wrong layer: cluster-bootstrap is only a generic tool launching some static pods and creating some manifests. It has no knowledge about what it actually creates. I could imagine a world where we move the bootkube.sh logic into cluster-bootstrap, like a |
|
/test e2e-aws |
|
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hexfusion, mfojtik 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 |
|
/test e2e-aws |
4 similar comments
|
/test e2e-aws |
|
/test e2e-aws |
|
/test e2e-aws |
|
/test e2e-aws |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/test e2e-aws |
This PR is a cherry-pick of a1f5d26 reverted by #21.
/hold