Add options to control images pulling of kubelet #11094
Conversation
k8s-ci-robot
added
kind/feature
k8s-ci-robot
added
the
size/S
tu1h
force-pushed
the
kubelet_pull_image
branch
3 times, most recently
from
2f3ca71
to
150c92e
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: MrFreezeex, tu1h 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 |
tu1h
force-pushed
the
kubelet_pull_image
branch
from
150c92e
to
54660da
Compare
k8s-ci-robot
removed
the
lgtm
| @@ -174,4 +174,8 @@ topologyManagerScope: {{ kubelet_topology_manager_scope }} | |||
| tracing: | |||
| endpoint: {{ kubelet_tracing_endpoint }} | |||
| samplingRatePerMillion: {{ kubelet_tracing_sampling_rate_per_million }} | |||
| {% endif %} | |||
| serializeImagePulls: {{ kubelet_serialize_image_pulls | bool | lower }} | |||
There was a problem hiding this comment.
HI @tu1h
Is it OK change it like to:
{% if kubelet_serialize_image_pulls is defined %}
....
{% endif %}
like the other optional variable.
There was a problem hiding this comment.
Maybe, but @MrFreezeex suggested to avoid using is defined in Kubespray. :-)
There was a problem hiding this comment.
Hmmm perhaps we could be adding this line in the if in the line just below (and in that case it could be "hard coded" to false), there are a few other example that would be like that. It would prevent the usage of is defined and not add those line by default (if this was the concern?)
|
(forgot to readd the lgtm label sorry) |
k8s-ci-robot
added
the
lgtm
| # Whether pull images one at a time | ||
| kubelet_serialize_image_pulls: true | ||
| # The maximum number of image pulls in parallel. This option takes effect if kubelet_serialize_image_pulls is false | ||
| kubelet_max_parallel_image_pulls: 5 |
There was a problem hiding this comment.
HI @tu1h
The maxParallelImagePulls default value is nil: https://kubernetes.io/docs/reference/config-api/kubelet-config.v1beta1/#kubelet-config-k8s-io-v1beta1-KubeletConfiguration.
So we need to comment on it by default like:
# kubelet_max_parallel_image_pulls: 5
There was a problem hiding this comment.
I updated it, please check if it is applicable.
k8s-ci-robot
removed
the
lgtm
|
New changes are detected. LGTM label has been removed. |
Signed-off-by: tu1h <[email protected]>
There was a problem hiding this comment.
To me, this means we can simply get away with only 1 variable maxParrallelImagePulls, defaulting to 1 (that or nil has the exact same effect.
This would have the benefits of a simpler template (no need for is defined which is something I'm actively trying to get rid of 😆 )
It's a nice solution for me. What do you think? @yankay @MrFreezeex |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR provides two options to control the images pulling efficiency of kubelet.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: