update manage-node to support multiple output formats #14655
Conversation
|
[test] |
pkg/cmd/admin/node/node.go
Outdated
| @@ -119,6 +88,41 @@ func NewCommandManageNode(f *clientcmd.Factory, commandName, fullName string, ou | |||
| return cmd | |||
| } | |||
|
|
|||
| func (n *NodeOptions) RunManageNode(c *cobra.Command, f *clientcmd.Factory, args []string, evacuateOp *EvacuateOptions, listpodsOp *ListPodsOptions, schedulableOp *SchedulableOptions, out, errout io.Writer) error { | |||
There was a problem hiding this comment.
This seems awkward. Looks like you would want:
type ManageNodeOptions struct {
nodeOptions NodeOptions
evacuateOptions EvacuateOptions
listPodsOptions ListPodOptions
schedulableOptions SchedulableOptions
out io.Writer
out, errout io.Writer
}
func (n *ManageNodeOptions) Complete(c *cobra.Command, f *clientcmd.Factory, args []string) error {
return n.nodeOptions.Complete(f, c, args, out, errout)
}
func (n *ManageNodeOptions) RunManageNode(c *cobra.Command, f *clientcmd.Factory, args []string) error {
// ...
}Or something like that
There was a problem hiding this comment.
That looks better, will update. Thanks
juanvallejo
force-pushed
the
jvallejo/fix-manage-node-output
branch
from
ce885e1 to
31c4abd
Compare
|
@stevekuznetsov thanks, review comment addressed |
pkg/cmd/admin/node/node.go
Outdated
| } | ||
|
|
||
| func (n *ManageNodeOptions) RunManageNode(c *cobra.Command) error { | ||
| if err := ValidOperation(c); err != nil { |
There was a problem hiding this comment.
Looks like a Validate?
pkg/cmd/admin/node/node.go
Outdated
| return kcmdutil.UsageError(c, err.Error()) | ||
| } | ||
|
|
||
| // Cross op validations |
|
@dobbymoodge did we edit rosie? |
|
No... Not yet at least. I have 2 PRs in the works but I've only run them
against redshirt.
What was in process pull requests for this time?
…On Jun 14, 2017 10:39 PM, "Steve Kuznetsov" ***@***.***> wrote:
@dobbymoodge <https://github.com/dobbymoodge> did we edit rosie?
[image: rosie]
<https://user-images.githubusercontent.com/7328370/27162966-3571318c-5139-11e7-8ad6-e396d580c911.PNG>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#14655 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABd0FOJob5bcY4ZvzwnhbwfJj9YgngOIks5sEJl4gaJpZM4N6ZHG>
.
|
juanvallejo
force-pushed
the
jvallejo/fix-manage-node-output
branch
from
31c4abd to
686f83e
Compare
|
@stevekuznetsov thanks, comments addressed |
Sure, here's Evacuate with |
|
Love it. |
|
flaked on #13271 |
|
No, you have a compilation failure: |
juanvallejo
force-pushed
the
jvallejo/fix-manage-node-output
branch
from
686f83e to
bd5608b
Compare
|
[merge][severity: bug] |
|
flaked on #13108 re[test] |
|
flaked on #10182 re[test] |
|
Evaluated for origin test up to bd5608b |
|
continuous-integration/openshift-jenkins/test Running (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2338/) (Base Commit: ee67917) (PR Branch Commit: bd5608b) |
|
continuous-integration/openshift-jenkins/test SUCCESS |
|
Evaluated for origin merge up to bd5608b |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/1027/) (Base Commit: 27afdba) (PR Branch Commit: bd5608b) (Extended Tests: bug) (Image: devenv-rhel7_6374) |
Fixes #14360
Updates
ManageNodeto follow complete,verify,run patternUpdates
Completemethod ofNodeOptionsto take command printer output format into accountcc @openshift/cli-review @stevekuznetsov