Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
MaxFedotov opened this issue Aug 2, 2022 · 15 comments
Labels
area/clusterclass Issues or PRs related to clusterclass kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@MaxFedotov
Copy link
Contributor

MaxFedotov commented Aug 2, 2022

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

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 2, 2022
@fabriziopandini
Copy link
Member

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

@enxebre
Copy link
Member

enxebre commented Aug 4, 2022

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
Copy link
Member

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
Copy link
Member

/triage accepted

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

  • Propagate existing metadata both to MD/KCP and Machines (to be verified if this as undesirable side effects)
  • Add new metadata fields for MD/KCP objects, TBD UX

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 5, 2022
@MaxFedotov
Copy link
Contributor Author

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
Copy link
Contributor

Related discussion about node annotations not propagating via Machine update: #6255

@jackfrancis
Copy link
Contributor

Also related: #5880

@MaxFedotov
Copy link
Contributor Author

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 Support to propagate properties in-place from MachineDeployments to Machines #5880 or 📖 Label Sync Between MachineDeployment and underlying Kubernetes Nodes #6255)

@sbueringer
Copy link
Member

Just to provide an update. We included this in this proposal: #7331

@k8s-triage-robot
Copy link

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:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 1, 2023
@fabriziopandini
Copy link
Member

@sbueringer @ykakarap should this issue be closed due to #7917 being merged?

@sbueringer
Copy link
Member

I think yes.

@sbueringer
Copy link
Member

sbueringer commented Feb 2, 2023

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

That's not solved yet, but we have another issue for it (#7731)

@fabriziopandini
Copy link
Member

/close
as per comments above

@k8s-ci-robot
Copy link
Contributor

@fabriziopandini: Closing this issue.

In response to this:

/close
as per comments above

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@killianmuldoon killianmuldoon added the area/clusterclass Issues or PRs related to clusterclass label May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/clusterclass Issues or PRs related to clusterclass kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
8 participants