Do not perform explicit cluster deletion in prod e2e

[tsatam]

Which issue this PR addresses:

Fixes no Jira yet

What this PR does / why we need it:

Skips explicit cluster deletion when CI=true and RP_MODE!=development, and let deletion of the cluster's resource group take care of cluster deletion for us. This is required in production e2e contexts to avoid a race condition between us and ARM performing cluster deletion simultaneously and causing one of the requests to fail.

Test plan for issue:

• E2E should still work on this PR

Is there any documentation that needs to be updated for this PR?

No

How do you know this will function as expected in production?

• E2E should still work in releases

[tsatam]

/azp run ci,e2e

[tsatam]

/azp run ci,e2e

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[niontive]

Why doesn't deleting the resource group only work in local development mode?

[tsatam]

Discussed during sync call: The implementation here, with the two boolean flags (CI and localDevelopmentMode) leads to confusion on which flags are set and what behavior we expect for each permutation. We should follow-up this change with a refactor to make it more explicit and clear what steps are being performed in what contexts, even if it does lead to code duplication.

[SudoBrendan]

LGTM - nit: would it be better to use %w vs %v in all these cases? idk if it matters in a logging context or not?

[tsatam]

nit: would it be better to use %w vs %v in all these cases? idk if it matters in a logging context or not?

%w only has special behavior if within a fmt.Errorf call - it will implement the Unwrap method on the returned error that returns the error(s) included in any %ws specified, otherwise it behaves identically to %v. https://pkg.go.dev/fmt#Errorf

Perhaps a better change here is to wrap the errors themselves using fmt.Errorf and both log and return the wrapped error, but without being 100% certain what downstream consumers of the error are expecting I did not want to make that change.

[tsatam]

I pushed up an opinionated refactor to address some legibility/intent concerns - we can back this change out or defer it to a follow-up PR if desired.

[tsatam]

/azp run ci,e2e

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[UlrichSchlueter]

Might be good to switch the Prod E2E case to use the deleteCluster call again in a future PR. We don't test our cluster deletion code the way it is.
Approving anyway to unblock the next release.