terraform-provider-vcd
terraform-provider-vcd copied to clipboard
Refactor VM "Create" code
The goal is to improve VM Create code and:
- Remove unnecessary power on/power off cycles during VM spinup which should improve guest customization experience
- Simplify legacy code which is hard to maintain (it should additionally improve performance of operations due to reduced API call number)
Architecture fixes:
-
Create used to call Update for a VM creation task. Create is now decoupled from Update.
-
Multiple power on and power off operations were executed during create operation. Power on is now executed as the last step of creation.
-
All VM creation structures (
types.XXXX
) are now populated insideCreate
code as historically some of them used a govcd function, while others populated it directly. The problem with such an approach is that one needs to create new SDK functions with every new feature in VM. This is the list oftypes
thatCreate
code now uses.-
types.InstantiateVmTemplateParams
(Standalone VM from template) -
types.ReComposeVAppParams
(vApp VM from template) -
types.RecomposeVAppParamsForEmptyVm
(Empty vApp VM) -
types.CreateVmParams
(Empty Standalone VM)
-
-
Update
code is not affected by this PR.
What wasn't done:
- The functions, responsible for particular structure, like guest customization, networks, etc - were not changed - some of them were just moved to a separate function instead of being inline.
The new structure of Create code allows users to inject code where it is needed:
Flow diagram for create
functions of vcd_vapp_vm
and vcd_vm
.
- Purple shows the main flow of code for
Create
- Green blocks signify
govcd.types
definition for each of VM types that are being created. - Yellow blocks signify where additional code can be hooked up for particular scope
flowchart TD
S[<b>VM Create</b>]
S ==> AAAA
AAAA{Standanlone or vApp VM?}
AAAA --> |resource <b>vcd_vm</b>| AA{vApp VM or Standalone}
AAAA --> |resource <b>vcd_vapp_vm</b>| A{vApp VM or Standalone}
AA[func <b>resourceVcdStandaloneVmCreate</b>] --> B[func <b>genericResourceVmCreate</b>]
A[func <b>resourceVcdVAppVmCreate</b>] -->|lock parent vApp\n defer unlock| B[func <b>genericResourceVmCreate</b>]
BB{From <b>template</b> or <b>empty</b> VM}
B --> BB
BB -->|isVmFromTemplate| D[func <b>createVmFromTemplate</b>]
BB -->|isEmptyVm| E[func <b>createVmEmpty</b>]
DDD([vApp VM from template])
DDDD(Standalone VM from template)
EEE(Empty vApp VM)
EEEE(Empty Standalone VM)
DD([<b>Optionally</b> perform <b>template VM</b> specific actions])
EE([<b>Optionally</b> perform <b>empty VM</b> specific actions])
DDOPTIONAL1([<b>Optionally</b> perform <b>vApp VM template</b> specific actions])
DDOPTIONAL2([<b>Optionally</b> perform <b>Standalone VM template</b> specific actions])
EEOPTIONAL1([<b>Optionally</b> perform <b>Empty vApp VM</b> specific actions])
EEOPTIONAL2([<b>Optionally</b> perform <b>Empty Standalone VM</b> specific actions])
D --> |Is vApp VM| DDD
D --> |Is Standalone VM| DDDD
E --> |Is vApp VM| EEE
E --> |Is Standalone VM| EEEE
DDD --> DDOPTIONAL1
DDOPTIONAL1 --> DD
DDDD --> DDOPTIONAL2
DDOPTIONAL2 --> DD
EEE --> EEOPTIONAL1
EEOPTIONAL1 --> EE
EEEE --> EEOPTIONAL2
EEOPTIONAL2 --> EE
F[func <b>genericResourceVmCreate</b> \n'Common API calls for ALL VMs' ]
DD --> F
EE --> F
F ==> G
G ==> Z
G[func <b>genericResourceVmCreate</b>\nlast step -> power on]
Z[<b>VM Created</b>]
style DDD fill:#45f248,stroke:#333,stroke-width:4px
style DDDD fill:#45f248,stroke:#333,stroke-width:4px
style EEE fill:#45f248,stroke:#333,stroke-width:4px
style EEEE fill:#45f248,stroke:#333,stroke-width:4px
style F fill:#F1EE8E,stroke:#333,stroke-width:4px
style DD fill:#F1EE8E,stroke:#333,stroke-width:4px
style EE fill:#F1EE8E,stroke:#333,stroke-width:4px
style DDOPTIONAL1 fill:#F1EE8E,stroke:#333,stroke-width:4px
style DDOPTIONAL2 fill:#F1EE8E,stroke:#333,stroke-width:4px
style EEOPTIONAL1 fill:#F1EE8E,stroke:#333,stroke-width:4px
style EEOPTIONAL2 fill:#F1EE8E,stroke:#333,stroke-width:4px
Bug fixes:
- VM from template would inherit a description from vApp if a description is not set in HCL schema. This is not a correct behavior and one would expect an empty description instead. This is now fixed by using
vm.SetDescription
function explicitly.
Extra fixes:
- Functions were converted to use diag.Diagnostics instead of err in some places
- Some test functions use
ComposeAggregateTestCheckFunc
instead ofComposeTestCheckFunc
- this will return all failed checks for a test step instead of the first one (makes troubleshooting faster)
Tests:
- Additional tests which being their names with
TestAccVcdVAppVm_4types*
to denote that all 4 types of VMs are tested in each of these tests.
Note. Many "helper" functions for VM have been moved to the resource_vcd_vapp_vm_tools.go
file so that the main file is less burdened.
Closes #874 , closes #866, closes #864