🌱 Try to fix test flake in which secret is not yet available

[AndiDog]

What type of PR is this?

/kind flake

What this PR does / why we need it:

There seems to be one flake that keeps reappearing:

=== RUN   TestAWSMachinePoolReconciler/Reconciling_an_AWSMachinePool/ReconcileLaunchTemplate_not_mocked/launch_template_and_ASG_created_from_zero,_then_bootstrap_config_reference_changes
E0624 07:12:01.962501   46441 awsmachinepool_controller.go:341] "failed to reconcile launch template" err="failed to retrieve bootstrap data secret bootstrap-data-new for AWSMachinePool default/test: Secret \"bootstrap-data-new\" not found"
    awsmachinepool_controller_test.go:966: 
        Expected success, but got an error:
            <*errors.withStack | 0xc002129788>: 
            failed to retrieve bootstrap data secret bootstrap-data-new for AWSMachinePool default/test: Secret "bootstrap-data-new" not found
            {
                error: <*errors.withMessage | 0xc0025847c0>{
                    cause: <*errors.StatusError | 0xc0021214a0>{
                        ErrStatus: {
                            TypeMeta: {Kind: "", APIVersion: ""},
                            ListMeta: {
                                SelfLink: "",
                                ResourceVersion: "",
                                Continue: "",
                                RemainingItemCount: nil,
                            },
                            Status: "Failure",
                            Message: "Secret \"bootstrap-data-new\" not found",
                            Reason: "NotFound",
                            Details: {
                                Name: "bootstrap-data-new",
                                Group: "",
                                Kind: "Secret",
                                UID: "",
                                Causes: nil,
                                RetryAfterSeconds: 0,
                            },
                            Code: 404,
                        },
                    },
                    msg: "failed to retrieve bootstrap data secret bootstrap-data-new for AWSMachinePool default/test",
                },
                stack: [0x24c366e, 0x2547151, 0x26e6bbe, 0x26fb374, 0x556854, 0x480101],
            }

This change tries to work around by waiting for the Secret to be available.

Special notes for your reviewer:

I couldn't reproduce the flake locally, so this is a guess.

Checklist:

• squashed commits
• includes documentation
• includes emoji in title
• adds unit tests
• adds or updates e2e tests

Release note:

NONE

[AndiDog]

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

[dlipovetsky]

/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

[dlipovetsky]

I can't look right now, but I'm wondering if this is a cached client? 🤔

That would explain the failure to get the Secret just after it has been created.

We should use a "direct" or "uncached" client in tests.

kubernetes-sigs/controller-runtime#343 (comment)

[AndiDog]

Hmm indeed we use the manager client

	return &TestEnvironment{
		Manager: mgr,
		Client:  mgr.GetClient(), // <----------
		Config:  mgr.GetConfig(),
		env:     t.env,
	}, nil

which may be a cache:

GetClient returns a client configured with the Config. This client may not be a fully "direct" client -- it may read from a cache, for instance.

But we're testing the reconciler here which uses its caching client. Should we override that to a direct client in tests? I'm not even sure that's possible (we'd need a SetClient or some option to avoid caching?).

[AndiDog]

Maybe we want to copy-paste CAPI's CreateAndWait to denote the intent

[dlipovetsky]

But we're testing the reconciler here which uses its caching client.

Yes, but in production, if the Secret is unavailable, the request would be requeued. That doesn't happen under test: we fail immediately. So one way to ensure that the reconcile is ready to run is to use a direct client.

I think switching to a direct client is something we want to do across all tests, so that's out of scope for this PR.

Maybe we want to copy-paste CAPI's CreateAndWait to denote the intent

That's a good idea.

[AndiDog]

/hold

for the direct/caching client question

[damdo]

@AndiDog is this fixing any of these by any chance? E2E Test Stabilization & Improvement

[AndiDog]

@AndiDog is this fixing any of these by any chance? E2E Test Stabilization & Improvement

I didn't find it listed there, and it's no E2E test

[damdo]

Got it thanks