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

Add cluster asset to launch an openshift cluster #289

Merged
merged 13 commits into from
Sep 23, 2018

Conversation

yifan-gu
Copy link
Contributor

  • Move some helper function from installer/pkg/workflow to pkg/terraform
  • Create a cluster asset that uses the tfvars asset, terraform templates and terraform binary to launch a cluster
  • Add a clutser target in the CLI so users can run the CLI to create a cluster.

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 20, 2018
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 20, 2018
pkg/asset/cluster/cluster.go Outdated Show resolved Hide resolved
return nil, err
}

data, err := ioutil.ReadFile(state)
Copy link
Contributor

Choose a reason for hiding this comment

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

/s/state/stateFile ?

pkg/terraform/terraform.go Show resolved Hide resolved
@yifan-gu
Copy link
Contributor Author

As this PR adding an asset called cluster, I opened another PR #288 for updating the dependency graph.

@yifan-gu yifan-gu force-pushed the run_terraform branch 7 times, most recently from ba13be1 to ee2936b Compare September 21, 2018 22:34
@openshift-ci-robot openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 21, 2018
@yifan-gu yifan-gu changed the title (WIP) Add cluster asset to launch an openshift cluster Add cluster asset to launch an openshift cluster Sep 21, 2018
@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 Sep 21, 2018
@yifan-gu yifan-gu force-pushed the run_terraform branch 5 times, most recently from b57faed to 1734560 Compare September 22, 2018 02:28
@crawford crawford mentioned this pull request Sep 23, 2018
Yifan Gu added 6 commits September 23, 2018 13:22
Previously, the installer binary generates the ignitions for master
and worker nodes, and pass the filename to the terraform.

However, in the new installer binary, we will instead pass the content
of the ignition files to terraform.
If the "ignition_bootstrap" is non-empty in the terraform.tfvars,
then use it as the content for the bootstrap ignition.

This will enable the new installer binary to launch a cluster.
This assumes that we ship the installer binary with the same dir
as the terraform templates.
The cluster asset invokes terraform binary with the terraform templates
and generated tfvars, ignitions, etc to launch a cluster.
Yifan Gu added 7 commits September 23, 2018 13:24
Some default values are set the the tfvars if they are missing.
Add default VPCCIDIR, network type, IfName, IPRange, imageURL.
Also set different default node numbers for different platform.

This is because on libvirt, there's a potential race for the network
setup on libvirt platform

Also ask users for the libvirt image URL.
ValidateAndLog() will help us to catch some early errors on config
generation.
Copy the terraform.tfvars to the working dir for the terraform.
@crawford
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 23, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: crawford, yifan-gu

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

@jlebon
Copy link
Member

jlebon commented Sep 25, 2018

Yeah, I hit that too and opened #316. I worked around it for now by just commenting it out.

wking added a commit to wking/openshift-installer that referenced this pull request Oct 19, 2018
Using Terraform to remove all resources created by the bootstrap
modules.  For this to work, all platforms must define a bootstrap
module (and they all currently do).

This command moves the previous destroy-cluster into a new 'destroy
cluster' subcommand, because grouping different destroy flavors into
sub-commands makes the base command easier to understand.  We expect
both destroy flavors to be long-running, because it's hard to write
generic logic for "is the cluster sufficiently live for us to remove
the bootstrap".  We don't want to hang forever if the cluster dies
before coming up, but there's no solid rules for how long to wait
before deciding that it's never going to come up.  When we start
destroying the bootstrap resources automatically in the future, will
pick reasonable timeouts, but will want to still provide callers with
the ability to manually remove the bootstrap resources if we happen to
fall out of that timeout on a cluster that does eventually come up.

I've also created a LoadMetadata helper to share the "retrieve the
metadata from the asset directory" logic between the destroy-cluster
and destroy-bootstrap logic.  The new helper lives in the cluster
asset plackage close to the code that determines that file's location.

I've pushed the Terraform module unpacking and 'terraform init' call
down into a helper used by the Apply and Destroy functions to make
life easier on the callers.

I've also fixed a path.Join -> filepath.Join typo in Apply, which
dates back to ff5a57b (pkg/terraform: Modify some helper functions
for the new binary layout, 2018-09-19, openshift#289).  These aren't network
paths ;).
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/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants