-
Notifications
You must be signed in to change notification settings - Fork 917
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
Implemented vm.upgrade operation. #918
Conversation
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.
Thanks @pupska looks good, a few small change requests.
govc/vm/upgrade.go
Outdated
} | ||
err = task.Wait(ctx) | ||
if err != nil { | ||
if err.Error() == "Virtual machine compatibility is already up-to-date." { |
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.
Rather than check the message string, you should be able to use:
func isAlreadyUpgraded(err error) bool {
if fault, ok := err.(task.Error); ok {
_, ok = fault.Fault().(*types.AlreadyUpgraded)
return ok
}
return false
}
Then:
if err != nil {
if isAlreadyUpraded(err) {
fmt.Println(err.Error())
}
}
govc/vm/upgrade.go
Outdated
return err | ||
} | ||
|
||
var target_version = "" |
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.
golint warns: don't use underscores in Go names; var target_version should be targetVersion
or you can just name it version
govc/vm/upgrade.go
Outdated
cmd.VirtualMachineFlag, ctx = flags.NewVirtualMachineFlag(ctx) | ||
cmd.VirtualMachineFlag.Register(ctx, f) | ||
|
||
f.IntVar(&cmd.version, "version", 0, "Target vm hardware version, by default -- latest available ") |
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.
remove trailing space
govc/vm/upgrade.go
Outdated
} else { | ||
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.
remove blank line
govc/vm/upgrade.go
Outdated
return `Upgrade VMs to latest hardware version | ||
|
||
Examples: | ||
govc vm.upgrade -vm $vm_name |
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.
2 space indent instead of tab to match the existing examples format.
@pupska, VMware has approved your signed contributor license agreement. |
@dougm thanks for your comments, I've pushed updated version. |
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.
nice, thanks for the updates!
Related to https://kb.vmware.com/s/article/1010675