Adapt installer to use Terraform 0.10.* #1841
Conversation
|
Close / re-open to trigger tests |
installer/api/handlers_terraform.go
Outdated
| @@ -175,12 +177,27 @@ func newExecutorFromApplyHandlerInput(input *TerraformApplyHandlerInput) (*terra | |||
| } | |||
| exPath := filepath.Join(binaryPath, "clusters", clusterName+time.Now().Format("_2006-01-02_15-04-05")) | |||
|
|
|||
| // Publish custom providers to execution environment | |||
| clusterPluginDir := filepath.Join(exPath, "terraform.d/plugins") | |||
There was a problem hiding this comment.
consider making this a var at the top of the file so it is clear it is a canonical value.
Makefile
Outdated
| TF_CMD = TERRAFORM_CONFIG=$(TF_RC) terraform | ||
| TF_CMD = terraform | ||
|
|
||
| PROVIDER_MATCHBOX_VER = v0.2.2 |
There was a problem hiding this comment.
I wish we didn't need to spec this so many places. We should make an internal not about all the places to bump this so it's easier when the time comes.
There was a problem hiding this comment.
It's a bit misleading. The only relevant place for this version is actually here in the Makefile.
The release scripts have been relocated to their own repo and the versions in here are to be removed. I'm waiting for @Quentin-M to give the green light on that.
In the meantime I just updated everywhere for consistency.
Makefile
Outdated
| TF_CMD = TERRAFORM_CONFIG=$(TF_RC) terraform | ||
| TF_CMD = terraform | ||
|
|
||
| PROVIDER_MATCHBOX_VER = v0.2.2 |
There was a problem hiding this comment.
s/VER/VERSION/ we can be explicit in configuration like this and it is more consistent with our other files.
| "${REPOSITORY_ROOT}/examples" | ||
| ) | ||
|
|
||
| export PROVIDER_MATCHBOX_VER=v0.2.2 |
…ugins each bring their own CLI interface).
|
@squat I fixed the nits (hopefully). PTAL. |
| // Wait for its termination. | ||
| select { | ||
| case <-done: | ||
| case <-time.After(30 * time.Second): |
There was a problem hiding this comment.
not a show-stopper, but raising awareness, that this could be a potential flake. Looking at the code for this test, terraform init downloads provider.local and provider.template for the tested main.tf file, hence we are dependent on the network.
In practice I did see terraform init failures locally due to temporarily unreachable endpoints, so I am predicting we will see it here too.
There was a problem hiding this comment.
This UI only code for now.
I agree that we need to do smarter things here (like retries) once this get on the critical path of any automation.
| @@ -18,13 +18,15 @@ export TECTONIC_RELEASE_TOP_DIR="$TECTONIC_RELEASE_DIR/tectonic" | |||
| export INSTALLER_RELEASE_DIR="$TECTONIC_RELEASE_TOP_DIR/tectonic-installer" | |||
|
|
|||
| export TERRAFORM_BIN_TMP_DIR="$TMP_DIR/terraform-bin" | |||
| export TERRAFORM_BIN_VERSION=0.9.6 | |||
| export TERRAFORM_BIN_VERSION=0.10.3 | |||
There was a problem hiding this comment.
super small nit: terraform is at 0.10.4 already.
| "${REPOSITORY_ROOT}/examples" | ||
| ) | ||
|
|
||
| export PROVIDER_MATCHBOX_VERSION=v0.2.2 |
There was a problem hiding this comment.
nit: we are declaring this version twice. here and here. can we have one place for this var?
There was a problem hiding this comment.
These release scripts have been moved out into their own repo so this version will soon be removed and is not in use anymore. I've just updated for consistency, but this is dead code already. Just waiting for Quentin to give the go-ahead to remove these.
tests/smoke/bare-metal/smoke.sh
Outdated
| @@ -30,7 +30,7 @@ BIN_DIR="$ROOT/bin_test" | |||
|
|
|||
| MATCHBOX_VERSION=v0.6.1 | |||
| KUBECTL_VERSION=v1.6.4 | |||
| TERRAFORM_VERSION=0.9.6 | |||
| TERRAFORM_VERSION=0.10.3 | |||
There was a problem hiding this comment.
same argument as above. can we unify this and maintain one place used for the tf version? it is also declared here.
There was a problem hiding this comment.
This version of the smoke test is in the refactoring pipeline to move to RSpec already.
The refactored version will use different configuration conventions, so I guess it's not worth refactoring now.
| # THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. | ||
| # yarn lockfile v1 | ||
|
|
||
|
|
There was a problem hiding this comment.
Did someone run yarn in the wrong directory, then check this file in?
| "lockfileEntries": {}, | ||
| "files": [], | ||
| "artifacts": {} | ||
| } |
There was a problem hiding this comment.
Ditto here. It looks like someone ran yarn install in the installer dir instead of frontend/.
|
Might have been me an my abysmal JS skills.
I'll clean them up.
…On Wed 13. Sep 2017 at 19:55, Geoff Greer ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In installer/node_modules/.yarn-integrity
<#1841 (comment)>
:
> @@ -0,0 +1,8 @@
+{
+ "flags": [],
+ "linkedModules": [],
+ "topLevelPatters": [],
+ "lockfileEntries": {},
+ "files": [],
+ "artifacts": {}
+}
Ditto here. It looks like someone ran yarn install in the installer dir
instead of frontend/.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#1841 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABC-myurU0SKHL1stcVPY0xYHwEyUV6dks5siBbygaJpZM4PNQCx>
.
--
This email and any files transmitted with it are confidential and intended
solely for the use of the individual or entity to whom they are addressed.
This message contains confidential information and is intended only for the
individual named. If you are not the named addressee you should not
disseminate, distribute or copy this e-mail. Please notify the sender
immediately by e-mail if you have received this e-mail by mistake and
delete this e-mail from your system. If you are not the intended recipient
you are notified that disclosing, copying, distributing or taking any
action in reliance on the contents of this information is strictly
prohibited.
|
These changes adapt the installer for the slightly different workflows introduced by Terraform 0.10.x
Notable changes:
$(BUILD_DIR)/terraform.d/plugins(as opposed to the old.terraformrc)terraform-provider-matchboxthat we use for bare-metal installation is fetched from it's github releases and placed next to the installer binary at BUILD time (all other steps assume it to be there).Possibly the release scripts need to be adapted, I'll leave that for @Quentin-M to decide.
/cc: @sym3tri