OCPBUGS-18986: Tag aws instance profiles. #7510

barbacbd · 2023-09-20T19:19:31Z

** Instance profiles are not always cleaned up correctly. Tags are available for these resources. Tag the resources and let the destroy functions find the resources by tag to be removed.

openshift-ci-robot · 2023-09-20T19:19:36Z

@barbacbd: This pull request references Jira Issue OCPBUGS-18986, which is invalid:

expected the bug to target the "4.15.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.

In response to this:

** Instance profiles are not always cleaned up correctly. Tags are available for these resources. Tag the resources and let the destroy functions find the resources by tag to be removed.

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.

barbacbd · 2023-09-20T19:20:05Z

/jira refresh

openshift-ci-robot · 2023-09-20T19:20:11Z

@barbacbd: This pull request references Jira Issue OCPBUGS-18986, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.15.0) matches configured target version for branch (4.15.0)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @gpei

In response to this:

/jira refresh

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.

r4f4 · 2023-09-21T09:10:25Z

e2e-aws-ovn:
on cluster create:

time="2023-09-20T21:22:14Z" level=debug msg="  + resource \"aws_iam_instance_profile\" \"worker\" {"
time="2023-09-20T21:22:14Z" level=debug msg="      + arn         = (known after apply)"
time="2023-09-20T21:22:14Z" level=debug msg="      + create_date = (known after apply)"
time="2023-09-20T21:22:14Z" level=debug msg="      + id          = (known after apply)"
time="2023-09-20T21:22:14Z" level=debug msg="      + name        = \"ci-op-q9vf6jjl-3266f-stcgj-worker-profile\""
time="2023-09-20T21:22:14Z" level=debug msg="      + name_prefix = (known after apply)"
time="2023-09-20T21:22:14Z" level=debug msg="      + path        = \"/\""
time="2023-09-20T21:22:14Z" level=debug msg="      + role        = \"ci-op-q9vf6jjl-3266f-stcgj-worker-role\""
time="2023-09-20T21:22:14Z" level=debug msg="      + tags        = {"
time="2023-09-20T21:22:14Z" level=debug msg="          + \"Name\"                                             = \"ci-op-q9vf6jjl-3266f-stcgj-worker-profile\""
time="2023-09-20T21:22:14Z" level=debug msg="          + \"expirationDate\"                                   = \"2023-09-21T05:20+0000\""
time="2023-09-20T21:22:14Z" level=debug msg="          + \"keyA\"                                             = \"valueA\""
time="2023-09-20T21:22:14Z" level=debug msg="          + \"keyB\"                                             = \"valueB\""
time="2023-09-20T21:22:14Z" level=debug msg="          + \"keyC\"                                             = \"valueC\""
time="2023-09-20T21:22:14Z" level=debug msg="          + \"kubernetes.io/cluster/ci-op-q9vf6jjl-3266f-stcgj\" = \"owned\""
time="2023-09-20T21:22:14Z" level=debug msg="        }"
time="2023-09-20T21:22:14Z" level=debug msg="      + tags_all    = {"
time="2023-09-20T21:22:14Z" level=debug msg="          + \"Name\"                                             = \"ci-op-q9vf6jjl-3266f-stcgj-worker-profile\""
time="2023-09-20T21:22:14Z" level=debug msg="          + \"expirationDate\"                                   = \"2023-09-21T05:20+0000\""
time="2023-09-20T21:22:14Z" level=debug msg="          + \"keyA\"                                             = \"valueA\""
time="2023-09-20T21:22:14Z" level=debug msg="          + \"keyB\"                                             = \"valueB\""
time="2023-09-20T21:22:14Z" level=debug msg="          + \"keyC\"                                             = \"valueC\""
time="2023-09-20T21:22:14Z" level=debug msg="          + \"kubernetes.io/cluster/ci-op-q9vf6jjl-3266f-stcgj\" = \"owned\""
time="2023-09-20T21:22:14Z" level=debug msg="        }"
time="2023-09-20T21:22:14Z" level=debug msg="      + unique_id   = (known after apply)"
time="2023-09-20T21:22:14Z" level=debug msg="    }"

on cluster destroy:

