Add option to define toleration on k8s jobs

[munishchouhan]

This PR will add config to setup toleration for build k8s jobs

[munishchouhan]

@gavinelder can I get nodes in dev with taints for AMD64 and ARM64 to test this feature?

[pditommaso]

@gavinelder is node selector AND taint rule really needed? I'd prefer keeping only one of the two

[pditommaso]

@gavinelder can you please have a look at previous comment at your convenience

[bebosudo]

nodeSelector and taints/tolerations are two different mechanisms to instruct the k8s scheduler:

• nodeSelector: mylabel=xyz tells the scheduler to only use nodes with a mylabel label equal to xyz
  • this doesn't prevent other pods from using the nodes labelled with mylabel=xyz
  • in a context where the cluster is shared between wave build pods and non-wave pods it means that non-wave pods could end up using the wave nodes
  • if the nodeSelector label used doesn't match a specific nodegroup, like kubernetes.io/arch=amd64, it would mean that wave pods may get scheduled on non-wave nodes
• on the other hand, when a taint is added to a node, only pods with a matching toleration will be able to run on that node
  • this means that you could set a taint like service=wave-builds on all nodegroups dedicated to wave builds, and if wave build pods have a matching tolerations they could run on those nodes, but no other pod would run on the wave-dedicated nodes
  • so taints/tolerations are a mechanism to prevent "contamination" between pods of different kinds
  • taints/tolerations can be used in conjunction with nodeSelector: in the taint example above, you could mark all wave build nodegroups with a generic service=wave-build taint, then use nodeSelector to direct the scheduler on more specific nodes, like amd/arm nodes, or low-priority vs premium wave build nodegroups, etc

So IMHO we should keep both mechanisms, hope it makes sense

[pditommaso]

I know, but want to simplify this, so if taint should our need, the other should be removed

[gavinelder]

I believe that wave should support both as they have unique use-cases however reviewing this code I don't see why we need to implement it in this fashion with both an inclusion of an AMD & ARM toleration as they are the same.

There are additional manifest extensions we should be aware of at the same time as implementing this feature such as container.apparmor.security.beta.kubernetes.io being deprecated and replaced with https://kubernetes.io/docs/tutorials/security/apparmor/ and ttlSecondsAfterFinished which will auto-delete kubernetes pods once they have completed allowing us to remove the cleanup logic from wave.

[pditommaso]

Please include and example how the taint you are expected to be defined

[pditommaso]

Thinking more about this, I believe that instead of implementing this into Wave we should look into having a mutating admission controller that applies the taint or ant other policy based on generic pod annotation.

There are good solution for this, for example https://kyverno.io/ and surely more are available.

[bebosudo]

A mutation admission controller was already considered when opening the issue: #702 (comment)
That delegates maintenance of wave build pods to the final user, instead of us making wave use the native solution.

[pditommaso]

Yes, that's the point. It looks like requirements vary depending the target installation and therefore we are not going to support this in Wave other then generic annotation.

The admission controller provides a better separation of concerns and finer control.

[pditommaso]

We discussed internally and agreed to implement a solution based on Kyverno (or similar approach)