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

new functions for creation of empty VM #296

Merged
merged 19 commits into from
Apr 28, 2020
Merged

new functions for creation of empty VM #296

merged 19 commits into from
Apr 28, 2020

Conversation

vbauzys
Copy link
Contributor

@vbauzys vbauzys commented Mar 30, 2020

New functions needed implementing empty VM functionality in terraform.

  • Add empty MV
  • Query all media is used to find media to attach as boot image
  • UpdateVmSpecSectionAsync/UpdateVmSpecSection

Signed-off-by: Vaidotas Bauzys <[email protected]>
@vbauzys vbauzys self-assigned this Mar 30, 2020
@vbauzys vbauzys requested review from Didainius, dataclouder and lvirbalas and removed request for Didainius, dataclouder and lvirbalas March 30, 2020 12:49
Copy link
Collaborator

@Didainius Didainius left a comment

Choose a reason for hiding this comment

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

Thanks. A few questions inline and one below.

Does it satisfy the need to create a VM without having a vApp? If so - I am wondering should the parent be vApp or vDC? (having in mind that there may be no vApp context for user)

Copy link
Contributor

@dataclouder dataclouder left a comment

Choose a reason for hiding this comment

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

First pass

CHANGELOG.md Outdated
@@ -2,6 +2,7 @@

* Added methods `OrgVdcNetwork.Update`, `OrgVdcNetwork.UpdateAsync`, and `OrgVdcNetwork.Rename` [#292](https://github.com/vmware/go-vcloud-director/pull/292)
* Added methods `EdgeGateway.Update` and `EdgeGateway.UpdateAsync` [#292](https://github.com/vmware/go-vcloud-director/pull/292)
* Added methods `vapp.AddEmptyVm`, `vapp.AddEmptyVmAsync` and vdc.QueryAllMedia [#296](https://github.com/vmware/go-vcloud-director/pull/296)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Added methods `vapp.AddEmptyVm`, `vapp.AddEmptyVmAsync` and vdc.QueryAllMedia [#296](https://github.com/vmware/go-vcloud-director/pull/296)
* Added methods `vapp.AddEmptyVm`, `vapp.AddEmptyVmAsync` and `vdc.QueryAllMedia` [#296](https://github.com/vmware/go-vcloud-director/pull/296)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

govcd/api.go Outdated
@@ -564,3 +564,8 @@ func combinedTaskErrorMessage(task *types.Task, err error) string {
func takeBoolPointer(value bool) *bool {
return &value
}

// takeIntAddress is a helper which can gives address of `int`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// takeIntAddress is a helper which can gives address of `int`
// takeIntAddress is a helper that returns the address of an `int`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

govcd/media.go Outdated
@@ -637,3 +637,42 @@ func (mediaRecord *MediaRecord) Delete() (Task, error) {
return mediaRecord.client.ExecuteTaskRequest(mediaRecord.MediaRecord.HREF, http.MethodDelete,
"", "error deleting Media item: %s", nil)
}

// QueryAllMedia returns media image found in system using `name` as query.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// QueryAllMedia returns media image found in system using `name` as query.
// QueryAllMedia returns all media images found in system using `name` as query.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated


testQueryMediaName := vcd.config.Media.Media

mediaSlise, err := vcd.vdc.QueryAllMedia(testQueryMediaName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
mediaSlise, err := vcd.vdc.QueryAllMedia(testQueryMediaName)
mediaList, err := vcd.vdc.QueryAllMedia(testQueryMediaName)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

govcd/vm.go Outdated
}, vm.client.GetSpecificApiVersionOnCondition(">= 32.0", "32.0"))

}

// AddEmptyVm adds new empty VM (without template) to vApp and returns new created VM or error.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// AddEmptyVm adds new empty VM (without template) to vApp and returns new created VM or error.
// AddEmptyVm adds an empty VM (without template) to vApp and returns the new created VM or an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

govcd/vm.go Outdated

}

// AddEmptyVmAsync adds new empty VM (without template) to vApp and returns Task or error.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// AddEmptyVmAsync adds new empty VM (without template) to vApp and returns Task or error.
// AddEmptyVmAsync adds an empty VM (without template) to the vApp and returns a Task and an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

types.MimeRecomposeVappParams, "error instantiating a new VM: %s", reComposeVAppParams)
}

func validateEmptyVmParams(reComposeVAppParams *types.RecomposeVAppParamsForEmptyVm) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add description

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

govcd/vm_test.go Outdated
Comment on lines 1356 to 1357
//media, err := cat.GetMediaByName("photon-custom-hw11-2.0-304b817.ova", false)
media, err := cat.GetMediaByName("vaido2", false)
Copy link
Contributor

Choose a reason for hiding this comment

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

hardcoded name. How does this media item become available?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated. Value comes from configuration

@@ -0,0 +1,39 @@
package types
Copy link
Contributor

Choose a reason for hiding this comment

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

Why a separate file for some VM related types?
If we split by entity, this file should contain all VM types, but then we should have separate files for each entity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am trying to start splitting by domain(VM/VAPP/EdgeGtw/ etc.). If everyone agrees then it is start to add new one to separate type files. In time we move everything accordingly. I won't do let's split everything now it's to much and not this PR scope, but we can start doing better from today.

vbauzys added 2 commits April 2, 2020 15:51
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
vbauzys added 5 commits April 3, 2020 17:14
Signed-off-by: Vaidotas Bauzys <[email protected]>
Merge branch 'master' of github.com:vmware/go-vcloud-director
Signed-off-by: Vaidotas Bauzys <[email protected]>

# Conflicts:
#	CHANGELOG.md
Copy link
Collaborator

@Didainius Didainius left a comment

Choose a reason for hiding this comment

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

Tests passed on 9.7, 10.0 and 10.1

Copy link
Contributor

@dataclouder dataclouder left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

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

LGTM

@vbauzys vbauzys merged commit 507d694 into vmware:master Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants