Skip to content

OCPBUGS-88035: Set cluster ownership tag on AWSCluster#589

Open
matzew wants to merge 1 commit into
openshift:mainfrom
matzew:OCPBUGS-88035-aws-ownership-tags
Open

OCPBUGS-88035: Set cluster ownership tag on AWSCluster#589
matzew wants to merge 1 commit into
openshift:mainfrom
matzew:OCPBUGS-88035-aws-ownership-tags

Conversation

@matzew

@matzew matzew commented Jun 10, 2026

Copy link
Copy Markdown
Member

Set the kubernetes.io/cluster/=owned tag on the AWSCluster object via AdditionalTags so that CAPI-created AWS resources are visible to the installer's destroy logic. Also propagate user-defined resource tags from Infrastructure.Status.PlatformStatus.AWS.ResourceTags to match MAPI behaviour.

Without this tag, openshift-install destroy cluster cannot identify CAPI-created resources (VMs, volumes, etc.) and they are leaked.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jun 10, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@matzew: This pull request references Jira Issue OCPBUGS-88035, which is invalid:

  • expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Set the kubernetes.io/cluster/=owned tag on the AWSCluster object via AdditionalTags so that CAPI-created AWS resources are visible to the installer's destroy logic. Also propagate user-defined resource tags from Infrastructure.Status.PlatformStatus.AWS.ResourceTags to match MAPI behaviour.

Without this tag, openshift-install destroy cluster cannot identify CAPI-created resources (VMs, volumes, etc.) and they are leaked.

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 openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@matzew, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 24 minutes and 30 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: f7fb68f7-f189-4b8e-a2a9-913501ec8989

📥 Commits

Reviewing files that changed from the base of the PR and between 05c113e and b9d6f3a.

📒 Files selected for processing (1)
  • pkg/controllers/infracluster/aws.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci Bot requested review from damdo and mdbooth June 10, 2026 15:32
@openshift-ci

openshift-ci Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign mdbooth for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

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

Set the kubernetes.io/cluster/<infraID>=owned tag on the AWSCluster
object via AdditionalTags so that CAPI-created AWS resources are
visible to the installer's destroy logic. Also propagate user-defined
resource tags from Infrastructure.Status.PlatformStatus.AWS.ResourceTags
to match MAPI behaviour.

Without this tag, openshift-install destroy cluster cannot identify
CAPI-created resources (VMs, volumes, etc.) and they are leaked.

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>
@matzew matzew force-pushed the OCPBUGS-88035-aws-ownership-tags branch from 8c2bae5 to b9d6f3a Compare June 10, 2026 16:02
@openshift-ci

openshift-ci Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

@matzew: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@mdbooth

mdbooth commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

/hold

Discussed this with @matzew . The missing (and unfortunately more difficult) piece here is the testing. I had it in mind that we could write something that would trigger in aws-deprovision-verification if it was not cleaned up, but this presents a chicken and egg problem: the deprovision verification step will not find an incorrectly tagged resource so it won't fail. I think the pragmatic solution here is to add an explicit e2e test that the created ec2 VM has the same tag the installer is looking for during destroy.

Incidentally, it's possible that the tag is already added implicitly by CAPA. It looks like CAPA adds a tag kubernetes.io/cluster/<KubernetesClusterName> = owned. KubernetesClusterName is the name of the core capi cluster which owns the machine, that the name of the core capi cluster should match the infrastructure name. Again, a test would verify this.

Therefore I think we should write the described test without adding the tag explicitly. Ideally the test would simply be an additional check in an existing test. Any test which already creates an AWSMachine should be sufficient.

If this passes, we're done. If not, we need to add the tag explicitly as done here.

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 10, 2026
@mdbooth

mdbooth commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

test: #594

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants