feat: auto-add member-cluster-name label to MemberCluster for override selection by cluster name#578
Conversation
5d6d127 to
3ec9d4c
Compare
7b33ca1 to
5723b43
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
5723b43 to
ad8245a
Compare
| // MemberClusterNameLabel is a label automatically added to MemberCluster objects | ||
| // with the value set to the MemberCluster's name. This enables selecting clusters | ||
| // by name in ResourceOverride and ClusterResourceOverride via labelSelector. | ||
| MemberClusterNameLabel = FleetPrefix + "member-cluster-name" |
There was a problem hiding this comment.
In Azure Kubernetes Fleet Manager, we have these 2 labels on the member cluster (there are more but these 2 are the most relevant ones)
fleet.azure.com/cluster-name=testmember-0
fleet.azure.com/member-name=testmember-0
fleet.azure.com/cluster-name is the AKS cluster name
fleet.azure.com/member-name is the fleet member name
I think this should be named
kubernetes-fleet.io/member-name=testmember-0
, which follows the existing Azure label convention
There was a problem hiding this comment.
Simon Waight (@sjwaight) I think having kubernetes-fleet.io/member-name=testmember-0 in addition to
fleet.azure.com/cluster-name=testmember-0
fleet.azure.com/member-name=testmember-0
is acceptable in Azure Kubernetes Fleet Manager's member cluster CRs?
There was a problem hiding this comment.
I think in an ideal world we would have just the kubernetes-fleet.io labels and retire the fleet.azure.com ones for cluster name and member name, as these concepts are portable between KubeFleet and Fleet Manager. I would recommend we add both: kubernetes-fleet.io/member-name and kubernetes-fleet.io/cluster-name (if it's not already there). We can add these to our documentation for Fleet Manager, but keep the existing labels to not break the current API contract.
There was a problem hiding this comment.
fleet.azure.com labels are owned by the azure service, and the service is the source of truth for them (we set these labels). this ensures parity in label selectors between controlplane and dataplane.
The Kubefleet system should not care more than that about them.
Having built-in labels in kubefleet make sense. They should mirror the ones from the service, when they serve the same purpose. That's what the namespacing of labels is for :).
We will certainly have labels that only kubefleet can define in the future too, because it can get them from the members directly.
I think that we should also always consider built-in kubernetes labels. for example topology has a region and zone label that might be relevant?
https://kubernetes.io/docs/reference/labels-annotations-taints/#topologykubernetesioregion
…e selection by cluster name Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com>
Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com>
a111e79 to
96d953f
Compare
Description of your changes
Automatically adds a kubernetes-fleet.io/member-cluster-name label to MemberCluster objects during reconciliation, enabling users to select clusters by name in ResourceOverride and ClusterResourceOverride via labelSelector.
Fixes #101
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
Locally, yes. Also added necessary tests and updated existing ones.
Special notes for your reviewer
N/A