Skip to content
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

Merged
merged 1 commit into from
Nov 15, 2017
Merged

Conversation

vmbobyr
Copy link
Contributor

@vmbobyr vmbobyr commented Nov 14, 2017

@vmwclabot
Copy link
Member

@pupska, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

Copy link
Member

@dougm dougm left a 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.

}
err = task.Wait(ctx)
if err != nil {
if err.Error() == "Virtual machine compatibility is already up-to-date." {
Copy link
Member

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())
   }
}

return err
}

var target_version = ""
Copy link
Member

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

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 ")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove trailing space

} else {
return err
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove blank line

return `Upgrade VMs to latest hardware version

Examples:
govc vm.upgrade -vm $vm_name
Copy link
Member

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.

@vmwclabot
Copy link
Member

@pupska, VMware has approved your signed contributor license agreement.

@vmbobyr
Copy link
Contributor Author

vmbobyr commented Nov 15, 2017

@dougm thanks for your comments, I've pushed updated version.

Copy link
Member

@dougm dougm left a 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!

@dougm dougm merged commit 0ea3b9b into vmware:master Nov 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants