terraform-provider-vcd icon indicating copy to clipboard operation
terraform-provider-vcd copied to clipboard

Refactor VM "Create" code

Open Didainius opened this issue 2 years ago • 0 comments

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 inside Create 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 of types that Create 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 of ComposeTestCheckFunc - 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

Didainius avatar Aug 16 '22 11:08 Didainius