Skip to content

Conversation

@cuppett
Copy link
Member

@cuppett cuppett commented Jan 9, 2019

This PR adds user documentation for AWS account configuration and installation to the repo.

Original document (for those with access) located here:

https://docs.google.com/document/d/1KSnNtFFtMDosX_dN7fgTTSvFezxJumC6leIIXWu616U/edit?usp=sharing

@crawford @eparis

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 9, 2019
@cuppett
Copy link
Member Author

cuppett commented Jan 9, 2019

/assign crawford

@wking
Copy link
Member

wking commented Jan 9, 2019

There are lots of PNGs here. Whose on the hook to keep those up to date ;) ? Can we replace some or all of them with text that's easier to maintain?

@wking
Copy link
Member

wking commented Jan 9, 2019

We probably want a link to this from the README (like we have for libvirt here) to make these more discoverable.

@cuppett
Copy link
Member Author

cuppett commented Jan 9, 2019

There are lots of PNGs here. Whose on the hook to keep those up to date ;) ? Can we replace some or all of them with text that's easier to maintain?

We can definitely winnow them down over time. The idea is to port in the existing doc in for these early drops.

Copy link
Member

Choose a reason for hiding this comment

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

Is this true? What are the 21 for? Three masters with public IPs, and...?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is. They may be occupied by the ALB targets, but they are assigned to ELBs. Also, one per instance is attached. I don't know why, private IPs assigned in the subnet via DHCP are permanent for the lifetime of the node (regardless of stops/starts). Usually, these are created so the same IP can be given/reassigned to another instance; I don't think this is a problem we have.

@cuppett
Copy link
Member Author

cuppett commented Jan 9, 2019

We probably want a link to this from the README (like we have for libvirt here) to make these more discoverable.

Done. Repushed with this.

Copy link
Contributor

Choose a reason for hiding this comment

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

i'd personally add a note saying that using sts doesn't work. you can see more examples of failure here or in the issues section

Copy link
Member

Choose a reason for hiding this comment

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

i'd personally add a note saying that using sts doesn't work...

I'm fine with this. But note that the credentials operator is coming soon which will make it a lot easier to guard against improper-cred issues, so it's probably not worth sinking too much time into stopgap docs.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather keep this user guide as a "straight shot" guide ATM. Introducing other wrinkles may make this a lot more complicated than we'd want in this particular spot.

Copy link
Contributor

Choose a reason for hiding this comment

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

fair point

@cuppett
Copy link
Member Author

cuppett commented Jan 9, 2019

Pushed in removal of additional non-ASCII quotes.

@cuppett
Copy link
Member Author

cuppett commented Jan 10, 2019 via email

@cuppett
Copy link
Member Author

cuppett commented Jan 10, 2019

/test tf-fmt

@cuppett
Copy link
Member Author

cuppett commented Jan 10, 2019

/test images

@cuppett
Copy link
Member Author

cuppett commented Jan 10, 2019

/test e2e-aws

@wking
Copy link
Member

wking commented Jan 10, 2019

I've pushed d38b4e2 with a few more minor pivots. You can pull it in with:

$ git pull https://github.com/wking/openshift-installer.git aws-doc-fixups
$ git rebase -i --autosquash HEAD^^

or similar. Feel free to ignore any of those changes as you see fit ;).

@cuppett
Copy link
Member Author

cuppett commented Jan 10, 2019

I've pushed d38b4e2 with a few more minor pivots. You can pull it in with:

$ git pull https://github.com/wking/openshift-installer.git aws-doc-fixups
$ git rebase -i --autosquash HEAD^^

or similar. Feel free to ignore any of those changes as you see fit ;).

All taken. Thanks!

@cuppett
Copy link
Member Author

cuppett commented Jan 10, 2019

/test e2e-aws

@wking
Copy link
Member

wking commented Jan 10, 2019

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cuppett, 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-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 10, 2019
@cuppett
Copy link
Member Author

cuppett commented Jan 10, 2019

/test e2e-aws

1 similar comment
@cuppett
Copy link
Member Author

cuppett commented Jan 10, 2019

/test e2e-aws

@cuppett
Copy link
Member Author

cuppett commented Jan 11, 2019

/test images

@cuppett
Copy link
Member Author

cuppett commented Jan 11, 2019

/test e2e-aws

@cuppett
Copy link
Member Author

cuppett commented Jan 11, 2019

/test images

@cuppett
Copy link
Member Author

cuppett commented Jan 11, 2019

/test e2e-aws

5 similar comments
@cuppett
Copy link
Member Author

cuppett commented Jan 11, 2019

/test e2e-aws

@cuppett
Copy link
Member Author

cuppett commented Jan 11, 2019

/test e2e-aws

@cuppett
Copy link
Member Author

cuppett commented Jan 11, 2019

/test e2e-aws

@cuppett
Copy link
Member Author

cuppett commented Jan 11, 2019

/test e2e-aws

@cuppett
Copy link
Member Author

cuppett commented Jan 12, 2019

/test e2e-aws

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@wking
Copy link
Member

wking commented Jan 13, 2019

Freeze stuff all landed.

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit a63e50e into openshift:master Jan 13, 2019
@cuppett cuppett deleted the cuppett/add-aws-doc branch January 14, 2019 12:18
wking added a commit to wking/openshift-installer that referenced this pull request Jan 18, 2019
Fixing an omission from b383cd7 (Adding user doc/guide for AWS
account and installation, 2019-01-09, openshift#1030).

While I'm doing pedantic things at the end of the file, also
alphabetize the link defintions.
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/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants