🐛 fix upgrade of managed node groups using custom AMIs

[fad3t]

Hello all,

What type of PR is this?

/kind bug

What this PR does / why we need it:

When using a custom AMI with a launch template, node group upgrades get stuck with the following error message:

E0227 15:07:47.415572       1 controller.go:329] "Reconciler error" err=<
	failed to reconcile machine pool for AWSManagedMachinePool dev/pocg1eks80-mmp-infra: failed to reconcile nodegroup version: failed to update EKS nodegroup: InvalidParameterException: Launch template details can't be null for Custom ami type node group
	{
	  RespMetadata: {
	    StatusCode: 400,
	    RequestID: "e8b04ccf-56aa-4bab-b313-0f4b25137e5c"
	  },
	  Message_: "Launch template details can't be null for Custom ami type node group"
	}
 > controller="awsmanagedmachinepool" controllerGroup="infrastructure.cluster.x-k8s.io" controllerKind="AWSManagedMachinePool" AWSManagedMachinePool="dev/pocg1eks80-mmp-infra" namespace="dev" name="pocg1eks80-mmp-infra" reconcileID="441e08da-f7ba-4fde-af7b-e3eb250aae02"

This is because the specVersion is checked first in the switch/case, causing the UpdateNodegroupVersion function to be called without the launch template details.

cluster-api-provider-aws/pkg/cloud/services/eks/nodegroup.go

Lines 350 to 365 in 91941d9

	switch {
	case specVersion != nil && ngVersion.LessThan(specVersion):
	// NOTE: you can only upgrade increments of minor versions. If you want to upgrade 1.14 to 1.16 we
	// need to go 1.14-> 1.15 and then 1.15 -> 1.16.
	input.Version = aws.String(versionToEKS(ngVersion.WithMinor(ngVersion.Minor() + 1)))
	updateMsg = fmt.Sprintf("to version %s", *input.Version)
	case specAMI != nil && *specAMI != ngAMI:
	input.ReleaseVersion = specAMI
	updateMsg = fmt.Sprintf("to AMI version %s", *input.ReleaseVersion)
	case statusLaunchTemplateVersion != nil && *statusLaunchTemplateVersion != *ngLaunchTemplateVersion:
	input.LaunchTemplate = &eks.LaunchTemplateSpecification{
	Id: s.scope.ManagedMachinePool.Status.LaunchTemplateID,
	Version: statusLaunchTemplateVersion,
	}
	updateMsg = fmt.Sprintf("to launch template version %s", *statusLaunchTemplateVersion)
	}

Changing the order of the switch/case makes it possible to upgrade a node group using a custom launch template.

This would fix #4327

Special notes for your reviewer:

Checklist:

• squashed commits
• includes documentation
• includes emojis
• adds unit tests
• adds or updates e2e tests

Release notes:

fix upgrade of managed node groups using custom AMIs

[k8s-ci-robot]

Hi @fad3t. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[mloiseleur]

/ok-to-test

[Ankitasw]

/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-eks

[Ankitasw]

/retest

[Ankitasw]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Ankitasw

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Ankitasw]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment