search

kubernetes-sigs / cluster-api

Home for Cluster API, a subproject of sig-cluster-lifecycle
https://cluster-api.sigs.k8s.io
Apache License 2.0
3.57k stars 1.31k forks source link

Labels and annotations for MachineDeployments and KubeadmControlPlane created by topology controller #7006

Closed MaxFedotov closed 1 year ago

MaxFedotov commented 2 years ago

User Story

As a user I would like to specify labels and annotations for MachineDeployment and KubeadmControlPlane created by topology controller to be used used by cluster-autoscaler or for filtering.

Detailed Description

It is possible to specify labels and annotations in cluster topology spec, but they are propagated only to machines created by MachineDeployment and KubeadmControlPlane.

As documented in cluster-autoscaler readme, in order to use it with Managed Topology, users had to skip spec.topology.workers.machineDeployments[].replicas field, which cause topology controller to always create MachineDeployment with only one replica. Users can later add specific cluster-autoscaler annotations in order to increase number of replicas. And as this annotations cannot be added to MachineDeployment during provisioning, it is impossible to create them with any other number of replicas other then one.

It will be nice to have a way to pass labels and annotation from Cluster topology spec to MachineDeployment and KubeadmControlPlane and also be able to update them without triggering rollout for corresponding Machines

/kind feature

fabriziopandini commented 2 years ago

/area topology @vincepri another possible improvement for cluster.spec.topolgy and/or ClusterClass

enxebre commented 2 years ago

Is there any other use cases we can think of for labels/annotations? e.g Syncing from class to Nodes. For autoscaling in particular I'd expect this to be baked in MachineDeploymentClass API eventually e.g .autoscaling {min: , max: }

fabriziopandini commented 2 years ago

Syncing from classes to a node can happen for free if we use existing metadata fields, otherwise, if we add a new field for node labels it should be surfaced in CC as well

fabriziopandini commented 2 years ago

/triage accepted

As discussed in the Aug 5th issue triage meeting we should evaluate options:

MaxFedotov commented 2 years ago

Propagate existing metadata both to MD/KCP and Machines (to be verified if this as undesirable side effects)

One side effect would be that any update of labels\annotations (e.g. for autoscaler) will cause Machine rollout

jackfrancis commented 2 years ago

Related discussion about node annotations not propagating via Machine update: https://github.com/kubernetes-sigs/cluster-api/pull/6255

jackfrancis commented 2 years ago

Also related: https://github.com/kubernetes-sigs/cluster-api/issues/5880

MaxFedotov commented 2 years ago

After the discussion during Cluster API meeting it was proposed to separate this into two different issues:

  1. Propagate labels and annotations from ClusterClass and Cluster.spec.topology not only to Machines, but also to MD and KCP
  2. Prevent triggering of Machine rollout when labels or annotations in MachineDeployment.spec.template.metadata or KubeadmControlPlane.spec.machineTemplate.metadata are updated (can be done as a part of #5880 or #6255)
sbueringer commented 1 year ago

Just to provide an update. We included this in this proposal: https://github.com/kubernetes-sigs/cluster-api/pull/7331

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

fabriziopandini commented 1 year ago

@sbueringer @ykakarap should this issue be closed due to https://github.com/kubernetes-sigs/cluster-api/pull/7917 being merged?

sbueringer commented 1 year ago

I think yes.

sbueringer commented 1 year ago

also be able to update them without triggering rollout for corresponding Machines

That's not solved yet, but we have another issue for it (https://github.com/kubernetes-sigs/cluster-api/issues/7731)

fabriziopandini commented 1 year ago

/close as per comments above

k8s-ci-robot commented 1 year ago

@fabriziopandini: Closing this issue.

In response to [this](https://github.com/kubernetes-sigs/cluster-api/issues/7006#issuecomment-1414404487): >/close >as per comments above Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.