-
Notifications
You must be signed in to change notification settings - Fork 76
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 metadata functionality for vAppTemplate and media image/item #225
Conversation
Signed-off-by: Vaidotas Bauzys <[email protected]>
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.
preliminary review
govcd/metadata.go
Outdated
return vAppTemplate, nil | ||
} | ||
|
||
// AddMetadata() function calls private function addMetadata() with vAppTemplate.client and vAppTemplate.VAppTemplate.HREF |
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 comment should say AddMetadataAsync
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.
fixed
govcd/metadata.go
Outdated
} | ||
|
||
// AddMetadata() function calls private function addMetadata() with vAppTemplate.client and vAppTemplate.VAppTemplate.HREF | ||
// which adds metadata key, value pair provided as input. |
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.
"key, value" -> "key/value"
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.
Fixed
govcd/metadata.go
Outdated
return getMetadata(mediaItem.client, mediaItem.MediaItem.HREF) | ||
} | ||
|
||
// AddMetadata() function adds metadata key, value pair provided as input. |
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.
"key, value" -> "key/value"
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.
fixed
govcd/metadata.go
Outdated
} | ||
err = task.WaitTaskCompletion() | ||
if err != nil { | ||
return nil, fmt.Errorf("error completing add metadata for media item task: %#v", 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.
"%#v" -> "%s"
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.
Changed
govcd/metadata.go
Outdated
|
||
err = mediaItem.Refresh() | ||
if err != nil { | ||
return nil, fmt.Errorf("error refreshing media item: %#v", 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.
"%#v" -> "%s"
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.
Changed
|
||
uploadTask, err := catalog.UploadMediaImage(itemName, "upload from test", vcd.config.Media.MediaPath, 1024) | ||
check.Assert(err, IsNil) | ||
err = uploadTask.WaitTaskCompletion() |
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.
task should be checked before usage
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.
added
|
||
// Check if metadata was added correctly | ||
metadata, err := mediaItem.GetMetadata() | ||
check.Assert(err, IsNil) |
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.
metadata should be also checked
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.
Changed
} | ||
|
||
catItem, err := cat.FindCatalogItem(vcd.config.VCD.Catalog.CatalogItem) | ||
check.Assert(err, IsNil) |
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.
catItem should be also checked
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.
Changed
|
||
// Get VAppTemplate | ||
vAppTemplate, err := catItem.GetVAppTemplate() | ||
check.Assert(err, IsNil) |
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.
vAppTemplate should be also checked
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.
changed
govcd/vapptemplate_test.go
Outdated
oldVAppTemplate := vAppTemplate | ||
|
||
err = vAppTemplate.Refresh() | ||
check.Assert(oldVAppTemplate.VAppTemplate, Equals, vAppTemplate.VAppTemplate) |
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.
err should be checked
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.
fixed
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.
Just a few comments inline.
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]> # Conflicts: # CHANGELOG.md
Signed-off-by: Vaidotas Bauzys <[email protected]> # Conflicts: # CHANGELOG.md
Signed-off-by: Vaidotas Bauzys <[email protected]>
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.
LGTM
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.
LGTM
Ref:https://github.com/terraform-providers/terraform-provider-vcd/issues/285
Added new CRUD metadata functions for vApp template and media image.
Also new refresh functions for vApp template and media image.
This new functions will be used in terraform to provide resources metadata capabilities.