pkg/asset/cluster: Pull metadata into the Cluster asset #454
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Terraform can die part-way through launching a cluster for many reasons, and we want it to be easy for users to clean up. destroy-cluster gets its teardown information from metadata.json, but before this commit we were only writing the file after a successful Terraform run. That left users with failed Terraform runs scrambling to delete their cluster on their own (e.g. with virsh-cleanup.sh or other external tools). One solution to this problem would be to move the Metadata asset before the Cluster asset in targetAssets. But Abhinav wants: * To provide metadata about the cluster (e.g. bootstrap and master IPs) that is only available after Terraform wraps up. * To minimize the number of assets, so no pre-terraform-metadata.json and post-terraform-metadata.json as separate assets. This commit squashes the metadata file into the Cluster asset, so we can fill it in as we go. I've also updated openshift-install to write any files from failed assets before exiting, so we can land the metadata and recovered Terraform state where the user can find them before we die. To avoid nil-dereference panics after this change, I've also updated a number of Files() implementations to avoid returning a [<nil>] slice. The SilenceError and SilenceUsage additions work around [1]. [1]: spf13/cobra#340
Avoid things like:
$ openshift-install cluster
...
var.tectonic_base_domain
The base DNS domain of the cluster. It must NOT contain a trailing period. Some
DNS providers will automatically add this if necessary.
Example: `openshift.example.com`.
Note: This field MUST be set manually prior to creating the cluster.
This applies only to cloud platforms.
Enter a value:
...
With this commit, we instead get:
$ openshift-install cluster
INFO Fetching OS image...
INFO Using Terraform to create cluster...
ERROR Failed to read tfstate: open /tmp/openshift-install-716419668/terraform.tfstate: no such file or directory
ERROR Error: Required variable not set: tectonic_base_domain
FATAL Error executing openshift-intall: exit status 1
with an introduced mismatch:
--- a/pkg/tfvars/tfvars.go
+++ b/pkg/tfvars/tfvars.go
@@ -17,7 +17,7 @@ import (
type config struct {
ClusterID string `json:"tectonic_cluster_id,omitempty"`
Name string `json:"tectonic_cluster_name,omitempty"`
- BaseDomain string `json:"tectonic_base_domain,omitempty"`
+ BaseDomain string `json:"xtectonic_base_domain,omitempty"`
Masters int `json:"tectonic_master_count,omitempty"`
Workers int `json:"tectonic_worker_count,omitempty"`
Ideally we won't have any mismatches, but it's good to handle them
cleanly when we do instead of allowing the caller to work around them.
Generated with: $ openshift-install graph | dot -Tsvg >docs/design/resource_dep.svg using: $ dot -V dot - graphviz version 2.30.1 (20170916.1124)
openshift-ci-robot
added
approved
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, wking 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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Terraform can die part-way through launching a cluster for many reasons, and we want it to be easy for users to clean up.
destroy-clustergets its teardown information frommetadata.json, but before this commit we were only writing the file after a successful Terraform run. That left users with failed Terraform runs scrambling to delete their cluster on their own (e.g. withvirsh-cleanup.shor other external tools).One solution to this problem would be to move the
Metadataasset before theClusterasset intargetAssets. But @abhinavdahiya wants:To provide metadata about the cluster (e.g. bootstrap and master IPs) that is only available after Terraform wraps up.
To minimize the number of assets, so no
pre-terraform-metadata.jsonandpost-terraform-metadata.jsonas separate assets.This commit squashes the metadata file into the
Clusterasset, so we can fill it in as we go.I've also updated
openshift-installto write any files from failed assets before exiting, so we can land the metadata and recovered Terraform state where the user can find them before we die. To avoid nil-dereference panics after this change, I've also updated a number ofFiles()implementations to avoid returning a[<nil>]slice.The
SilenceErrorandSilenceUsageadditions work around spf13/cobra#340.I've also add
-input=falseto our Terraform invocations, details on that in the 2b5aee7 commit message. I can spin that off into a separate PR if it makes review easier./assign @abhinavdahiya