🐛 There is no need to check instance refresh if ASG is not found

[fiunchinho]

What type of PR is this?

/kind bug

What this PR does / why we need it:

On this PR we tried to fix #4655 by ignoring the "not found" error when describing instance refreshes resources on AWS. But the AWS API returns a 400 error with code ValidationError when trying to describe the instance refreshes for a non existing AutoScalingGroup. Since ValidationError sounds super generic, instead of ignoring that error, on this PR I'm moving the call to find the AutoScalingGroup before defining the function that will check if we can do an instance refresh. That way, if the AutoScalingGroup is nil, we know we can skip checking. Also, we still want to update the LaunchTemplate when the AutoScalingGroup does not exist, because an error in the LaunchTemplate may be the root cause why the ASG is not created yet.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #4655

Special notes for your reviewer:

Unfortunately, there is no easy way to test this change, because ReconcileLaunchTemplate, which is the one receiving the anonymous function that we want to modify here is mocked.

Checklist:

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

Release note:

Skip instance refresh attempt if ASG does not yet exist

[fiunchinho]

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

[fiunchinho]

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

[AndiDog]

Mostly LGTM – one minor comment.

Can you please fill the release note on the PR description? The release note, commit and PR title could read something like "Skip instance refresh attempt if ASG does not yet exist".

[AndiDog]

For the record, this is removed because AWS returns ValidationError instead of a clear "not found" code, right?

[fiunchinho]

Exactly. It never returns an error that would match IsNotFound() so it's useless to have the check here.

[fiunchinho]

Mostly LGTM – one minor comment.

Can you please fill the release note on the PR description? The release note, commit and PR title could read something like "Skip instance refresh attempt if ASG does not yet exist".

I left the release note intentionally because I added it here, but I guess it's better to have a more specific one.

[fiunchinho]

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

[AndiDog]

/lgtm

[fiunchinho]

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

[fiunchinho]

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

[richardcase]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: richardcase

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 [richardcase]

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

[richardcase]

/cherrypick release-2.3

[k8s-infra-cherrypick-robot]

@richardcase: #4662 failed to apply on top of branch "release-2.3":

Applying: There is no need to check instance refresh if ASG is not found
Using index info to reconstruct a base tree...
M	exp/controllers/awsmachinepool_controller.go
M	pkg/cloud/services/autoscaling/autoscalinggroup.go
M	pkg/cloud/services/autoscaling/autoscalinggroup_test.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/cloud/services/autoscaling/autoscalinggroup_test.go
CONFLICT (content): Merge conflict in pkg/cloud/services/autoscaling/autoscalinggroup_test.go
Auto-merging exp/controllers/awsmachinepool_controller.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 There is no need to check instance refresh if ASG is not found
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-2.3

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.