kubeadm: Print all control plane images on init error #66503
Conversation
kubeadm init dumps a comprehensive error message on failure. This message contains some of the control plane images, the fetching of which could have caused the init failure. Unfortunately, this is not the full list of images. Most notably, pause, kube-proxy and DNS images are missing. This fixes the issue, by using GetAllImages. Furthermore, it simplifies the code by deduplicating some checks already made in GetAllImages. Signed-off-by: Rostislav M. Georgiev <[email protected]>
k8s-ci-robot
added
release-note
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rosti If they are not already assigned, you can assign the PR to them by writing 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 |
k8s-ci-robot
added
the
needs-ok-to-test
|
@rosti: Reiterating the mentions to trigger a notification: In 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. |
k8s-ci-robot
added
the
cncf-cla: yes
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
|
@rosti: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
| Images []string | ||
| }{ | ||
| Error: fmt.Sprintf("%v", err), | ||
| Images: images.GetAllImages(i.cfg), |
There was a problem hiding this comment.
If this includes images other than the control plan I think this will be more confusing.
|
@luxas @timothysc you are both right. I can extract portions of I'll be on a PTO for the rest of the week, so I'll probably finish it off next week. WDYT? |
|
Adding the pause image is totally ok, dns and proxy not IMO.
Take your time and enjoy! |
|
@luxas in theory, every image that is referenced by manifests maintained inside kubeadm, should be reported. This really helps for air-gap k8s installations. |
|
Roughly lgtm. |
|
@rosti: PR needs rebase. 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. |
k8s-ci-robot
added
the
needs-rebase
|
This is superseded by #66658 . Closing for that matter. |
What this PR does / why we need it:
kubeadm init dumps a comprehensive error message on failure. This message
contains some of the control plane images, the fetching of which could have
caused the init failure. Unfortunately, this is not the full list of images.
Most notably, pause, kube-proxy and DNS images are missing.
This fixes the issue, by using GetAllImages. Furthermore, it simplifies the
code by deduplicating some checks already made in GetAllImages.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Inspired by kubernetes/kubeadm#1016, but not fixing it.
Special notes for your reviewer:
/cc @kubernetes/sig-cluster-lifecycle-pr-reviews
/area kubeadm
/assign @luxas
/assign @timothysc
Release note: