-
Notifications
You must be signed in to change notification settings - Fork 112
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 vcd_catalog_vapp_template resource and data source for managing vApp Templates and deprecate vcd_catalog_item #899
Add vcd_catalog_vapp_template resource and data source for managing vApp Templates and deprecate vcd_catalog_item #899
Conversation
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
…ider-vcd into add-catalog-vapp-template Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
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.
A few inline comments. Looks good otherwise - just double check that catalog tests use NSX-T VDC and templates - this should save us refactoring headache in future and add to CDS testing.
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
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! Looking forward to the needed changes in VM to be able to deprecate the catalog item.
Signed-off-by: abarreiro <abarreiro@vmware.com>
Signed-off-by: abarreiro <abarreiro@vmware.com>
Overview
This PR adds the resource and data source vcd_catalog_vapp_template to be able to upload, manage and fetch vApp Templates. This resource and data source is very similar to the existing vcd_catalog_item, in fact the idea is to potentially deprecate the latter once other resources like vcd_vm and vcd_vapp_vm are adjusted to use vApp Templates instead of Catalog Items.
Detailed description
As #502 mentions, current vcd_catalog_item implementation is an hybrid of Catalog Item and vApp Template. In order to decouple both concepts, this PR adds the resource and data source vcd_catalog_vapp_template which only considers vApp Templates.
As v3.8.0 doesn't break compatibility, vcd_catalog_item resource and data source specification and source code are preserved as-is with minor code changes that doesn't affect its behaviour.
vcd_catalog_item is NOT deprecated in this PR as it can still be used in the vcd_vm and vcd_vapp_vm resources. A new PR must be done to adjust these two resources and be able to deprecate vcd_catalog_item
resource_vcd_catalog_vapp_template.go: New resource for vApp Template
datasource_vcd_catalog_vapp_template.go: New data source for vApp Template
catalogitem.go: Removed the comment stating that
findCatalogItem
should be changed, as there isfindVAppTemplate
instead (see "Other comments").filter_get.go: Created
getVappTemplateByVdcAndFilter
to be able to fetch vApp Templates by VDC instead of Catalog:getVappTemplateByCatalogAndFilter
.vcd/config_test.go: Added required entry in configuration for a vApp Template present in NSX-T backed VDC:
nsxtCatalogItem
.Other comments
findVAppTemplate
that behaves similarly tofindCatalogItem
(in fact it started as a clone).I must admit that I'm not very happy with the implementation of this function, as the resulting code is quite complex and hard to read. The situation here is that I tried to split it in little functions to reduce complexity and comply a bit more with SRP, but in exchange the
vcd
package got polluted with these tiny semantic functions that were not quite re-usable. So I finally decided to have this big function. Let me know your thoughts about this.Testing
catalog
passresource/vcd_catalog_vapp_template
anddatasource/vcd_catalog_vapp_template
resource/vcd_catalog_item
anddatasource/vcd_catalog_item
to avoid regressions