-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Return directly if no pods found when evacuating #10447
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
Return directly if no pods found when evacuating #10447
Conversation
Not sure about this. Evacuating pods could be a precondition in a script, which must succeed before taking some other action. If all the pods with a given label selector are already evacuated (Length is 0), that is not an error |
Agreed. But we'd better return when Length is 0, anyway. We can give prompt info by |
Printing to e.Options.ErrWriter seems ok |
dbfce71
to
f2fef91
Compare
Fixed. @liggitt PTAL, thanks! |
@@ -89,6 +89,10 @@ func (e *EvacuateOptions) RunEvacuate(node *kapi.Node) error { | |||
if err != nil { | |||
return err | |||
} | |||
if len(pods.Items) == 0 { | |||
fmt.Fprint(e.Options.ErrWriter, "\nNo such pods on node: ", node.ObjectMeta.Name, "\n\n") |
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.
No pods found on node:
f2fef91
to
7019a7a
Compare
LGTM, [merge] |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8033/) (Image: devenv-rhel7_4852) |
Evaluated for origin merge up to 7019a7a |
[Test]ing while waiting on the merge queue |
Evaluated for origin test up to 7019a7a |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8019/) |
When evacuating selected pods with command
oadm manage-node <mynode> --evacuate --pod-selector="<service=myapp>"
, if there are no fit pods found, the command won't give any prompt. Users don't know what happened. Does the command execute successfully? Has any pod been evacuated? We'd better return an error if no pods found and the error info will be printed at terminal./cc @fabianofranz Thanks!