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

Update some readme's #2031

Merged
merged 1 commit into from Mar 22, 2017
Merged

Update some readme's #2031

merged 1 commit into from Mar 22, 2017

Conversation

anandkumarpatel
Copy link

@anandkumarpatel anandkumarpatel commented Mar 2, 2017

  • update link to IAM user info
  • fix trailing spaces

This change is Reviewable

@k8s-ci-robot
Copy link
Contributor

Hi @anandkumarpatel. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

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. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 2, 2017
@justinsb
Copy link
Member

justinsb commented Mar 2, 2017

The first two changes look good, but I think you're running an older version of kops. At least the --dns flag should be in 1.5.1

docs/aws.md Outdated
@@ -182,7 +182,7 @@ Kops by default will assume that the NS records created above are publicly avail
Note: There is a DNS flag that can be configured if you plan on using private DNS records

```
kops create cluster --dns private $NAME
kops create cluster --dns-zone private $NAME
Copy link
Member

Choose a reason for hiding this comment

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

It is actually --dns

docs/aws.md Outdated
Also kops ships with a handy validation tool that can be ran to ensure your cluster is working as expected.

```bash
kops validate cluster
Copy link
Member

Choose a reason for hiding this comment

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

This does ship with kops

@@ -12,7 +12,7 @@ export CLUSTER_NAME=<sharedvpc.mydomain.com>
export VPC_ID=vpc-12345678 # replace with your VPC id
export NETWORK_CIDR=10.100.0.0/16 # replace with the cidr for the VPC ${VPC_ID}

kops create cluster --zones=us-east-1b --name=${CLUSTER_NAME} --vpc=${VPC_ID}
kops create cluster --zones=us-east-1b --name=${CLUSTER_NAME} --vpc=${VPC_ID} --network-cidr=${NETWORK_CIDR}
Copy link
Member

Choose a reason for hiding this comment

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

This is now optional

@anandkumarpatel
Copy link
Author

ahh I see, I am using kops that ships with brew. Will update and try again.

@justinsb
Copy link
Member

@anandkumarpatel do you want to pursue this PR? Might be easier to close & submit a new one...

PS Thank you for contributing either way :-)

@anandkumarpatel
Copy link
Author

@justinsb just merged with master and it looks like most of my changes were already fixed :) I just have a small nit I updated now.

Copy link
Contributor

@yissachar yissachar left a comment

Choose a reason for hiding this comment

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

/lgtm

@yissachar
Copy link
Contributor

@k8s-bot ok to test

@justinsb justinsb merged commit 12f2a52 into kubernetes:master Mar 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants