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

Transition to Terraform 0.12 for installer #1739

Merged
merged 17 commits into from
May 16, 2019

Conversation

abhinavdahiya
Copy link
Contributor

@abhinavdahiya abhinavdahiya commented May 10, 2019

Terraform community is getting ready for 0.12 release that introduces wide variety of improvements in the language
and fixes a lot of the subtle plan and apply bugs. see here 1 for official announcement

New platforms like Azure require that installer moves to 0.12 version of terraform as it provides the required dynamic blocks in HCL2 2. For example,
Azure dns_srv record requires use of foreach to create dynamic count of records for control plane 3. RHHI platform owners have also requested 0.12 terraform version to
simplify their terraform templates. Therefore, moving the installer to 0.12-rc.1 sets us up to switch to final release of 0.12 which is very close 4.

NOTE: This PR is restricted to the minimum set of changes that are required for use to move to 0.12. All work to utilize the new hcl2 specification for current templates is out of scope for this and can be done as a follow-up.

NOTE: I have also left the terraform templates in the upi directory for vsphere and metal as is, as we can migrate them to 0.12 at a later stage.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 10, 2019
@abhinavdahiya
Copy link
Contributor Author

using this to get initial testing feedback.

@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 10, 2019
@abhinavdahiya abhinavdahiya force-pushed the terraform_0.12 branch 9 times, most recently from f2f289f to 806257a Compare May 14, 2019 00:00
@praveenkumar
Copy link
Contributor

/test e2e-libvirt

@abhinavdahiya
Copy link
Contributor Author

/retest

@abhinavdahiya
Copy link
Contributor Author

/test e2e-libvirt

Terraform community is getting ready for 0.12 release that introduces wide variety of improvements in the language
and fixes a lot of the subtle plan and apply bugs. see here [1] for official announcement

New platforms like Azure require that installer moves to 0.12 version of terraform as it provides the required `dynamic` blocks in HCL2 [2]. For example,
Azure `dns_srv` record requires use of `foreach` to create dynamic count of records for control plane [3]. `RHHI` platform owners have also requested 0.12 terraform version to
simplify their terraform templates. Therefore, moving the installer to 0.12-rc.1 sets us up to switch to final release of 0.12 which is very close [4].

This commit,
- moves the vendored copy to v0.12.0-rc1 [5] tag revision.
- expands the list of required packages from terraform helper packages as these are required by newer terraform-providers
- dep is sadly unsuccesful in understanding the correct version for the indirect dependencies, and therefore it was required to add overrides for faulty vendored packages based on upstream go.mod [6] ang go.sum [7]

[1]: https://www.hashicorp.com/resources/introducing-terraform-0-12
[2]: https://www.terraform.io/upgrade-guides/0-12.html#attributes-vs-blocks
[3]: https://github.com/openshift/installer/pull/1454/files#diff-764adb23dcc0edbbebc09192eb233e9aR46
[4]: https://github.com/hashicorp/terraform/pulls?q=is%3Aopen+is%3Apr+milestone%3Av0.12.0
[5]: https://github.com/hashicorp/terraform/releases/tag/v0.12.0-rc1
[6]: https://github.com/hashicorp/terraform/blob/v0.12.0-rc1/go.mod
[7]: https://github.com/hashicorp/terraform/blob/v0.12.0-rc1/go.sum
@abhinavdahiya abhinavdahiya changed the title WIP: Transition to Terraform 0.12 for installer Transition to Terraform 0.12 for installer May 14, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 14, 2019
@abhinavdahiya
Copy link
Contributor Author

/test e2e-libvirt

The `terraform 0.12upgrade` commands left certain template file in a little undesired format. This commit uses combination of manually fixing the formating and `terraform fmt -recursive .` to fix the formatting for templates.
…g lists

Terraform 0.12 language no longer performs the float -> int conversion when indexing on lists. floor [2] function should be
used to explicity convert float to int.

[1]: https://www.terraform.io/upgrade-guides/0-12.html#integer-vs-float-number-types
[2]: https://www.terraform.io/docs/configuration/functions/floor.html
With addition of first-class expressions [1] to language in 0.12, list no longer need to be wrapped in `[]`.
As a side-effect, wrapping lists creates a 2-d list and fails validation of a list of string for variables. Therefore, all outputs and variables that are already lists were updated to [2]

[1]: https://www.terraform.io/upgrade-guides/0-12.html#first-class-expressions
[2]: https://www.terraform.io/upgrade-guides/0-12.html#referring-to-list-variables
Terraform 0.12 adds dynamic blocks [1] [2] to the language specification. This commit updates the network
resource to use dynamic blocks for srvs and hosts in dns block.

[1]: https://www.terraform.io/upgrade-guides/0-12.html#attributes-vs-blocks
[2]: https://www.hashicorp.com/blog/hashicorp-terraform-0-12-preview-for-and-for-each
With terraform 0.12 `init` behavior has changed to re-use the state file when initializing. With the previous override directory set to the working directory of terraform, init would assume initializing has already occured and fail to load the state file, when it should not try to load the state file.

Moving the override directory to `<working dir>/.tf` forces terraform to correctly initialize
@abhinavdahiya
Copy link
Contributor Author

/test e2e-libvirt

abhinavdahiya added a commit to abhinavdahiya/release that referenced this pull request May 15, 2019
Installer [1] is in progress to move to terraform version 0.12, which has breaking changes for formatting.
To add to that, `upi` terraform templates will be moved over to 0.12 at a later stage.

Therefore marking this optional for the time being until we have moved entire code base to 0.12.

[1]: openshift/installer#1739
@abhinavdahiya
Copy link
Contributor Author

/test e2e-aws-upgrade

@abhinavdahiya
Copy link
Contributor Author

/retest

1 similar comment
@abhinavdahiya
Copy link
Contributor Author

/retest

@abhinavdahiya
Copy link
Contributor Author

Hoping this was a flake e2e-aws

Failing tests:

[sig-storage] In-tree Volumes [Driver: nfs] [Testpattern: Inline-volume (ext4)] volumes should allow exec of files on the volume [Suite:openshift/conformance/parallel] [Suite:k8s]

Writing JUnit report to /tmp/artifacts/junit/junit_e2e_20190516-053734.xml

/retest

@praveenkumar
Copy link
Contributor

libvirt succeed finally 👍

@zeenix
Copy link
Contributor

zeenix commented May 16, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 16, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, zeenix

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 1b3d3fe into openshift:master May 16, 2019
@abhinavdahiya
Copy link
Contributor Author

🥳🥳

@abhinavdahiya abhinavdahiya deleted the terraform_0.12 branch May 22, 2019 16:11
wking added a commit to wking/openshift-installer that referenced this pull request Oct 1, 2019
The zone-count variables date back to f828666 (modules/vpc: support
re-apply of terraform when AZ number changes, 2018-03-12,
coreos/tectonic-installer#3092).  But with Terraform 0.12, which we've
used since 64c44cd (terraform: bump the vendored version to
0.12-rc.1, 2019-05-14, openshift#1739), we have better array handling, and no
longer need count variables.

Similarly, there's no need for vpc_id, when we can extract that ID
from data.aws_vpc.cluster_vpc.
jhixson74 pushed a commit to jhixson74/installer that referenced this pull request Dec 6, 2019
The zone-count variables date back to f828666 (modules/vpc: support
re-apply of terraform when AZ number changes, 2018-03-12,
coreos/tectonic-installer#3092).  But with Terraform 0.12, which we've
used since 64c44cd (terraform: bump the vendored version to
0.12-rc.1, 2019-05-14, openshift#1739), we have better array handling, and no
longer need count variables.

Similarly, there's no need for vpc_id, when we can extract that ID
from data.aws_vpc.cluster_vpc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants