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

trigger finalizer even if the machine is already deleted from packet #25

Merged
merged 1 commit into from
May 6, 2020
Merged

trigger finalizer even if the machine is already deleted from packet #25

merged 1 commit into from
May 6, 2020

Conversation

gianarb
Copy link
Contributor

@gianarb gianarb commented May 6, 2020

If we attempt to remove a machine that is not running on Packet (for any
reason), we are in a loop where the reconciler returns 404 from Packet
API and the source never gets deleted.

This code should check for a 404, and it should pass forward the
finalizer trigger a garbage collection.

I presume we do not need to check for the number of instances anymore,
because if they are 0 we never get over the not found error.

@gianarb gianarb requested a review from deitch May 6, 2020 12:15
Copy link
Contributor

@deitch deitch left a comment

Choose a reason for hiding this comment

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

One typo and one minor change

return ctrl.Result{}, fmt.Errorf("error retrieving machine status %s: %v", packetmachine.Name, err)
}
if device == nil {

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

if err.(*packngo.ErrorResponse).Response != nil && err.(*packngo.ErrorResponse).Response.StatusCode == http.StatusNotFound {
// When the server does not exist we do not have anything left to do.
// Probably somebody manually deleted the server from the UI or via API.
logger.Info("Server not found, nothig left to do")
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: "nothig" should be "nothing"

@gianarb gianarb requested a review from deitch May 6, 2020 12:59
If we attempt to remove a machine that is not running on Packet (for any
reason), we are in a loop where the reconciler returns 404 from Packet
API and the source never gets deleted.

This code should check for a 404, and it should pass forward the
finalizer trigger a garbage collection.

I presume we do not need to check for the number of `instances` anymore,
because if they are `0` we never get over the `not found error`.
@deitch deitch merged commit 2cd36af into kubernetes-sigs:v1alpha3 May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants