-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Changes on CoreOS related documentation. #3205
Conversation
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
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. |
Hi @tigerlinux. 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 I understand the commands that are listed here. 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. |
Hi, |
--master-zones=us-east-1a,us-east-1b,us-east-1c \ | ||
--zones=us-east-1a,us-east-1b,us-east-1c \ | ||
--node-count=2 \ | ||
${NAME} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use the flag --image
in one please and avoid using kops edit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Changed all related stuff and explained the ways to use --image (with either the name of the ami)
Looks great - thanks @tigerlinux . I did have a few suggestions, but I really like the idea of having more "hands on" documentation. Want to go through the suggestions and see if you agree with them? /ok-to-test |
docs/images.md
Outdated
|
||
> Note: SSH username will be `core` | ||
As part of our documentation, you will find a practical exercise using CoreOS with KOPS. See the file "coreos-kops-tests-multimaster.md" in the "examples" directory. This exercise covers not only using kops with CoreOS, but also a practical view of KOPS with a multi-master kubernetes setup. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can make this a link, I think the syntax is [practical exercise using CoreOS with kops](examples/coreos-kops-tests-multimaster.md)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done !
|
||
- AWS cli fully configured (aws account already with proper permissions/roles needed for kops). Depending on your distro, you can setup directly from packages, or if you want the most updated version, use "pip" and installed by issuing a "pip install awscli" command. Your choice !. | ||
- Local ssh key ready on ~/.ssh/id_rsa / id_rsa.pub. You can generate it using "ssh-keygen" command: `ssh-keygen -t rsa -f ~/.ssh/id_rsa -P ""` | ||
- Region set to us-east-1 (az's: us-east-1a, us-east-1b, us-east-1c, us-east-1d and us-east-1e). For this exercise we deployed our cluster on US-EAST-1. If you want to use another region, feel free to change it. Just ensure to have at least 3 availability zones. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of "Just ensure to have at least 3 availability zones" how about "It is recommended to pick a region with at least 3 availability zones, for true high-availability".
Some regions only have 2 AZs; we support multi master in those regions, but it isn't as HA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done !. Explained and included some links from AWS global infra.
```bash | ||
export AWS_ACCESS_KEY_ID=`grep aws_access_key_id ~/.aws/credentials|awk '{print $3}'` | ||
export AWS_SECRET_ACCESS_KEY=`grep aws_secret_access_key ~/.aws/credentials|awk '{print $3}'` | ||
echo "$AWS_ACCESS_KEY_ID $AWS_SECRET_ACCESS_KEY" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of this, you should just set export AWS_PROFILE=nameofprofile
if you're not using the default profile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed and explained the two ways (using the default profile, and when is not being used)
Create a bucket (if you don't already have one) for your cluster state: | ||
|
||
```bash | ||
aws s3api create-bucket --bucket kops-tigerlinux-cluster-state --region us-east-1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine, but probably a good idea to use example
and example.com
instead of your name in future :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done !
Then export the name of your cluster along to the "S3" URL of your bucket: | ||
|
||
```bash | ||
export NAME=coreosbasedkopscluster.k8s.local |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You typically don't need to set NAME
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are consistent with "https://github.com/kubernetes/kops/blob/master/docs/aws.md", we should do it.
--filters "Name=virtualization-type,Values=hvm" "Name=name,Values=CoreOS-stable*" \ | ||
--query 'sort_by(Images,&CreationDate)[-1].{id:ImageLocation}' \ | ||
--output table | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given this command doesn't use the AMI we just found here? Or should we remove the previous way of querying? I worry it's confusing to find an AMI using the CoreOS JSON file, but then we don't use it...
I do like your way though!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include both ways !. You never now when CoreOS page will be available to use the "json".
${NAME} | ||
``` | ||
|
||
NOTE: Remember ${NAME} was exported with our cluster name: coreosbasedkopscluster.k8s.local |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be clearer if you set NAME just before you do kops create cluster
, because then we differentiate between the things that are one-off and the things that are per-cluster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, can you add a note that by using .k8s.local
you're using "gossip dns" and avoiding the need for a route53 domain and a real domain name!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Also I included a mention to the "aws.md" documentation.
kops edit ig --name=${NAME} master-us-east-1c | ||
``` | ||
|
||
What you need to change on all your instance groups (masters and nodes) is this: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also specify --image=595879546273/CoreOS-stable-1409.8.0-hvm
to kops create cluster
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can show how to do it both ways though - when you change an image we don't have a shortcut!
|
||
## LAUNCH A SIMPLE REPLICATED APP ON THE CLUSTER. | ||
|
||
Before doing the tasks ahead, we created a simple "webservers" security group inside our KOPS's cluster VPC (using the AWS WEB-UI) allowing inbound port 80 and applied it to our two nodes (not the masters). Then, with the following command we proceed to create a simple replicated app in our coreos-based kops-launched cluster: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great to add a comment that this is not the recommended way to do things - that we should be using a service of Type=LoadBalancer or an Ingress. But that it is useful to understand how things work :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, and also included a link to the nginx-ingress MD file.
Deleted cluster: "coreosbasedkopscluster.k8s.local" | ||
``` | ||
|
||
NOTE: Before destroying the cluster, "really ensure" any extra security group "not created" directly by KOPS has been removed by you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right - maybe say "as otherwise kops will likely be unable to delete the cluster"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok !. I'll take all recommendations into account and do the changes to the documentation. All sound OK to me !.
Hi there. Required changes/improvements pushed !. |
/ok-to-test /lgtm Some nits, but I think I'm going to send you back a PR @tigerlinux - seems more efficient with docs :-) |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb, tigerlinux The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
/test all [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue |
images.
with CoreOS