-
Notifications
You must be signed in to change notification settings - Fork 28
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
use the copied node obj to drain the node #165
use the copied node obj to drain the node #165
Conversation
f7602e9
to
8bb6043
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #165 +/- ##
==========================================
+ Coverage 69.74% 69.78% +0.04%
==========================================
Files 12 12
Lines 1312 1314 +2
==========================================
+ Hits 915 917 +2
Misses 325 325
Partials 72 72 ☔ View full report in Codecov by Sentry. |
pkg/service/nodes.go
Outdated
err = drainNodeUtil(node, int(timeout), kubeClient) | ||
// create a copy of the node obj, since RunCordonOrUncordon() modifies the node obj | ||
nodeCopy := node.DeepCopy() | ||
err = drainNodeUtil(nodeCopy, int(timeout), kubeClient) | ||
if err == nil { | ||
log.Info("drain suceeded") |
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.
Can you also fix the spelling error and change to succeeded
?
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.
Please help to add the node name and instance id in both retry drain
and drain succeeded
logs.
it will help debugging.
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.
fixed both. please review again.
8bb6043
to
5788885
Compare
Signed-off-by: Zihan Jiang <zihan_jiang@intuit.com>
5788885
to
6f12728
Compare
Thanks for the fix. |
fixes #164