Set the csi-node-driver DaemonSet PriorityClass to system-node-critical

[innossh]

Description

It should be set the csi-node-driver DaemonSet PriorityClass to system-node-critical.

When I updated the tigera-oprator Helm chart (quay.io/tigera/operator:v1.27.1 -> v1.28.0), some csi-node-driver pods were stack in pending because they couldn't be scheduled to a node. I'd like to set the PriorityClass to system-node-critical, like the calico-node DaemonSet.
ref. #1473

For PR author

• Tests for change.
• If changing pkg/apis/, run make gen-files
• If changing versions, run make gen-versions

For PR reviewers

A note for code reviewers - all pull requests must have the following:

• Milestone set according to targeted release.
• Appropriate labels:
  • kind/bug if this is a bugfix.
  • kind/enhancement if this is a a new feature.
  • enterprise if this PR applies to Calico Enterprise only.

[CLAassistant]

All committers have signed the CLA.

[innossh]

@Josh-Tigera Thank you for the review! Could you merge this PR?

[tmjd]

LGTM
Thank you for submitting this.

[tmjd]

/sem-approve

[jjchambl]

Does this set the PriorityClass by default? Our clusters are running v1.29.1 of the tigera operator, and the csi-node-driver containers still do not have the PriorityClass set.

[tmjd]

@jjchambl yes it should be set, the changes from this PR are in the v1.29.1 release. Are you perhaps using RKE(2) or MKE or have a mutating webhook that might be removing the PriorityClass?
I think I've seen odd behavior with those installations because they have a mutating webhook that modifies resources when they're created or updated to remove some permissions or tolerations because the service account creating them doesn't have a binding to a certain cluster role.

[jjchambl]

@tmjd thanks for the reply. We're running in EKS, but it's possible there's a mutating webhook somewhere that I'm missing. Will look into that.

[jjchambl]

@tmjd thanks for the reply. We're running in EKS, but it's possible there's a mutating webhook somewhere that I'm missing. Will look into that.

On second thought, the calico-node Daemonset has and applies the priorityClassName just fine; the field is missing from the csi-node-driver Daemonset entirely. I don't think it would make sense for a mutating webhook to remove one of those and not the other.

[tmjd]

I agree @jjchambl, I wouldn't expect that to happen. The only way I'd guess it would happen was if there was an install which included priorityClass for calico-node but not csi-node-driver, then the webhook installed, and then upgraded operator to one that added priorityClass to csi-node-driver. Still probably possible but very unlikely.

I'm not really sure how this would happen.

I just tried out in a cluster the v1.29.1 image and it had the priorityClassName, I even disabled operator, removed it from the daemonset, and then re-enabled the operator and the priorityClassName was added back.
EKS should not matter because the csi-node-driver always has the priorityClassName added.