Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Sep 27, 2018

Builds on #351; review that first.

The old examples/aws.yaml location is no longer discoverable for folks using the new installer.

Tables are annoying to maintain in ASCII, and lists give us more space
to talk about the expected input format, etc.
@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 27, 2018
@wking wking force-pushed the aws-domain-comment-for-next-gen-docs branch from b8e2588 to 9324f35 Compare September 27, 2018 19:17
@wking
Copy link
Member Author

wking commented Sep 27, 2018

Example prompt output (if you use ? for help):

$ ./openshift-install install-config
? Email Address [email protected]
? Password [? for help] ****
? SSH Public Key <none>
???? The base domain of the cluster. All DNS records will be sub-domains of this base.

For AWS, this must be a previously-existing public Route 53 zone.  You can check for any already in your account with:

  $ aws route53 list-hosted-zones --query 'HostedZones[? !(Config.PrivateZone)].Name' --output text
? Base Domain 

@abhinavdahiya
Copy link
Contributor

This might be useful in docs; but not sure if this is required in help for prompt.

@wking
Copy link
Member Author

wking commented Sep 27, 2018

This might be useful in docs; but not sure if this is required in help for prompt.

I expect folks reading environment-variable docs will be mostly power-users who have gotten tired of the interactive prompts, and the advice is most useful to fist-timers. Also, you only see the prompt help if you hit ?. But I can remove it from there if you like. Thoughts?

@dak1n1
Copy link

dak1n1 commented Sep 27, 2018

Hey guys, thanks so much for working on these docs. Here is some feedback, in case that's helpful.

As someone with more of a systems / Ops background, I went straight for the env variables, rather than ever wanting to be prompted. The reason for this is that I write down the settings of every install attempt when I'm figuring out new software, and I like to be able to reproduce installs with a quick copy/paste, avoiding interactive steps whenever possible.

I started at https://github.com/openshift/installer/blob/master/README.md and followed the links from there to the env variables list. So the additions you made to the env vars list are really helpful to me.

The old examples/aws.yaml location is no longer discoverable for folks
using the new installer.
@wking wking force-pushed the aws-domain-comment-for-next-gen-docs branch from 9324f35 to fa7c9e3 Compare September 27, 2018 20:12
Copy link

@dak1n1 dak1n1 left a comment

Choose a reason for hiding this comment

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

Looks great! (Though you might want to get someone from the installer team to +1 as well)

@crawford
Copy link
Contributor

crawford commented Oct 2, 2018

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: crawford, wking

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-merge-robot openshift-merge-robot merged commit 49db3a9 into openshift:master Oct 2, 2018
@wking wking deleted the aws-domain-comment-for-next-gen-docs branch October 2, 2018 17:11
wking added a commit to wking/openshift-installer that referenced this pull request Dec 19, 2018
This used to be covered in the docs from fa7c9e3 (*: Copy route53
baseDomain advice to openshift-install locations, 2018-09-27, openshift#353),
but in order to see those you'd need to have a moment of doubt and
think to hit '?'.  Even if you read the docs, it's possible you'd typo
the base domain or add a trailing period (theoretically trailing
periods would be fine, but they may have some issues at the moment
[1]).

With this commit, we go ahead and fetch available public zones
ourselves, so AWS users don't have to.  And it also reduces the help
noise on the base-domain input for users targeting non-AWS platforms.

The empty struct map is slightly more efficient than a boolean map,
because the empty struct takes up no space [2].  Although it's hard to
imagine an account with enough public zones for that space savings to
be significant.

The IsForbidden handling lets us fall back to the free-form input if
we aren't authorized to list zones for the select widget:

  $ openshift-install --dir=wking create install-config
  ? SSH Public Key <none>
  ? Platform aws
  ? Region us-west-1
  ERROR list hosted zones: AccessDenied: User: arn:aws:iam::...:user/trking is not authorized to perform: route53:ListHostedZones with an explicit deny
          status code: 403, request id: 1d..29
  ? Base Domain [? for help]

[1]: openshift#831 (comment)
[2]: https://dave.cheney.net/2014/03/25/the-empty-struct
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/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants