[Release 4.14] OCPBUGS-24556: Fix a bug on deletion of a hostedcluster by nunnatsa · Pull Request #3290 · openshift/hypershift

nunnatsa · 2023-12-07T09:13:58Z

What this PR does / why we need it

A user destroying a HostedCluster can cause the HostedCluster to hang indefinitely if the destroy command times out during execution.

This is due to the hcp cli placing a finalizer on the HostedCluster during deletion which the cli tool later removes after waiting for some clean up actions to occur. If a user cancels the hcp destroy cluster command (or the command times out) while the cli is waiting for cleanup, then the HostedCluster will hang indefinitely with a DeletionTimestamp != nil.

This PR only performs the additional cleaning logic, including the setting and removing the special finalizer, for platform that actually creates additional resources. Also, this PR sets the hostedCluster as the owner for these secrets, so they will be deleted with the HostedCluster.

This PR is a manual cherry-pick of #3234

Which issue(s) this PR fixes

Fixes #OCPBUGS-24556

Checklist

Subject and description added to both, commit and PR.
Relevant issues have been referenced.
This change includes docs.
This change includes unit tests.

openshift-ci-robot · 2023-12-07T09:14:04Z

@nunnatsa: This pull request references Jira Issue OCPBUGS-24556, which is invalid:

expected dependent Jira Issue OCPBUGS-23126 to be in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA), but it is ON_QA instead

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:

What this PR does / why we need it**:

A user destroying a HostedCluster can cause the HostedCluster to hang indefinitely if the destroy command times out during execution.

This is due to the hcp cli placing a finalizer on the HostedCluster during deletion which the cli tool later removes after waiting for some clean up actions to occur. If a user cancels the hcp destroy cluster command (or the command times out) while the cli is waiting for cleanup, then the HostedCluster will hang indefinitely with a DeletionTimestamp != nil.

This PR only performs the additional cleaning logic, including the setting and removing the special finalizer, for platform that actually creates additional resources. Also, this PR sets the hostedCluster as the owner for these secrets, so they will be deleted with the HostedCluster.

This PR is a manual cherry-pick of #3234

Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story:
Fixes #OCPBUGS-24556

Checklist

Subject and description added to both, commit and PR.

Relevant issues have been referenced.

This change includes docs.

This change includes unit tests.

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.

nunnatsa · 2023-12-07T09:15:46Z

/jira refresh

openshift-ci-robot · 2023-12-07T09:15:49Z

@nunnatsa: This pull request references Jira Issue OCPBUGS-24556, which is invalid:

expected dependent Jira Issue OCPBUGS-23126 to be in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA), but it is ON_QA instead

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.

Details

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.

nunnatsa · 2023-12-07T09:17:25Z

/jira refresh

openshift-ci-robot · 2023-12-07T09:17:31Z

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

6 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.14.z) matches configured target version for branch (4.14.z)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)
dependent bug Jira Issue OCPBUGS-23126 is in the state Closed (Done), which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA))
dependent Jira Issue OCPBUGS-23126 targets the "4.15.0" version, which is one of the valid target versions: 4.15.0
bug has dependents

Requesting review from QA contact:
/cc @LiangquanLi930

Details

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.

davidvossel · 2023-12-07T15:13:19Z

