-
Notifications
You must be signed in to change notification settings - Fork 1
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
adding delayed removal of xobjectbucket #239
Conversation
Did you test, that Crossplane correctly removes the |
age := now.Sub(creationTimestamp.Time) | ||
|
||
if age < 61*time.Minute { | ||
return nil, fmt.Errorf("XObjectBucket is too young to be deleted") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: Maybe print how many minutes remaining until it should be deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the phrasing "too young" really does not help. Either print the minutes as suggested above or just something like "XObjectBucket has been created less than 1 hour".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just added enhanced logging
I tested if xobjectbucket was gone after 61 seconds :D |
Could you still try it with a larger amount of time? 1 minute is within a single Crossplane reconcile loop. Also, something interesting to test as well: does it still delete it, if Crossplane gets restarted before the timer runs out? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
b5d6a1f
to
3156fd9
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
Summary
Checklist
bug
,enhancement
,documentation
,change
,breaking
,dependency
as they show up in the changelog