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

ClusterClass template name *only* update triggers reconcilliation #5227

Closed
killianmuldoon opened this issue Sep 10, 2021 · 9 comments
Closed
Assignees
Labels
area/clusterclass Issues or PRs related to clusterclass kind/bug Categorizes issue or PR as related to a bug.
Milestone

Comments

@killianmuldoon
Copy link
Contributor

killianmuldoon commented Sep 10, 2021

What steps did you take and what happened:
[A clear and concise description on how to REPRODUCE the bug.]
Reproduced in CAPD but assumed to be an issue in the ClusterClass controller:

  1. Create a clusterclass that includes two identical DockerMachineTemplates with different names e.g. template1,template2
  2. Create a cluster that uses the clusterClass from (1)
  3. Wait until the cluster has been reconciled fully i.e. machines and nodes up and running
  4. Edit the clusterclass to change the template under workers.machineDeployments[*].template.infrastructure.ref.name e.g.
    from template1 to template 2
  5. View clusterctl describe cluster or similar to see machine recreation happen based on the name change, even though the spec of the machines is identical.

What did you expect to happen:
Machine reconciliation should not occur unless the underlying objects - in this case DockerMachineTemplates - are actually different. Name-only changes shouldn't trigger rolling out of new MachineDeployments.

Environment:

/kind bug

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Sep 10, 2021
@killianmuldoon
Copy link
Contributor Author

@fabriziopandini @sbueringer Am I right in saying the above is a bug that should be fixed?

@sbueringer
Copy link
Member

I think yes. We can pair on debugging this by running it through the debugger if you want (but next week)

@ykakarap
Copy link
Contributor

/area topology

@sbueringer
Copy link
Member

sbueringer commented Sep 13, 2021

@killianmuldoon and I debugged it and the rollout is triggered because of metadata changes in the desired template

{
  "metadata": {
    "annotations": {
      "cluster.x-k8s.io/cloned-from-name": "my-cluster-md-0-2",
      "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"infrastructure.cluster.x-k8s.io/v1alpha4\",\"kind\":\"DockerMachineTemplate\",\"metadata\":{\"annotations\":{},\"name\":\"my-cluster-md-0-2\",\"namespace\":\"default\"},\"spec\":{\"template\":{\"spec\":{}}}}\n"
    }
  }
}

So I think it's fine for now. I want to open an issue for consistent metadata (label/annotations) rollout anyway. I'll link it here and then close the current issue

@sbueringer
Copy link
Member

/assign

@fabriziopandini
Copy link
Member

I want to open an issue for consistent metadata (label/annotations) rollout anyway. I'll link it here and then close the current issue

I have still to investigate this, but it seems to me that we should not trigger rollout to align annotations or labels. However this has different nuances in KCP (and possible other control plane providers) and machine deployments, and I need some time to dig into...

@sbueringer
Copy link
Member

@fabriziopandini My plan for the issue was mostly to investigate how we currently propagate labels/annotations on KCP/MD/MS to MS/M InfrastructureMachineTemplate, BootstrapConfigTemplate, InfrastructureMachine and BootstrapConfig and then start a discussion in the issue if the current state is as intended or what we want to change and when.

@fabriziopandini fabriziopandini added this to the v0.4 milestone Sep 15, 2021
@sbueringer
Copy link
Member

Created the issue here: #5240

/close

@k8s-ci-robot
Copy link
Contributor

@sbueringer: Closing this issue.

In response to this:

Created the issue here: #5240

/close

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/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

5 participants