kubeadm: check if all images already exist #129474
Conversation
k8s-ci-robot
added
release-note
|
This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The 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-sigs/prow repository. |
k8s-ci-robot
added
needs-triage
k8s-ci-robot
added
area/kubeadm
sig/cluster-lifecycle
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: afbjorklund The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
is reducing the number of output lines really that much of a problem?
overall i'm not a fan of this change, as is.
a smarter integration would be to instead of having a preliminary RunImagesExistCheck before RunPullImagesCheck, is to improve RunPullImagesCheck to do the following:
- determine what images need to be pulled (must support concurrency.
- if no pulls are needed print message about it.
- if some pulls need to be done print the old message about slow internet and the 'config images' command. then do the pull (must support concurrency).
but if this is only about reducing the number of lines, i don't think there is a good argument. the current implementation and UX works fine. it might have been a problem if the list of images is larger.
| @@ -75,6 +75,10 @@ func runPreflight(c workflow.RunData) error { | |||
| return nil | |||
| } | |||
|
|
|||
| if err := preflight.RunImagesExistCheck(utilsexec.New(), data.Cfg(), data.IgnorePreflightErrors()); err == nil { | |||
There was a problem hiding this comment.
this type of calls ignore early errors from the CRI client.
There was a problem hiding this comment.
I think it was assuming that they would be repeated below.
| if err := preflight.RunImagesExistCheck(utilsexec.New(), initConfig, data.IgnorePreflightErrors()); err == nil { | ||
| fmt.Println("[upgrade/preflight] All images required for setting up a Kubernetes cluster exist") | ||
| return nil | ||
| } | ||
| fmt.Println("[upgrade/preflight] Pulling images required for setting up a Kubernetes cluster") | ||
| fmt.Println("[upgrade/preflight] This might take a minute or two, depending on the speed of your internet connection") |
There was a problem hiding this comment.
this line can be removed, but it helps newbie users who anticipate a progress but are on slow internet.
There was a problem hiding this comment.
It's a good line, since it can take several minutes - with all the progress output staying in the runtime.
It is not, but then again every line added leads to the problem of the output (and the serial log) getting ignored... i.e. cloud-init calls kubeadm init, and then that output goes into the lima serial.log for troubleshooting (if needed) The warnings about the containerd config (for 2.0) is also just spam, and everything is working fine now regardless.
But opened up some issues about it anyway, so was thinking that kubeadm could also cut down on the "logging"...
Let's close it then. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Less lines in the output of kubeadm, if the user has followed instructions and done
kubeadm config images pullWhich issue(s) this PR fixes:
Fixes kubernetes/kubeadm#3145
Special notes for your reviewer:
This does not actually change anything about the pulling of images, just exists early if nothing really needs doing.
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: