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

Azure Virtual Machine Support #16

Merged
merged 8 commits into from
Jan 9, 2023

Conversation

bryan-bar
Copy link
Collaborator

@bryan-bar bryan-bar commented Jan 5, 2023

Azure Virtual Machine Support Changes:

  • terraform version >= 1.3.6 required
  • agreement resource added to confirm operating system agreements automatically
  • var.additional_volumes created in machine module so that validation can be used
  • added support for zones
  • use of regions without zones supported
    • zone is passed in as null to the machine module when assigned to 0
  • firewall support for public access and regional subnet access
  • region peering support
  • validations a mix of variable validation, lifestyle preconditions and provider's default checks
    • Preferably any invalid configurations are caught during terraform plan but provider implementation varies
    • contains fails during terraform plan if given a null value
      • use try(contains(...), boolean) as a workaround when null value needs to be checked when not null
        • shortcircuit evalution does not work here: variable == null || contains([...], variable) will fail
  • provider resource uses prevent_deletion_if_contains_resources = false

* agreement resource added to confirm agreements automatically
* var.additional_volumes created in machine module so that validation can be used
* added support for zones
  * not all regions support zones but certain services require zones
    * List of regions with zones: https://learn.microsoft.com/en-us/azure/reliability/availability-zones-service-support
    * use of zoned virtual machines requires a zoned public ip with standard sku/static ip or results in error due to not being in the same zone
  * public ip must be standard sku and static ip to be used with a zone
  * public ip basic sku or dynamic ip will not work with a vm that has a zone
* firewall support for public access and regional subnet access
* use of regions without zones supported
  * zone is passed in as null to the machine module when assigned to 0
* optional requires terraform >= 1.3.6
    * when omitted, optional returns null or can be assigned a static value
    * contains fails during `terraform plan` if given a null value
      * use `try(contains(...), boolean)` as a workaround when null value needs to be checked when not null
        * shortcircuit evalution does not work here: `variable == null || contains([...], variable)` will fail
* Testing of validation/optional vs pre/post-conditions
* variables azs and az changed to zones and zone
* additional use of validations for required objects
  * due to limitiation of validations, pre-conditions might be better used with similar abilities to catch errors during plan and to be able to use locals instead of manual definitions
* removed unneeded validations that are properly handled by providers resources
  * preferably any invalid configurations are caught during `terraform plan` but provider implementation varies
* added precondition for region-zone in network module
* added precondition in machine module for premium ssd availability
@bryan-bar bryan-bar marked this pull request as ready for review January 5, 2023 01:10
Copy link
Collaborator

@jt-edb jt-edb left a comment

Choose a reason for hiding this comment

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

Nice work! Only 2 minor comments.

edbterraform/data/templates/azure/network.tf.j2 Outdated Show resolved Hide resolved
edbterraform/data/templates/azure/providers.tf.j2 Outdated Show resolved Hide resolved
* removed commented out code
* removed unused routes module
* moved template `agreements.tf` to `agreements.tf.j2` and included in `main.tf.j2`
* machine module uses a machine object variable with region and zone keys already. `var.region` / `var.zone` not needed.
  * output references its resource's zone instead of zone variable
* network module should be self-contained and not reference infrastructure.yml file expectations within errors (zone = 0)
  * Validation module will handle these checks
@bryan-bar bryan-bar requested a review from jt-edb January 6, 2023 21:07
Copy link
Collaborator

@jt-edb jt-edb left a comment

Choose a reason for hiding this comment

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

LGTM

@jt-edb jt-edb merged commit dcbd4fb into EnterpriseDB:main Jan 9, 2023
@bryan-bar bryan-bar deleted the azure-virtual-machines branch January 20, 2023 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants