Skip to content

SPLAT-1995: vsphere on destroy remove cns volumes#9425

Merged
openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
jcpowermac:vsphere-destroy-cns-volumes
Mar 5, 2025
Merged

SPLAT-1995: vsphere on destroy remove cns volumes#9425
openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
jcpowermac:vsphere-destroy-cns-volumes

Conversation

@jcpowermac
Copy link
Contributor

@jcpowermac jcpowermac commented Jan 31, 2025 •
edited
Loading

This pr does the following:

  • Add vSphere CNS client
  • Returns a slice of CNS volumes associated with the infraid via GetCnsVolumes()
  • Destroys all CNS volumes via DeleteCnsVolumes() based on slice provide by GetCnsVolumes()
  • Updates the generated mock for the new methods

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 31, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 31, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@jcpowermac jcpowermac changed the title vsphere on destroy remove cns volumes SPLAT-1995: vsphere on destroy remove cns volumes Jan 31, 2025
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 31, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jan 31, 2025 •
edited by openshift-ci bot
Loading

@jcpowermac: This pull request references SPLAT-1995 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

Details

In response to this:

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.

@jcpowermac
Copy link
Contributor Author

jcpowermac commented Jan 31, 2025

Related to #9388

@jcpowermac jcpowermac force-pushed the vsphere-destroy-cns-volumes branch 2 times, most recently from 6f0d6ec to 1f18415 Compare February 5, 2025 18:05
@jcpowermac
Copy link
Contributor Author

jcpowermac commented Feb 5, 2025

/test gofmt
/test golint
/test govet
/test images
/test integration-tests
/test unit
/test verify-vendor
/test verify-codegen

@jcpowermac jcpowermac force-pushed the vsphere-destroy-cns-volumes branch from 1f18415 to 0424786 Compare February 5, 2025 19:11
@jcpowermac
Copy link
Contributor Author

jcpowermac commented Feb 5, 2025

/test gofmt
/test golint
/test govet
/test images
/test integration-tests
/test unit
/test verify-vendor
/test verify-codegen

@jcpowermac jcpowermac force-pushed the vsphere-destroy-cns-volumes branch 4 times, most recently from a7e70b5 to 2204c48 Compare February 6, 2025 14:02
@jcpowermac
Copy link
Contributor Author

jcpowermac commented Feb 6, 2025

/test gofmt
/test golint
/test govet
/test images
/test integration-tests
/test unit
/test verify-vendor
/test verify-codegen
/test e2e-vsphere-ovn

@jcpowermac jcpowermac force-pushed the vsphere-destroy-cns-volumes branch from 2204c48 to 48049b9 Compare February 6, 2025 18:06
@jcpowermac jcpowermac marked this pull request as ready for review February 6, 2025 18:07
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 6, 2025
@openshift-ci openshift-ci bot requested review from sadasu and vr4manta February 6, 2025 18:08
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Feb 6, 2025 •
edited by openshift-ci bot
Loading

@jcpowermac: This pull request references SPLAT-1995 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

Details

In response to this:

This pr does the following:

  • Add CNS client
  • Returns a slice of CNS volumes associated with the infraid via GetCnsVolumes()
  • Destroys all CNS volumes via DeleteCnsVolumes() based on slice provide by GetCnsVolumes()
  • Updates the generated mock for the new methods

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.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Feb 6, 2025 •
edited by openshift-ci bot
Loading

@jcpowermac: This pull request references SPLAT-1995 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

Details

In response to this:

This pr does the following:

  • Add vSphere CNS client
  • Returns a slice of CNS volumes associated with the infraid via GetCnsVolumes()
  • Destroys all CNS volumes via DeleteCnsVolumes() based on slice provide by GetCnsVolumes()
  • Updates the generated mock for the new methods

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.

@jcpowermac jcpowermac force-pushed the vsphere-destroy-cns-volumes branch 2 times, most recently from 06439ec to 6221566 Compare February 10, 2025 14:11
@vr4manta
Copy link
Contributor

vr4manta commented Feb 10, 2025

/lgtm

@jcpowermac
Copy link
Contributor Author

jcpowermac commented Feb 10, 2025

@gnufied can you take a look too? Thanks!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 10, 2025
gnufied
gnufied reviewed Feb 10, 2025
View reviewed changes
pkg/destroy/vsphere/vsphere.go Outdated
Copy link
Member

@gnufied gnufied Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't delete CNS volumes be done before tags, categories or even storage policies are deleted? IIRC - if one or more volumes that refer the storage policy being deleted still exists then deletion of storage policy fails (this is one of the primary reasons we have steady accumulation of storage policies in devqe environments I think).

