[EKS controller]: can't change a size of instance in the node group

[gecube]

Good day!

I had an node group:

apiVersion: eks.services.k8s.aws/v1alpha1
kind: Nodegroup
metadata:
  name: production-fix-ng-20241010-b
  namespace: infra-production
spec:
  name: production-fix-ng-20241010-b
  clusterName: production
  diskSize: 100
  subnetRefs:
    - from:
        name: production-private-eu-west-2b
  nodeRole: arn:aws:iam::*****:role/eks-production-nodegroup-role
  scalingConfig:
    minSize: 1
    maxSize: 1
    desiredSize: 1
  instanceTypes:
    - m5.large
  taints:
    - key: "node-role.kubernetes.io/fix"
      value: ""
      effect: "NO_SCHEDULE"
  labels: {}
  amiType: BOTTLEROCKET_x86_64

Then I am changing the instance type:

apiVersion: eks.services.k8s.aws/v1alpha1
kind: Nodegroup
metadata:
  name: production-fix-ng-20241010-b
  namespace: infra-production
spec:
  name: production-fix-ng-20241010-b
  clusterName: production
  diskSize: 100
  subnetRefs:
    - from:
        name: production-private-eu-west-2b
  nodeRole: arn:aws:iam::*****:role/eks-production-nodegroup-role
  scalingConfig:
    minSize: 1
    maxSize: 1
    desiredSize: 1
  instanceTypes:
    - m5.large
  taints:
    - key: "node-role.kubernetes.io/fix"
      value: ""
      effect: "NO_SCHEDULE"
  labels: {}
  amiType: BOTTLEROCKET_x86_64

The node group is stuck:

status:
  ackResourceMetadata:
    arn: 'arn:aws:eks:eu-west-2:*****:nodegroup/production/production-fix-ng-20241010-b/48c98b02-726a-81e5-2958-d89886c0a63b'
    ownerAccountID: '*****'
    region: eu-west-2
  conditions:
    - lastTransitionTime: '2024-11-10T13:10:06Z'
      status: 'True'
      type: ACK.ReferencesResolved
    - lastTransitionTime: '2024-11-10T13:10:06Z'
      message: Nodegroup is currently being updated
      status: 'False'
      type: ACK.ResourceSynced
    - message: 'nodegroup has started asynchronously updating, cannot be modified until ''ACTIVE'''
      status: 'True'
      type: ACK.Recoverable
  createdAt: '2024-11-10T12:20:41Z'
  health: {}
  modifiedAt: '2024-11-10T13:09:56Z'
  resources:
    autoScalingGroups:
      - name: eks-production-fix-ng-20241010-b-48c98b02-726a-81e5-2958-d89886c0a63b
  status: ACTIVE

[gecube]

Unfortunately, I was forced to remove this node group, as no changes were introduced after 2h of awaiting.

[fspaniol]

I wanted to up this thread. I do understand that the nodegroup instance types can't be updated with the SDK: https://repost.aws/questions/QUCBGgvgAcRLqQC8FayUSUAg/changing-eks-node-group-instance-type

The quick way to do this is to change the .spec.name and let eks-controller create a new nodegroup.

The problem here, is that the previous one is left dangling. What's the proposed solution?

[fspaniol]

Maybe @michaelhtm or @rushmash91 can help here?

[michaelhtm]

Not sure if that approach would work..the only way to retrieve a nodegroup is by its name combined with clusterName. If the user changes the spec.name we would no longer be able to retrieve and cleanup the dangling node-group..
cc: @a-hilaly

[a-hilaly]

IMO spec.name should marked as immutable along with the instance size (cc @rushmash91 )

[rushmash91]

Yes, I agree there isn't a way to change the instance type for managed nodegroups. We should make the the instanceType immutable along with the spec.name and spec.clusterName.

cc @michaelhtm @a-hilaly

[fspaniol]

Just to give some context. We are migrating our nodegroup declaration from CDK to Kubernetes leveraged by Flux.

In CDK we could simply change the nodegroup name and the instance type and it was able to create a new one and delete the previous. Can't something similar be used here? It would make this so much easier than create a new one and delete the previous one.

Internally, to be able keep track of spec changes, we leverage webhooks and take according action when something like this changes, you could compare the previous name with the current one and either block the request or delete the previous nodegroup and let it create a new one fresh?

[a-hilaly]

In CDK we could simply change the nodegroup name and the instance type and it was able to create a new one and delete the previous

@fspaniol the reason CDK/CF can do that is because of how AWS CF Stacks are implemented. It allows AWS systems to detect resource name changes and react accordingly. In k8s land, things are different - the source of truth is in the api server, and users can modify it directly. yes in theory a controller could catch the old and new name, similar to how webhooks work... but if for some reason the controller misses the event or wasn't running at that time, that info is lost and impossible to recover. k8s recommends controllers do a Get to grab a fresh snapshot of the resource before acting on anything. In fact, we use controller-runtime which doesn't even expose old and new snapshots to controllers (AFAIK).

Can't something similar be used here? It would make this so much easier than create a new one and delete the previous one.

As for the delete/create when instanceType changes, I think that's definitely doable. We could aim for it :)

[fspaniol]

In CDK we could simply change the nodegroup name and the instance type and it was able to create a new one and delete the previous

@fspaniol the reason CDK/CF can do that is because of how AWS CF Stacks are implemented. It allows AWS systems to detect resource name changes and react accordingly. In k8s land, things are different - the source of truth is in the api server, and users can modify it directly. yes in theory a controller could catch the old and new name, similar to how webhooks work... but if for some reason the controller misses the event or wasn't running at that time, that info is lost and impossible to recover. k8s recommends controllers do a Get to grab a fresh snapshot of the resource before acting on anything. In fact, we use controller-runtime which doesn't even expose old and new snapshots to controllers (AFAIK).

Can't something similar be used here? It would make this so much easier than create a new one and delete the previous one.

As for the delete/create when instanceType changes, I think that's definitely doable. We could aim for it :)

Thanks for the reply :)

I failed to mention that my example was also using controller-runtime 😆 . We do something (maybe hacky?) like adding an annotation to the resource with whatever we want persisted. Then on updates, we compare the spec.name to the annotation, and then act accordingly. On fresh resources it would simply populate the annotation and create what's needed, the magic here would have to be done when the annotation is already populated and it's different than spec.name.

Of course there are other ways to achieve this, this is simply how we are doing it :)

For now we'll simply create a new nodegroup object, apply it and then remove the older one