Skip to content

Conversation

@cuppett
Copy link
Member

@cuppett cuppett commented Mar 13, 2019

This is the start of providing an AWS UPI installation path. It contains CloudFormation templates to help document the basic mechanics of how our installation happens with AWS-native primitives. It also modifies the user doc section a bit to start identifying the major inflection points during an install for further explanation or enhancements in the process.

I've stubbed out the major steps and provided some words, I'll capture more words and examples in subsequent commits either in this PR or additional PRs if this gets merged.

cc @abhinavdahiya @wking

@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 13, 2019
@cuppett
Copy link
Member Author

cuppett commented Mar 13, 2019

/assign @wking

@cuppett
Copy link
Member Author

cuppett commented Mar 13, 2019

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 13, 2019

### Cleanup Machine API Resources

TODO: Identify deleting the pre-populated, disconnected machine entries.
Copy link
Member

Choose a reason for hiding this comment

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

Having this as a post-Install cleanup step is silly. We should just not generate the resource for this install sequence, and let users decide what machines they want the machine api to adopt or mint thereafter.

Copy link
Member

Choose a reason for hiding this comment

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

Having this as a post-Install cleanup step is silly...

I'm having trouble parsing this. What are you saying needs cleaning up?

You'll need to remove the bootstrap resources at this point (after receiving the bootstrap-complete event), but that's not silly, that's just "we're no longer bootstrapping". For machine-API adoption, that's currently the clusterid tag (#479, although I still wish the tag had better namespacing like openshift.io/cluster/{name}: owned or we just used kubernetes.io/cluster/{name}: owned for adoption). If you don't want machines adopted, just don't set that tag.

@cuppett
Copy link
Member Author

cuppett commented Mar 14, 2019

@wking This PR is ready to be merged. I am able to get the cluster up and adjust the worker pool to get a worker to launch. See the pods list in the doc. I think we should merge this and ping different groups to help sort out the changes/tolerances needed for DNS, routing, machine API, etc. for the klunky bits.

I want to add an ASG example worker CloudFormation as well, but can tack it in with a discrete PR.

Okay to merge? I don't know how to remove the hold, etc.

@sdodson
Copy link
Member

sdodson commented Mar 14, 2019

/hold cancel
/approve

Still needs lgtm from reviewer. I haven't tried this myself but reading through the documentation seems clear enough to me.

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 14, 2019
Copy link
Member

Choose a reason for hiding this comment

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

This graphic has "Master Subnet (public) ..." etc., which is no longer the case for IPI AWS since #1045. And as of b29c9736a77 you have 05_cluster_master_nodes.yaml content like:

  Master0Subnet:
    Description: The subnets (recommend private) to launch the master nodes into
    Type: AWS::EC2::Subnet::Id

which suggests your CloudFormation configs are following the new IPI approach and putting all nodes in the private subnets. Can you update your graphic to match?

Copy link
Member

Choose a reason for hiding this comment

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

On the "update the graphic" approach, while I think this graphic is great for communicating the structure, I'm concerned about maintaining it. Can you provide it as an SVG or similar to make it easier for others to edit it going forward? Also, if there's a preferred tool (Inkscape?) for editing it, that would be good to document in the commit message.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated graphic. It's a Dia diagram. I could add the source material to a subdir. Path preference?

Copy link
Member

Choose a reason for hiding this comment

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

Path preference?

Right next to the PNG, as docs/user/aws/images/install_upi_vpc.dia?

@abhinavdahiya
Copy link
Contributor

/cc @kalexand-rh for tracking docs

@openshift-ci-robot
Copy link
Contributor

@abhinavdahiya: GitHub didn't allow me to request PR reviews from the following users: tracking, for.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @kalexand-rh for tracking docs

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Moved the encrypted-AMI section from "Create Cluster" to "Running
Cluster", because it has more value there folks inspecting their
account and what has happened after the fact.  Since we call out the
running instances (by count with a picture) they may wonder "where did
this unique AMI come from it is running"?  It goes along with some of
the other explanations we chase with as well.  The Create Cluster
section just has the whole IPI output, but no explanation of all
wizardry under the covers, calling this particular wrinkle out there
seemed a bit awkward to me.
@wking
Copy link
Member

