-
Notifications
You must be signed in to change notification settings - Fork 911
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 cluster vm override commands #977
Conversation
govc/cluster/vm/change.go
Outdated
if err != nil { | ||
return err | ||
} | ||
|
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.
do we need to check cluster here as well?
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, the cluster flag will never return nil, nil
. VirtualMachineFlag is optional in some commands, and can have multiple values in other commands. So it's left to the caller to decide what to do if none are specified.
govc/cluster/vm/remove.go
Outdated
if err != nil { | ||
return err | ||
} | ||
|
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.
ditto.
govc/test/cluster.bats
Outdated
run govc cluster.rule.create -cluster DC0_C0 -name my_deps -depend | ||
assert_failure # requires 2 groups | ||
|
||
run govc cluster.rule.create -cluster DC0_C0 -name my_deps -depend my_app my_db |
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.
this is -depend
or -depends
?, as well as the 3 cases above this.
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.
Good catch, it is -depends
, those tests were expected to fail, but they were failing because of an invalid flag name.
/lgtm |
Thinking about the command names, esp. after adding
Thinking |
- Add cluster.override.change command to set VM DRS and HA overrides - Add cluster.override.remove to remove VM overrides - Add cluster.override.info to view current VM overrides - Add cluster.rule.create '-depends' flag to support VM-to-VM dependency rules
Sticking with the new name, code is unchanged otherwise. |
Add cluster.override.change command to set VM DRS and HA overrides
Add cluster.override.remove to remove VM overrides
Add cluster.override.info to view current VM overrides
Add cluster.rule.create '-depends' flag to support VM-to-VM dependency rules