destroy: provide a way to stop all uninstalls using context #3800

staebler · 2020-06-24T17:28:21Z

The Run function in the Destroyer interface was modified to take a context as a parameter. This provides a way for the user stop the uninstall after a period of time by providing a context with a deadline.

The baremetal, libvirt, openstack, and ovirt providers do not provide a means by which most requests made to the provider can be stopped prematurely. In these cases, the context is checked prior to making the requests as a best effort. But the uninstall may continue for a period of time after the context is done.

The RunWithContext function introduced in #3765 for AWS has been obsoleted since the Run function now accepts a context.

This will be used by Hive to backoff uninstall attempts.

https://issues.redhat.com/browse/CO-974

abhinavdahiya · 2020-06-24T18:08:06Z

A "--timeout" flag was added to the openshift-install destroy cluster command. This allows the user to time out the destroy after a specified number of seconds.

If the users want to add a timeout, they should invoke the installer with timeout openshift-install ...
I don't think the installer destroy should provide that api.

abhinavdahiya · 2020-06-24T18:09:27Z

also if you can maybe split these into per-platform, switch destroy to pass context that would be very helpful in reviewing.

staebler · 2020-06-24T19:05:00Z

Update to put the changes for each provider in separate commits and to remove the "--timeout" flag.

abhinavdahiya · 2020-06-25T05:16:47Z

0195751

So i think GCP needs a relook. there is a global context for Uninstaller that is used to create new contexts with tighter timeouts for requests. This change just overwrites that as part of Run, while also removing the local tigher context for API calls.

1685b1f

hmm.. the placement to the ctx Errs seems a little random to me, is it possible to provide a little more context in the commit message? setting the convention like check for ctx before each item to be deleted or maybe only once per cycle etc. is what we need to try should help guide if change is good.

ed8ccf3

this needs review from openstack folks, cc @mandre @Fedosin

pkg/destroy/ovirt/destroyer.go

staebler · 2020-06-26T15:07:23Z

0195751

So i think GCP needs a relook. there is a global context for Uninstaller that is used to create new contexts with tighter timeouts for requests. This change just overwrites that as part of Run, while also removing the local tigher context for API calls.

I fixed this to use the 2-minute timeout for the calls to get the session and services.

1685b1f

hmm.. the placement to the ctx Errs seems a little random to me, is it possible to provide a little more context in the commit message? setting the convention like check for ctx before each item to be deleted or maybe only once per cycle etc. is what we need to try should help guide if change is good.

I fixed this to check the context directly before every request.

mandre · 2020-06-29T08:03:23Z

/test e2e-openstack

openshift-ci-robot · 2020-06-29T15:01:22Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign crawford
You can assign the PR to them by writing /assign @crawford in a comment when ready.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

staebler · 2020-06-29T15:02:59Z

78e20f8a6...a33077627

In gcp destroy, change PollImmediateInfinite to PollImmediateUntil to stop the polling as soon as the context is done rather than waiting for the polling interval to elapse.

mandre

OpenStack changes look good, I only have some reservations about the logging.

pkg/destroy/openstack/openstack.go

pierreprinetti · 2020-06-30T13:26:34Z

/test e2e-openstack

pierreprinetti · 2020-07-10T15:46:53Z

/retest

jstuever · 2020-07-15T21:54:18Z

gcp changes looks good to me.

The Run function in the Destroyer interface was modified to take a context as a parameter. This provides a way for the user stop the uninstall after a period of time by providing a context with a deadline. This will be used by Hive to backoff uninstall attempts. https://issues.redhat.com/browse/CO-974

Use the context passed into the Run function for AWS.

Use the context passed into the Run function for Azure.

Use the context passed into the Run function for GCP.

Use the context passed into the Run function for vSphere.

Use the context passed into the Run function for libvirt. Note that there is no means by which to abort in-flight requests made to libvirt. If the context completes while the uninstall is making a request, then the uninstall will not return until the request has completed.

Use the context passed into the Run function for OpenStack. Note that there is no means by which to abort in-flight requests made to OpenStack. If the context completes while the uninstall is making a request, then the uninstall will not return until the request has completed.

Use the context passed into the Run function for ovirt. Note that there is no means by which to abort in-flight requests made to ovirt. If the context completes while the uninstall is making a request, then the uninstall will not return until the request has completed.

staebler · 2020-07-21T17:23:19Z

a33077627...783ba77d2

Rebased onto master
Removed log field and log entry from OpenStack destroyer.

pierreprinetti · 2020-08-12T07:18:57Z

/retest

openshift-ci-robot · 2020-08-12T07:20:17Z

@staebler: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-libvirt	`783ba77`	link	`/test e2e-libvirt`
ci/prow/e2e-ovirt	`783ba77`	link	`/test e2e-ovirt`
ci/prow/e2e-crc	`783ba77`	link	`/test e2e-crc`
ci/prow/e2e-aws-fips	`783ba77`	link	`/test e2e-aws-fips`
ci/prow/e2e-aws-workers-rhel7	`783ba77`	link	`/test e2e-aws-workers-rhel7`
ci/prow/e2e-metal-ipi	`783ba77`	link	`/test e2e-metal-ipi`

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.

pierreprinetti · 2020-08-12T07:58:12Z

openstack-e2e is passing now.

openshift-bot · 2020-11-10T08:11:25Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-merge-robot · 2020-12-09T13:53:46Z

@staebler: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/openstack-manifests	`783ba77`	link	`/test openstack-manifests`

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.

openshift-bot · 2021-01-08T17:56:42Z

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

openshift-ci · 2021-01-14T15:59:08Z

@staebler: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/openstack-manifests	`783ba77`	link	`/test openstack-manifests`

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.

openshift-bot · 2021-02-13T17:27:03Z

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci-robot · 2021-02-13T17:27:14Z

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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 requested review from jstuever and mtnbikenc June 24, 2020 17:28

staebler force-pushed the destroy_with_context branch 2 times, most recently from 8234c43 to 7e98bab Compare June 24, 2020 17:32

staebler force-pushed the destroy_with_context branch 2 times, most recently from e31c1f0 to 0a722d8 Compare June 24, 2020 19:03

staebler force-pushed the destroy_with_context branch from 0a722d8 to 6ec4a5a Compare June 24, 2020 19:28

abhinavdahiya reviewed Jun 25, 2020

View reviewed changes

pkg/destroy/ovirt/destroyer.go Outdated Show resolved Hide resolved

staebler force-pushed the destroy_with_context branch 2 times, most recently from 2f25439 to 4f68b5b Compare June 26, 2020 15:05

staebler force-pushed the destroy_with_context branch from 4f68b5b to 78e20f8 Compare June 26, 2020 16:26

staebler force-pushed the destroy_with_context branch from 78e20f8 to a330776 Compare June 29, 2020 15:01

mandre reviewed Jun 30, 2020

View reviewed changes

pkg/destroy/openstack/openstack.go Outdated Show resolved Hide resolved

pkg/destroy/openstack/openstack.go Outdated Show resolved Hide resolved

mandre mentioned this pull request Jul 10, 2020

Bug 1816096: Add conditional exponential backoff for destroy module #3350

Closed

staebler added 5 commits July 21, 2020 13:11

destroyer: use context when running AWS uninstall

dab1f73

Use the context passed into the Run function for AWS.

destroyer: use context when running azure uninstall

3689f75

Use the context passed into the Run function for Azure.

destroyer: use context when running gcp uninstall

8985f45

Use the context passed into the Run function for GCP.

destroyer: use context when running vsphere uninstall

6bf5341

Use the context passed into the Run function for vSphere.

staebler added 3 commits July 21, 2020 13:11

staebler force-pushed the destroy_with_context branch from a330776 to 783ba77 Compare July 21, 2020 17:21

openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 10, 2020

openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 8, 2021

openshift-ci-robot closed this Feb 13, 2021

destroy: provide a way to stop all uninstalls using context #3800

destroy: provide a way to stop all uninstalls using context #3800

Uh oh!

Conversation

staebler commented Jun 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abhinavdahiya commented Jun 24, 2020

Uh oh!

abhinavdahiya commented Jun 24, 2020

Uh oh!

staebler commented Jun 24, 2020

Uh oh!

abhinavdahiya commented Jun 25, 2020

Uh oh!

Uh oh!

staebler commented Jun 26, 2020

Uh oh!

mandre commented Jun 29, 2020

Uh oh!

openshift-ci-robot commented Jun 29, 2020

Uh oh!

staebler commented Jun 29, 2020

Uh oh!

mandre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pierreprinetti commented Jun 30, 2020

Uh oh!

pierreprinetti commented Jul 10, 2020

Uh oh!

jstuever commented Jul 15, 2020

Uh oh!

staebler commented Jul 21, 2020

Uh oh!

pierreprinetti commented Aug 12, 2020

Uh oh!

openshift-ci-robot commented Aug 12, 2020

Uh oh!

pierreprinetti commented Aug 12, 2020

Uh oh!

openshift-bot commented Nov 10, 2020

Uh oh!

openshift-merge-robot commented Dec 9, 2020

Uh oh!

openshift-bot commented Jan 8, 2021

Uh oh!

openshift-ci bot commented Jan 14, 2021

Uh oh!

openshift-bot commented Feb 13, 2021

Uh oh!

openshift-ci-robot commented Feb 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

staebler commented Jun 24, 2020 •

edited

Loading