Validate Dask Cluster Names

[jo-migo]

Issues

#870
#826

Details

This PR introduces cluster name validation in order to avoid the invalid state in which a DaskCluster resource with a too-long or RFC-1123-noncompliant name is created but cannot be deleted while the operator retries infinitely to create a scheduler service (see #826 for more details).

Alternative Suggestion

It would be nicer (from a UX perspective) do this validation with a validation admission webhook which would directly reject the cluster creation with an error message, but this would require adding cryptography and certbuilder as dependencies to the operator, or mounting user-provided SSL certs into the operator container. In the current approach, the request to create the cluster succeeds, and one has to look into the events to understand why the cluster is then in an Error state.

Proof

In those examples, an error event is emitted (as shown) and the clusters are able to be manually deleted via kubectl.

[jacobtomlinson]

Thanks for raising this!

I agree an admission time check would be cleaner. I wonder if there is a way to use validation rules to achieve this.

We have a long term god to add better support for hooks to the controller. I don't mind adding the dependencies, but the CI and test machinery is the biggest roadblock. Mainly because our workflow is to run the controller outside of the cluster when developing/testing.

In the meantime I'm happy to merge this PR. Would you mind adding some tests that check for clusters with invalid names do go into an error state as expected?

Also it would be nice to move the checking logic somewhere common and also use it in KubeCluster as a client side check.

[jo-migo]

Thanks @jacobtomlinson for your quick review!

I've added some tests and updated KubeCluster to use the same validation.

Good point about the validation rules. It looks like you could specify metadata on the DaskCluster OpenAPI schema to be a custom ObjectMeta schema having:

   properties:
      name:
          type: string
          pattern: '^(?=.,53$)[a-z0-9]([-a-z0-9]*[a-z0-9])?$'

The only minor downside of that approach is that the 53 would be hard-coded instead of computed using the scheduler service name (63 - len("-scheduler")). An update to the scheduler service name without update to this regex could cause a regression.

Let me know what you prefer to go with.

[jacobtomlinson]

Thanks @jo-migo.

I think given that validation rules are only Kubernetes 1.29+ we should definitely accept the changes here. It leaves us in a much better state than we are currently in.

However I think I would prefer to go down the route of validation rules over an admission webhook in the future. I agree hardcoding the length isn't ideal, perhaps a mitigation to future issues is to add a comment where we set -scheduler explaining that the CRDs need updating if this is ever changed.

I'll merge this now. @jo-migo do you have any interest in exploring the validation rules change in a follow on PR?