🐛 Update awsmachine providerID and instanceID immediately after ec2:RunInstances is called

[mjlshen]

What type of PR is this?

/kind bug

What this PR does / why we need it:
This mitigates issues caused by falling back to tag-based searching for EC2 instances in case future AWS calls fail within CreateInstance(), such as attaching ENIs to security groups or tagging ENIs.

cluster-api-provider-aws/controllers/awsmachine_controller.go

Lines 421 to 448 in c9b9276

	func (r *AWSMachineReconciler) findInstance(scope *scope.MachineScope, ec2svc services.EC2Interface) (*infrav1.Instance, error) {
	var instance *infrav1.Instance
	// Parse the ProviderID.
	//nolint:staticcheck
	// Usage of noderefutil pkg would be removed in a future release.
	pid, err := noderefutil.NewProviderID(scope.GetProviderID())
	if err != nil {
	//nolint:staticcheck
	// Usage of noderefutil pkg would be removed in a future release.
	if !errors.Is(err, noderefutil.ErrEmptyProviderID) {
	return nil, errors.Wrapf(err, "failed to parse Spec.ProviderID")
	}
	// If the ProviderID is empty, try to query the instance using tags.
	// If an instance cannot be found, GetRunningInstanceByTags returns empty instance with nil error.
	instance, err = ec2svc.GetRunningInstanceByTags(scope)
	if err != nil {
	return nil, errors.Wrapf(err, "failed to query AWSMachine instance by tags")
	}
	} else {
	// If the ProviderID is populated, describe the instance using the ID.
	// InstanceIfExists() returns error (ErrInstanceNotFoundByID or ErrDescribeInstance) if the instance could not be found.
	//nolint:staticcheck
	instance, err = ec2svc.InstanceIfExists(pointer.String(pid.ID()))
	if err != nil {
	return nil, err
	}
	}

Which issue(s) this PR fixes:
Related to #4629

Special notes for your reviewer:

This PR focuses on mitigating the impact when the controller fails to tag ENIs after creating EC2 instances. Another PR to make the fallback tag-based searching more robust is in the works in #4630

Checklist:

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

Release note:

Update AWSMachine providerID and instanceID earlier to minimize scenarios where tag-based searching is needed

[enxebre]

/lgtm

[mjlshen]

/assign killianmuldoon

[AndiDog]

Nice improvement!

/lgtm

[richardcase]

/test ?

[k8s-ci-robot]

@richardcase: The following commands are available to trigger required jobs:

• /test pull-cluster-api-provider-aws-build
• /test pull-cluster-api-provider-aws-test
• /test pull-cluster-api-provider-aws-verify

The following commands are available to trigger optional jobs:

• /test pull-cluster-api-provider-aws-apidiff-main
• /test pull-cluster-api-provider-aws-e2e
• /test pull-cluster-api-provider-aws-e2e-blocking
• /test pull-cluster-api-provider-aws-e2e-clusterclass
• /test pull-cluster-api-provider-aws-e2e-conformance
• /test pull-cluster-api-provider-aws-e2e-conformance-with-ci-artifacts
• /test pull-cluster-api-provider-aws-e2e-eks
• /test pull-cluster-api-provider-aws-e2e-eks-gc
• /test pull-cluster-api-provider-aws-e2e-eks-testing

Use /test all to run the following jobs that were automatically triggered:

• pull-cluster-api-provider-aws-apidiff-main
• pull-cluster-api-provider-aws-build
• pull-cluster-api-provider-aws-test
• pull-cluster-api-provider-aws-verify

In response to this:

/test ?

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.

[richardcase]

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

[dlipovetsky]

/lgtm

Can we please keep #4629 open after merging this PR? I think we haven't addressed the root cause there, but this is a welcome improvement nevertheless!

[mjlshen]

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

[dlipovetsky]

We discussed this at today's office hours.

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dlipovetsky

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

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

[damdo]

@richardcase do you think we could add this to the upcoming 2.4.0 and 2.3.2 milestones?