-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add additional requestors #11
Add additional requestors #11
Conversation
Pull Request Test Coverage Report for Build 10511201415Details
💛 - Coveralls |
38f6e93
to
ec0c566
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.
Looks, good. We can merge the PR after rebasing it.
defaultMaxNodeMaintenanceTime = 1600 * time.Second | ||
waitPodCompletionRequeueTime = 10 * time.Second | ||
drainReqeueTime = 10 * time.Second | ||
defaultMaxNodeMaintenanceTime = 1600 * time.Second |
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: Time > Seconds?
It is ok for me keep the "time" postfix if you prefer so
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.
lets keeps as is :)
- remove un-needed protobuf markers - remove unsupported strategic merge related markes as they are not supported for CRDs [1] - add new field in spec: AdditionalRequestors to be used to allow multiple entities to share the same NodeMaintenance object. [1] kubernetes/kubectl#1280 Signed-off-by: adrianc <[email protected]>
nodemaintenance controller modifications to support additionalRequestors. - check that no additional requestors are present before proceeding with object removal flow when object is in ready state - update controller tests Signed-off-by: adrianc <[email protected]>
ec0c566
to
0f5f558
Compare
Merging once CI is green, thx for the review ! |
only last two commits are relevant, the rest is PR #9