feat(deployment/daemonset): Remove imagePullPolicy from charts

[Mart-Kuc]

Pull Request Checklist

• Any new images or tags consumed by charts has been added here
• Chart version has been incremented (if necessary)
• That helm lint and pack run successfully on the chart.
• Deployment of the chart has been tested and verified that it functions as expected.
• Changes to scripting or CI config have been tested to the best of your ability

Types of Change

• Remove ImagePullPolicy: "Always" from daemonset and deployment

According to documentation if you specify version of image tag, automatically ImagePullPolicy is set to IfNotPressent.
And vice versa if you not specify version of image tag (keep latest) ImagePullPolicy is set to Always.

So keep ImagePullPolicy hard coded to Always in charts does not make sense.
It overrides to Always even if image tag is defined.

Linked Issues

Additional Notes

• Helm lint

After the PR is merged

Once the PR is merged, typically upon a new release, the necessary teams will be notified via Slack hook to perform the RKE2 Charts and RKE2 changes. Any developer working on this issue is not responsible for updating RKE2 Charts or RKE2.

[jiaqiluo]

Hi @Mart-Kuc, according to k8s documentation, the overwriting happens only when the ImagePullPolicy is not specified. So setting ImagePullPolicy: "Always" actually takes effect and works as expected.

The documentation also suggests setting the imagePullPolicy of the container to Always if you would like to always force a pull.

Please correct me if I misunderstand anything.

[Mart-Kuc]

Hi @jiaqiluo, you're absolutely right. However, my point is related to cases where pulling images every time may not be desirable. For example, if imagePullPolicy is hardcoded to Always, the image will always be pulled, which might not be suitable in certain scenarios.

Take an air-gapped solution like Rancher or a fully isolated Kubernetes cluster, for instance. In such environments, we don't have a connection to the public network, so pulling images on every pod restart is not feasible. If the Always policy is enforced, the pod won't be able to start because it will continuously try (and fail) to download the image.

By removing the imagePullPolicy setting and specifying a tag for the image (other than latest, since that set defaults imagePullPolicy to Always), the default behavior would be IfNotPresent, which is more appropriate for air-gapped environments.

[jiaqiluo]

Hi @Mart-Kuc, yeah, it makes sense. Just for clarification, the tag latest will never be used because the value will be overwritten by a specific tag from the versionOverrides section.

Instead of removing the ImagePullPolicy line from both files, could you convert it to a new option in the value.yaml and set the default value ("") , in each section under csiController.images and csiNode.images sections? It will provide some flexibility for users who need it.

I noticed the ImagePullPolicy is not set to every container definition in the controller deployment and node daemonset template. Could you add it to all containers?

[jiaqiluo]

The changes look good to me!
There is one last charge we need to add: bumping the appVersion and version in the Chart.yaml file as we are making changes to the chart. Could you bump those two fields to 3.3.1-rancher3?

[Mart-Kuc]

Bumped

[jiaqiluo]

Hi @brandond, could you review this PR? We need two approvals before merging it.