🌱 Provide namespace of the referenced ClusterClass in TEMPLATENAMESPACE

[Danil-Grigorev]

What this PR does / why we need it:

Allow to display ClusterClass and templates namespace in the table output of Cluster objects:

> kubectl get clusters 
NAME              CLUSTERCLASS   PHASE          AGE   VERSION
capi-quickstart   quick-start    Provisioning   96m   v1.32.0
> kubectl get clusters -o wide
NAME              CLUSTERCLASS   TEMPLATENAMESPACE   PHASE          AGE   VERSION
capi-quickstart   quick-start    test                Provisioning   96m   v1.32.0

A cluster without classNamespace:

NAME              CLUSTERCLASS   TEMPLATENAMESPACE   PHASE   AGE   VERSION
capi-quickstart   quick-start    default                     81m   v1.32.0

Using classNamespace as the source for the value, unless is not specified, in which case the metadata.namespace is displayed.

This will improve UX and reduce confusion, when looking up referenced templates or the ClusterClass in the namespace of the Cluster, when those are located elsewhere.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Related to #5673

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign fabriziopandini for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Danil-Grigorev]

/area api

[sbueringer]

TemplateNamespace seems a bit misleading to me.

We have many templates. Some referenced in control plane / MD / MS objects. Some referenced from the ClusterClass. Wouldn't the more appropriate name be ClusterClassNamespace?

Ideally maybe something that ends up as CLUSTERCLASS_NAMESPACE if that is possible? (not sure if there are naming conventions for columns)

[Danil-Grigorev]

I think CLUSTERCLASS_NAMESPACE is better here. The problem is thought that it will be displayed without spec.template.class being set, if queried with kubectl get clusters -o wide. What I wanted to convey is CLUSTER_CLASS_TEMPLATES_NAMESPACE, as the location from where the definitions originate and being cloned. Maybe in combination with ClusterClass field defaulting to empty is not that misleading.

[sbueringer]

Hm yeah an empty column is probably confusing in all variants. Ideally we could fallback to show the Cluster.Namespace instead of course (I assume with CEL that would be possible, with jsonPath today probably not?)

[Danil-Grigorev]

This is possible with the expression provided.

1. If the classNamespace is not set, it uses the namespace of the cluster:

NAME              CLUSTERCLASS   CLUSTERCLASS_NAMESPACE   PHASE   AGE   VERSION
capi-quickstart   quick-start    default                          81m   v1.32.0

2. If the classNamespace is set, it displays the value

NAME              CLUSTERCLASS   CLUSTERCLASS_NAMESPACE   PHASE   AGE   VERSION
capi-quickstart   quick-start    otherns                          81m   v1.32.0

3. The only thing it can’t do now, but a CEL will be able (though it will be a while?) is to not display a namespace if topology.class is empty:

NAME              CLUSTERCLASS   CLUSTERCLASS_NAMESPACE   PHASE   AGE   VERSION
capi-quickstart                  default                          81m   v1.32.0

The field is present in the -o wide now anyway, so it is not a problem?

[sbueringer]

Ah got it, should have seen that.

The field is present in the -o wide now anyway, so it is not a problem?

Not sure, looks a bit awkward for folks not using ClusterClass

[fabriziopandini]

I don't know exact timeline, but if CEL is an option in the near future, what if we defer this change?

[sbueringer]

No strong opinions.

If we wait for CEL we have to wait for this KEP to reach GA + ~ 1,5 years. So we're probably talking by delaying at least by 2 or 2,5 years

[sbueringer]

But on the other side. I don't see a way to implement this today which is not confusing in some cases

[fabriziopandini]

Yep, it feels that we are forcing a solution that doesn't make anyone happy.
Also, we are going to review all the columns is a short term for v1beta2, and we already know that we have space constraints

[sbueringer]

I would probably go with not adding the column for now.

(Note: I think it's fine if CAPI distributions are adding additional columns like this downstream if they want. They might not have all the use cases so the column might not be confusing there)