time="2023-09-20T22:54:14Z" level=info msg=Deleted InstanceProfileName=ci-op-q9vf6jjl-3266f-stcgj-worker-profile arn="arn:aws:iam::460538899914:instance-profile/ci-op-q9vf6jjl-3266f-stcgj-worker-profile" id=i-00aa34b45ed683e2c resourceType=instance`

/lgtm

r4f4 · 2023-09-21T11:20:56Z

@barbacbd do you know in which version of the provider the instance profile tagging capability was added? We probably want to backport this fix to all supported OCP versions.

barbacbd · 2023-09-25T14:44:23Z

@barbacbd do you know in which version of the provider the instance profile tagging capability was added? We probably want to backport this fix to all supported OCP versions.

I don't but I will try to find this out so we can backport

barbacbd

The installer v4.11 uses the provider v4.5.0. This well beyond the minimum supported version for instance profile tags in the provider. We can backport to 4.11 if we want to. @r4f4 @sadasu

r4f4 · 2023-09-25T16:03:24Z

The installer v4.11 uses the provider v4.5.0. This well beyond the minimum supported version for instance profile tags in the provider. We can backport to 4.11 if we want to. @r4f4 @sadasu

Then I think we should do it.

sadasu · 2023-09-25T16:52:20Z

Do we want to rename "findUntaggableResources" to "findInstanceProfiles" after this fix?

r4f4 · 2023-09-25T17:00:27Z

Do we want to rename "findUntaggableResources" to "findInstanceProfiles" after this fix?

I'd say "no". findUntaggableResources is for resources that cannot be tagged, which in this case happened to be instance profiles but could include other types in the future.

r4f4 · 2023-09-25T17:06:35Z

That being said, we could probably remove the profile retrieval from findUntaggableResources if we also include tagging for the bootstrap profile. That would result in an empty findUntaggableResources which we could consider deleting and re-adding once we bump into another untaggable resource. Or we could just leave it as-is and the profiles will be deleted either by the tag or by their name.

sadasu · 2023-09-25T17:32:36Z

Should we add the same tag here https://github.com/openshift/installer/blob/master/data/data/aws/bootstrap/main.tf#L83?

r4f4 · 2023-09-25T18:24:29Z

Should we add the same tag here https://github.com/openshift/installer/blob/master/data/data/aws/bootstrap/main.tf#L83?

Yeah I think so. WDYT @barbacbd ?

** Instance profiles are not always cleaned up correctly. Tags are available for these resources. Tag the resources and let the destroy functions find the resources by tag to be removed.

r4f4

/lgtm

sadasu · 2023-09-26T13:27:19Z

/approve

openshift-ci · 2023-09-26T13:31:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sadasu

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:

~~data/data/aws/OWNERS~~ [sadasu]
~~pkg/destroy/aws/OWNERS~~ [sadasu]

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

openshift-ci · 2023-09-26T16:51:19Z

@barbacbd: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-ovn-shared-vpc	`9b83b98`	link	false	`/test e2e-aws-ovn-shared-vpc`
ci/prow/e2e-aws-ovn-shared-vpc-localzones	`9b83b98`	link	false	`/test e2e-aws-ovn-shared-vpc-localzones`
ci/prow/e2e-aws-ovn-localzones	`9b83b98`	link	false	`/test e2e-aws-ovn-localzones`
ci/prow/okd-e2e-aws-ovn	`9b83b98`	link	false	`/test okd-e2e-aws-ovn`
ci/prow/okd-scos-e2e-aws-ovn	`9b83b98`	link	false	`/test okd-scos-e2e-aws-ovn`

Full PR test history. Your PR dashboard.

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

openshift-ci-robot · 2023-09-26T22:16:42Z

@barbacbd: Jira Issue OCPBUGS-18986: All pull requests linked via external trackers have merged:

openshift/installer#7510

Jira Issue OCPBUGS-18986 has been moved to the MODIFIED state.

In response to this:

** Instance profiles are not always cleaned up correctly. Tags are available for these resources. Tag the resources and let the destroy functions find the resources by tag to be removed.

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.

openshift-merge-robot · 2023-09-27T15:55:07Z

Fix included in accepted release 4.15.0-0.nightly-2023-09-27-073353

** Partially overrides openshift#7510 to ensure that instance profiles that are tagged as shared do not get deleted. This fix will be combined with openshift#7510 to ensure that the solution can be backported.

openshift-merge-robot · 2023-10-31T09:41:09Z

Fix included in accepted release 4.15.0-0.nightly-2023-10-31-054858

requirement Since openshift#7510, IPI installs fail if the permission is missing: ``` level=error msg=Error: creating IAM Instance Profile (ci-op-4hw2rz1v-49c30-zt9vx-worker-profile): AccessDenied: User: arn:aws:iam::301721915996:user/ci-op-4hw2rz1v-49c30-minimal-perm is not authorized to perform: iam:TagInstanceProfile on resource: arn:aws:iam::301721915996:instance-profile/ci-op-4hw2rz1v-49c30-zt9vx-worker-profile because no identity-based policy allows the iam:TagInstanceProfile action ```

…ment Since openshift#7510, IPI installs fail if the permission is missing: ``` level=error msg=Error: creating IAM Instance Profile (ci-op-4hw2rz1v-49c30-zt9vx-worker-profile): AccessDenied: User: arn:aws:iam::301721915996:user/ci-op-4hw2rz1v-49c30-minimal-perm is not authorized to perform: iam:TagInstanceProfile on resource: arn:aws:iam::301721915996:instance-profile/ci-op-4hw2rz1v-49c30-zt9vx-worker-profile because no identity-based policy allows the iam:TagInstanceProfile action ```

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 Sep 20, 2023

openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Sep 20, 2023

openshift-ci-robot removed the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Sep 20, 2023

openshift-ci bot requested review from gpei, mtulio and patrickdillon September 20, 2023 19:20

openshift-ci bot assigned r4f4 Sep 21, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 21, 2023

barbacbd commented Sep 25, 2023

View reviewed changes

OCPBUGS-18986: Tag aws instance profiles.

9b83b98

** Instance profiles are not always cleaned up correctly. Tags are available for these resources. Tag the resources and let the destroy functions find the resources by tag to be removed.

barbacbd force-pushed the OCPBUGS-18986 branch from a1f758a to 9b83b98 Compare September 25, 2023 18:52

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 25, 2023

r4f4 reviewed Sep 25, 2023

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 25, 2023

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 26, 2023

openshift-merge-robot merged commit 4ce62d1 into openshift:master Sep 26, 2023
24 of 29 checks passed

barbacbd mentioned this pull request Sep 28, 2023

OCPBUGS-18986: Skip the deletion of instance profiles marked shared #7537

Merged

r4f4 mentioned this pull request Dec 15, 2023

OCPBUGS-25440: ic: aws: add iam:TagInstanceProfile permission requirement #7843

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OCPBUGS-18986: Tag aws instance profiles. #7510

OCPBUGS-18986: Tag aws instance profiles. #7510

barbacbd commented Sep 20, 2023

openshift-ci-robot commented Sep 20, 2023

barbacbd commented Sep 20, 2023

openshift-ci-robot commented Sep 20, 2023

r4f4 commented Sep 21, 2023

r4f4 commented Sep 21, 2023

barbacbd commented Sep 25, 2023

barbacbd left a comment •

edited

r4f4 commented Sep 25, 2023

sadasu commented Sep 25, 2023

r4f4 commented Sep 25, 2023

r4f4 commented Sep 25, 2023 •

edited

sadasu commented Sep 25, 2023

r4f4 commented Sep 25, 2023

r4f4 left a comment

sadasu commented Sep 26, 2023

openshift-ci bot commented Sep 26, 2023

openshift-ci bot commented Sep 26, 2023

openshift-ci-robot commented Sep 26, 2023

openshift-merge-robot commented Sep 27, 2023

openshift-merge-robot commented Oct 31, 2023

OCPBUGS-18986: Tag aws instance profiles. #7510

OCPBUGS-18986: Tag aws instance profiles. #7510

Conversation

barbacbd commented Sep 20, 2023

openshift-ci-robot commented Sep 20, 2023

barbacbd commented Sep 20, 2023

openshift-ci-robot commented Sep 20, 2023

r4f4 commented Sep 21, 2023

r4f4 commented Sep 21, 2023

barbacbd commented Sep 25, 2023

barbacbd left a comment • edited

Choose a reason for hiding this comment

r4f4 commented Sep 25, 2023

sadasu commented Sep 25, 2023

r4f4 commented Sep 25, 2023

r4f4 commented Sep 25, 2023 • edited

sadasu commented Sep 25, 2023

r4f4 commented Sep 25, 2023

r4f4 left a comment

Choose a reason for hiding this comment

sadasu commented Sep 26, 2023

openshift-ci bot commented Sep 26, 2023

openshift-ci bot commented Sep 26, 2023

openshift-ci-robot commented Sep 26, 2023

openshift-merge-robot commented Sep 27, 2023

openshift-merge-robot commented Oct 31, 2023

barbacbd left a comment •

edited

r4f4 commented Sep 25, 2023 •

edited