But please do double check the order.

Copy link
Contributor Author

@jcpowermac jcpowermac Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gnufied, let me test it

@jcpowermac
vsphere on destroy remove cns volumes
2d6771b 
This commit does the following:

    Add vSphere CNS client
    Returns a slice of CNS volumes associated with the infraid via GetCnsVolumes()
    Destroys all CNS volumes via DeleteCnsVolumes() based on slice provide by GetCnsVolumes()
    Updates the generated mock for the new methods
@jcpowermac jcpowermac force-pushed the vsphere-destroy-cns-volumes branch from 6221566 to 2d6771b Compare February 11, 2025 18:12
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 11, 2025
@jcpowermac
Copy link
Contributor Author

jcpowermac commented Feb 11, 2025

@gnufied tested, moved cns destroy to right after virtual machine delete.
The storage policies were removed.

@gnufied
Copy link
Member

gnufied commented Feb 11, 2025

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 11, 2025
@jcpowermac
Copy link
Contributor Author

jcpowermac commented Feb 13, 2025

works as expected

time="2025-02-11T20:25:52Z" level=info msg=Destroyed VirtualMachine=ci-op-wchi1y89-bf920-fgs4v-worker-0-55p7j
time="2025-02-11T20:25:52Z" level=debug msg="Delete CNS Volumes"
time="2025-02-11T20:25:56Z" level=info msg=Destroyed CNS Volume=ad548b88-9fde-47a1-9ff8-22d058535e88
time="2025-02-11T20:25:57Z" level=info msg=Destroyed CNS Volume=a52c7316-a770-4854-8b98-6c0f7936e54d
time="2025-02-11T20:25:57Z" level=info msg=Destroyed CNS Volume=375e5f98-2d8c-46c2-afbe-bb59aeb3a4c7
time="2025-02-11T20:25:57Z" level=debug msg="Delete Folder"

@jcpowermac
Copy link
Contributor Author

jcpowermac commented Feb 24, 2025

/label acknowledge-critical-fixes-only

@openshift-ci openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Feb 24, 2025
@jcpowermac
Copy link
Contributor Author

jcpowermac commented Feb 24, 2025

/test okd-scos-images

2 similar comments
@jcpowermac
Copy link
Contributor Author

jcpowermac commented Feb 25, 2025

/test okd-scos-images

@jcpowermac
Copy link
Contributor Author

jcpowermac commented Feb 25, 2025

/test okd-scos-images

@jcpowermac
Copy link
Contributor Author

jcpowermac commented Feb 27, 2025

/assign @sadasu

@jcpowermac
Copy link
Contributor Author

jcpowermac commented Feb 27, 2025

/assign @patrickdillon

@patrickdillon
Copy link
Contributor

patrickdillon commented Mar 3, 2025

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 3, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 3, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 3, 2025

/retest-required

Remaining retests: 0 against base HEAD de563b9 and 2 for PR HEAD 2d6771b in total

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 4, 2025

/retest-required

Remaining retests: 0 against base HEAD b20e29d and 2 for PR HEAD 2d6771b in total

1 similar comment
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 4, 2025

/retest-required

Remaining retests: 0 against base HEAD b20e29d and 2 for PR HEAD 2d6771b in total

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 4, 2025

/retest-required

Remaining retests: 0 against base HEAD 783df5a and 2 for PR HEAD 2d6771b in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 4, 2025 •
edited
Loading

@jcpowermac: 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-vsphere-host-groups-ovn-custom-no-upgrade 06439ecdd201b3469dda13607950a7f3ee5626be link false /test e2e-vsphere-host-groups-ovn-custom-no-upgrade
ci/prow/okd-scos-e2e-aws-ovn 2d6771b link false /test okd-scos-e2e-aws-ovn
ci/prow/e2e-azure-ovn-resourcegroup 2d6771b link false /test e2e-azure-ovn-resourcegroup

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.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 5, 2025

/retest-required

Remaining retests: 0 against base HEAD 5b9f706 and 2 for PR HEAD 2d6771b in total

@openshift-merge-bot openshift-merge-bot bot merged commit e4014cd into openshift:main Mar 5, 2025
23 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sadasu sadasu Awaiting requested review from sadasu

@vr4manta vr4manta Awaiting requested review from vr4manta

1 more reviewer

@gnufied gnufied gnufied left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@gnufied gnufied

@sadasu sadasu

@patrickdillon patrickdillon

@vr4manta vr4manta

Labels

acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jcpowermac @openshift-ci-robot @vr4manta @gnufied @patrickdillon @sadasu