wking commented Mar 18, 2019

@cuppett is out this week so I've picked this up and force-pushed to address some of the remaining review. Still unresolved:

nodes. It can also be inspected for the set of required attributes needed for manual creation of the nodes, DNS entries
and load balancer configuration.

## Monitor for `bootstrap-complete` and Initialization
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember @wking recommending no using ticks in titles.

file to an S3 bucket in your account and run this template to get a bootstrap node along with a predictable clean up of
the resources when complete. It can also be inspected for the set of required attributes via manual creation.

## Launch Permanent Master Nodes
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Control Plane Nodes

@abhinavdahiya
Copy link
Contributor

/lgtm

Let's merge this and iterate on this.

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, cuppett, sdodson

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

The pull request process is described here

Details 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-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 18, 2019
@openshift-merge-robot openshift-merge-robot merged commit 9777df0 into openshift:master Mar 18, 2019
wking added a commit to wking/origin that referenced this pull request Mar 20, 2019
awscli will allow us to add CI for user-provided infrastructure, as
documented by openshift/installer@39a926a918 (Adding initial user
doc/guide & materials for UPI AWS installation, 2019-03-12,
openshift/installer#1408).

jq will allow us to simplify the templating which landed in
openshift/release@7e56379d0b (templates/openshift: grab bootstrap log
on failure, 2019-01-14, openshift/release#2633).
@cuppett cuppett deleted the cuppett/upi/aws branch April 3, 2019 09:14
wking added a commit to wking/openshift-installer that referenced this pull request Apr 5, 2019
Dia is from Stephen Cuppett, replacing the PNG he'd submitted via
39a926a (Adding initial user doc/guide & materials for UPI AWS
installation, 2019-03-12, openshift#1408).

We aren't using the full file with all the layers yet, but I'm
building it anyway because folks without Dia may still want to look at
it ;).

SVGs generated with:

  $ dia --version
  Dia version 0.97.3, compiled 18:02:21 Feb 11 2017

relink-dia.py embeds the referenced icons in the SVG with def and use
[1,2] to avoid icon URIs like:

  file:///.../openshift/installer/docs/user/aws/images/./AWS-Architecture-Icons_SVG/Light-BG/_General%20AWS/AWS-General_AWS-Cloud_light-bg.svg

Ideally Dia would have a way to do this sort of thing automatically
with a command-line switch, but if it does, I can't find it.

[1]: https://developer.mozilla.org/en-US/docs/Web/SVG/Element/def
[2]: https://developer.mozilla.org/en-US/docs/Web/SVG/Element/use
wking added a commit to wking/openshift-installer that referenced this pull request Aug 30, 2019
We've had this since the templates landed in 39a926a (Adding
initial user doc/guide & materials for UPI AWS installation,
2019-03-12, openshift#1408).  But our Terraform modules contain no analog using
aws_network_acl [1] and their presence in the UPI templates is
breaking the ping-based network connectivity test [2].

[1]: https://www.terraform.io/docs/providers/aws/r/network_acl.html
[2]: https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/logs/canary-openshift-ocp-installer-e2e-aws-upi-4.2/28#0:build-log.txt%3A10235
jhixson74 pushed a commit to jhixson74/installer that referenced this pull request Dec 6, 2019
We've had this since the templates landed in 39a926a (Adding
initial user doc/guide & materials for UPI AWS installation,
2019-03-12, openshift#1408).  But our Terraform modules contain no analog using
aws_network_acl [1] and their presence in the UPI templates is
breaking the ping-based network connectivity test [2].

[1]: https://www.terraform.io/docs/providers/aws/r/network_acl.html
[2]: https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/logs/canary-openshift-ocp-installer-e2e-aws-upi-4.2/28#0:build-log.txt%3A10235
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