Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make delete async #24

Merged
merged 3 commits into from May 31, 2019

Conversation

frobware
Copy link
Contributor

Testing is minimal and would need further rework which can be done as
a follow-up PR.

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 29, 2019
@frobware
Copy link
Contributor Author

frobware commented May 29, 2019

I don't know whether we care for updating the status during delete, just added it there for consistency (and some visual debugging). Dropped status updating.

pkg/cloud/gcp/actuators/machine/reconciler.go Outdated Show resolved Hide resolved
pkg/cloud/gcp/actuators/machine/reconciler.go Outdated Show resolved Hide resolved
pkg/cloud/gcp/actuators/machine/reconciler.go Outdated Show resolved Hide resolved
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 30, 2019
Copy link
Contributor

@michaelgugino michaelgugino left a comment

Choose a reason for hiding this comment

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

this lgtm. Need to test.

@frobware
Copy link
Contributor Author

/hold

Waiting on #25

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 31, 2019
Testing is minimal and would need further rework which can be done as
a follow-up PR.
This function and the constants it references are no longer used.
@frobware
Copy link
Contributor Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 31, 2019
@enxebre
Copy link
Member

enxebre commented May 31, 2019

/approve

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre

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:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 31, 2019
@frobware
Copy link
Contributor Author

/hold

Manual / sanity testing.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 31, 2019
This was introduced in PR openshift#25.
@frobware
Copy link
Contributor Author

Runtime behaviour:

I0531 13:37:34.187256    1703 controller.go:193] Reconciling machine "amcdermo-test-gcp-1" triggers delete
I0531 13:37:34.187268    1703 actuator.go:89] Deleting machine amcdermo-test-gcp-1
I0531 13:37:34.712075    1703 reconciler.go:237] Machine "amcdermo-test-gcp-1" already exists
I0531 13:37:35.256189    1703 reconciler.go:261] machine "amcdermo-test-gcp-1" status is exists, requeuing...
E0531 13:37:35.256266    1703 controller.go:207] Failed to delete machine "amcdermo-test-gcp-1": requeue in: 20s
I0531 13:37:35.256297    1703 controller.go:346] Actuator returned requeue-after error: requeue in: 20s
I0531 13:37:55.256673    1703 controller.go:129] Reconciling Machine "amcdermo-test-gcp-1"
I0531 13:37:55.256724    1703 controller.go:292] Machine "amcdermo-test-gcp-1" in namespace "test-gcp" doesn't specify "cluster.k8s.io/cluster-name" label, assuming nil cluster
I0531 13:37:55.256763    1703 controller.go:193] Reconciling machine "amcdermo-test-gcp-1" triggers delete
I0531 13:37:55.256788    1703 actuator.go:89] Deleting machine amcdermo-test-gcp-1
I0531 13:37:55.822772    1703 reconciler.go:237] Machine "amcdermo-test-gcp-1" already exists
I0531 13:37:56.232196    1703 reconciler.go:261] machine "amcdermo-test-gcp-1" status is exists, requeuing...
E0531 13:37:56.232281    1703 controller.go:207] Failed to delete machine "amcdermo-test-gcp-1": requeue in: 20s
I0531 13:37:56.232315    1703 controller.go:346] Actuator returned requeue-after error: requeue in: 20s
I0531 13:38:16.232527    1703 controller.go:129] Reconciling Machine "amcdermo-test-gcp-1"
I0531 13:38:16.232567    1703 controller.go:292] Machine "amcdermo-test-gcp-1" in namespace "test-gcp" doesn't specify "cluster.k8s.io/cluster-name" label, assuming nil cluster
I0531 13:38:16.232579    1703 controller.go:193] Reconciling machine "amcdermo-test-gcp-1" triggers delete
I0531 13:38:16.232597    1703 actuator.go:89] Deleting machine amcdermo-test-gcp-1
I0531 13:38:16.780866    1703 reconciler.go:234] Machine "amcdermo-test-gcp-1" is considered as non existent as its status is "TERMINATED"
I0531 13:38:16.780913    1703 reconciler.go:255] Machine amcdermo-test-gcp-1 not found during delete, skipping
I0531 13:38:16.894092    1703 controller.go:226] Machine "amcdermo-test-gcp-1" deletion successful

@frobware
Copy link
Contributor Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 31, 2019
Copy link
Contributor

@michaelgugino michaelgugino left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 31, 2019
@openshift-merge-robot openshift-merge-robot merged commit b5ea9a7 into openshift:master May 31, 2019
@frobware frobware deleted the make-delete-async branch May 31, 2019 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants