🐛 subnets: use "owned" k8s tag value for managed subnets.

[r4f4]

The "shared" value will be used for unmanaged subnets.

What type of PR is this?
/kind bug

What this PR does / why we need it:

Tag managed subnets with "kubernetes.io/cluster/: owned" tag.

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 #5050

Special notes for your reviewer:

Checklist:

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

Release note:

Fix managed subnet tagging with "owned" value for "kubernetes.io/cluster/<clusterID>" tag.

[k8s-ci-robot]

Hi @r4f4. 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-sigs/prow repository.

[patrickdillon]

LGTM

[r4f4]

No regressions in the Openshift e2e tests: openshift/installer#8730

• BYO VPC: the cluster spec subnets have the kubernetes.io/cluster/<clusterID>: shared tag
• CAPA-created VPC: the cluster spec subnets have the kubernetes.io/cluster/<clusterID>: owned tag

[AndiDog]

/ok-to-test

[AndiDog]

Quite a major tagging change, but I agree it was incorrect to set shared for managed subnets, given the meaning of the tags:

const (
	// ResourceLifecycleOwned is the value we use when tagging resources to indicate
	// that the resource is considered owned and managed by the cluster,
	// and in particular that the lifecycle is tied to the lifecycle of the cluster.
	ResourceLifecycleOwned = ResourceLifecycle("owned")

	// ResourceLifecycleShared is the value we use when tagging resources to indicate
	// that the resource is shared between multiple clusters, and should not be destroyed
	// if the cluster is destroyed.
	ResourceLifecycleShared = ResourceLifecycle("shared")

I'm not sure if this has an impact somewhere, except for custom user code that may rely on the values. But then again, we already had inconsistent tags and that would now be fixed:

# Before the change
sigs.k8s.io/cluster-api-provider-aws/cluster/mycluster=owned
kubernetes.io/cluster/mycluster=shared

So, if the tests pass:

/lgtm

[r4f4]

/hold

It seems I missed some unit tests.

[r4f4]

Weird, the test is passing locally:

--- PASS: TestAWSClusterReconcilerReconcile (0.21s)
    --- PASS: TestAWSClusterReconcilerReconcile/Should_fail_Reconcile_if_owner_cluster_not_found (0.13s)
    --- PASS: TestAWSClusterReconcilerReconcile/Should_not_reconcile_if_owner_reference_is_not_set (0.02s)
    --- PASS: TestAWSClusterReconcilerReconcile/Should_not_Reconcile_if_cluster_is_paused (0.05s)
    --- PASS: TestAWSClusterReconcilerReconcile/Should_Reconcile_successfully_if_no_AWSCluster_found (0.00s)

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

[r4f4]

/test ?

[k8s-ci-robot]

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

• /test pull-cluster-api-provider-aws-build
• /test pull-cluster-api-provider-aws-build-docker
• /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-build-docker
• 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-sigs/prow repository.

[r4f4]

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

[r4f4]

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

[r4f4]

pull-cluster-api-provider-aws-e2e: different failure this time, seems to be a fluke.

/hold cancel

[r4f4]

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

[AndiDog]

/lgtm

[nrb]

/retest

timeout

[nrb]

/retest

This pass isn't strictly required, but I would love to see it not timeout.

[r4f4]

@nrb all the triggered tests have passed.

[nrb]

@r4f4 🤦 that's what I get for only reading email.

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nrb

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

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