Helm chart: keep supporting `nvidia-dra-driver-gpu-component` label for now

[jgehrcke]

Selectors based on nvidia-dra-driver-gpu-component=kubelet-plugin and nvidia-dra-driver-gpu-component=controller should keep working in the next major release.

Consumers are for example docs and test harness code.

Found in the context of debugging an upgrade test, see #1078. Quote from there:

For a grace period, we can also apply the old label key/value pair and the new one (that may be the ideal choice). Interesting insight: these component labels should maybe be considered part of the public interface, and hence changing them may (in the future) be considered a breaking change (especially relevant if we were to use semantic versioning).

[shivamerla]

@jgehrcke we had to adopt these as per the decision to rename the repository and the helm chart. See details here: #988

These upgrade/downgrade tests will need to be updated once we have the upstream release.

[jgehrcke]

we had to adopt these as per the decision to rename the repository and the helm chart

I understand.

My recommendation (see above) is to keep the old label working (next to the new one) for a grace period (for e.g. one release).

These labels are part of a public interface.

A test suite is just one of many consumers of these labels.

Recommended action items:

• Add the old key/value pairs again.
• Discuss the public interface question (also see above -- need to have a strong opinion about things like that if we want to be able to apply semantic versioning).

[shivamerla]

Just adding these labels won’t solve the problem. The plugin, controller, and webhook resource names are derived from the chart name, so they’ll be installed as dra-driver-nvidia-gpu instead of nvidia-dra-driver-gpu. If we force an upgrade of an existing release with the new chart, we’ll end up with duplicate resources (daemonsets, deployments, etc.).

Having said that, users still have an option to explicitly provide nameOverride=nvidia-dra-driver-gpu during helm upgrade which retains same values (k8s manifests, labels etc derive from that name instead of the chart name) as before. This is a good option for adding upgrade tests.

[jgehrcke]

adding these labels won’t solve the problem

Do we think about different problems?

The problem I have in mind is really boring and simple: the old selectors do currently not work anymore, and commands/scripts/automation in user environments that use these selectors will break.

The simple solution that I like:

• make both, the old and the new selectors work at the same time.
• announce that the selectors nvidia-dra-driver-gpu-component=<component> are deprecated.
• remove the old selectors in a future release.

Less abstract: for example, commands and scripts using

kubectl [...] -l nvidia-dra-driver-gpu-component=kubelet-plugin [...]

will stop working.

Statically adding the old labels in addition to the new ones (dynamically generated by the Helm chart) I think has one clear upside, and I believe no downside.

In #359 we introduced these labels for external consumers. Internally they are afair not used. This is meant to be a public interface (albeit not the most important one). Breaking it will not be the end of the world. But here it is arguably very easy to go through a deprecation cycle at virtually no effort; and hence it should be done :-) (unless I miss something, see below).

More specifically: plugin pods would for example have both of these labels:

dra-driver-nvidia-gpu-component=kubelet-plugin
nvidia-dra-driver-gpu-component=kubelet-plugin

That is, the old and the new selector would pick the same set of objects. Two different ways to reach the same thing.

If we force an upgrade of an existing release with the new chart, we’ll end up with duplicate resources (daemonsets, deployments, etc.).

I can't see this right now -- can you elaborate? Of course I might have missed something here.

explicitly provide nameOverride=nvidia-dra-driver-gpu during helm upgrade

This affects more parameters than necessary, and puts a tiny bit more burden on users.

I am still interested in understanding better where our misunderstanding is and believe that I haven't grasped a part of your strong technical opinion yet. Having said that, this particular problem isn't quite worth sending very long love letters for 💌.

[shivamerla]

Closing this as fixed now with #1119