+		if _, ok := hostedCluster.Annotations[hyperv1.CleanupCloudResourcesAnnotation]; ok {
+			if meta.FindStatusCondition(hostedCluster.Status.Conditions, string(hyperv1.HostedClusterDestroyed)) != nil {
+				return true, nil
+			}


does this need the same fix that you provided in #3289?

A user destroying a HostedCluster can cause the HostedCluster to hang indefinitely if the destroy command times out during execution. This is due to the hcp cli placing a finalizer on the HostedCluster during deletion which the cli tool later removes after waiting for some clean up actions to occur. If a user cancels the hcp destroy cluster command (or the command times out) while the cli is waiting for cleanup, then the HostedCluster will hang indefinitely with a DeletionTimestamp != nil. This PR only performs the additional cleaning logic, including the setting and removing the special finalizer, for platform that actually creates additional resources. Also, this PR sets the hostedCluster as the owner for these secrets, so they will be deleted with the HostedCluster. This PR is a manual cherry-pick of openshift#3234 Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>

nunnatsa · 2023-12-07T21:27:53Z

/test e2e-kubevirt-aws-ovn

davidvossel

/lgtm
/approve

openshift-ci · 2023-12-07T21:34:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidvossel, nunnatsa

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [davidvossel]

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-12-08T00:55:47Z

@nunnatsa: 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-ibmcloud-iks	`5767ccd`	link	false	`/test e2e-ibmcloud-iks`
ci/prow/e2e-ibmcloud-roks	`5767ccd`	link	false	`/test e2e-ibmcloud-roks`

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

davidvossel · 2023-12-08T16:07:18Z

/override "Red Hat Trusted App Test / mce-24-75q5r / tektok-runner-enterprise-contract-mce-2.4"

openshift-ci · 2023-12-08T16:08:15Z

@davidvossel: Overrode contexts on behalf of davidvossel: Red Hat Trusted App Test / mce-24-75q5r / tektok-runner-enterprise-contract-mce-2.4

Details

In response to this:

/override "Red Hat Trusted App Test / mce-24-75q5r / tektok-runner-enterprise-contract-mce-2.4"

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-ci-robot · 2023-12-08T16:11:03Z

@nunnatsa: Jira Issue OCPBUGS-24556: All pull requests linked via external trackers have merged:

openshift/hypershift#3290

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

Details

In response to this:

What this PR does / why we need it

A user destroying a HostedCluster can cause the HostedCluster to hang indefinitely if the destroy command times out during execution.

This is due to the hcp cli placing a finalizer on the HostedCluster during deletion which the cli tool later removes after waiting for some clean up actions to occur. If a user cancels the hcp destroy cluster command (or the command times out) while the cli is waiting for cleanup, then the HostedCluster will hang indefinitely with a DeletionTimestamp != nil.

This PR only performs the additional cleaning logic, including the setting and removing the special finalizer, for platform that actually creates additional resources. Also, this PR sets the hostedCluster as the owner for these secrets, so they will be deleted with the HostedCluster.

This PR is a manual cherry-pick of #3234

Which issue(s) this PR fixes

Fixes #OCPBUGS-24556

Checklist

Subject and description added to both, commit and PR.

Relevant issues have been referenced.

This change includes docs.

This change includes unit tests.

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-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 Dec 7, 2023

openshift-ci Bot added the do-not-merge/needs-area label Dec 7, 2023

openshift-ci Bot requested review from enxebre and sjenning December 7, 2023 09:16

openshift-ci Bot added area/cli Indicates the PR includes changes for CLI area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release and removed do-not-merge/needs-area labels Dec 7, 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 Dec 7, 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 Dec 7, 2023

openshift-ci Bot requested a review from LiangquanLi930 December 7, 2023 09:17

davidvossel reviewed Dec 7, 2023

View reviewed changes

nunnatsa force-pushed the r414-fix-OCPBUGS-23126 branch from 3896c6a to 5767ccd Compare December 7, 2023 17:56

nunnatsa requested a review from davidvossel December 7, 2023 21:28

davidvossel reviewed Dec 7, 2023

View reviewed changes

openshift-ci Bot assigned davidvossel Dec 7, 2023

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2023

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 7, 2023

openshift-merge-bot Bot merged commit 792a93b into openshift:release-4.14 Dec 8, 2023

nunnatsa deleted the r414-fix-OCPBUGS-23126 branch December 10, 2023 06:21

Conversation

nunnatsa commented Dec 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it

Which issue(s) this PR fixes

Checklist

Uh oh!

openshift-ci-robot commented Dec 7, 2023

What this PR does / why we need it**:

Uh oh!

nunnatsa commented Dec 7, 2023

Uh oh!

openshift-ci-robot commented Dec 7, 2023

Uh oh!

nunnatsa commented Dec 7, 2023

Uh oh!

openshift-ci-robot commented Dec 7, 2023

Uh oh!

davidvossel Dec 7, 2023

Choose a reason for hiding this comment

Uh oh!

nunnatsa commented Dec 7, 2023

Uh oh!

davidvossel left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Dec 7, 2023

Uh oh!

openshift-ci Bot commented Dec 8, 2023

Uh oh!

davidvossel commented Dec 8, 2023

Uh oh!

openshift-ci Bot commented Dec 8, 2023

Uh oh!

openshift-ci-robot commented Dec 8, 2023

What this PR does / why we need it

Which issue(s) this PR fixes

Checklist

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nunnatsa commented Dec 7, 2023 •

edited

Loading