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

InterruptionEvent.Drained leads to confusion #407

Closed
cucxabong opened this issue Apr 13, 2021 · 1 comment · Fixed by #410
Closed

InterruptionEvent.Drained leads to confusion #407

cucxabong opened this issue Apr 13, 2021 · 1 comment · Fixed by #410
Labels
good first issue Good for newcomers Priority: Low This issue will not be seen by most users. The issue is a very specific use case or corner case

Comments

@cucxabong
Copy link
Contributor

cucxabong commented Apr 13, 2021

At first, I think this property mean that all Pod has been drained or whether or not pod should be drained. But this is something to told that an event has been processed or not. I think we should rename this to something like InterruptionEvent.Processed or InterruptionEvent.Done. What do you think about this?

@haugenj
Copy link
Contributor

haugenj commented Apr 13, 2021

I agree it's a bit confusing, but it doesn't exactly mean that the event has been processed either. The intention is that we avoid multiple drain calls for the same node, so it's more of a reference to the node associated with an event than the event itself. Perhaps InterruptionEvent.NodeDrained or .NodeProcessed or something like that would be better. Would you like to contribute a refactor for this?

@haugenj haugenj added good first issue Good for newcomers Priority: Low This issue will not be seen by most users. The issue is a very specific use case or corner case labels Apr 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers Priority: Low This issue will not be seen by most users. The issue is a very specific use case or corner case
Projects
None yet
2 participants