Skip to content

doc: Update the EKS getting started guide#11697

Merged
aanm merged 1 commit intomasterfrom
pr/michi/eks
Jun 4, 2020
Merged

doc: Update the EKS getting started guide#11697
aanm merged 1 commit intomasterfrom
pr/michi/eks

Conversation

@michi-covalent
Copy link
Copy Markdown
Contributor

@michi-covalent michi-covalent commented May 26, 2020 •
edited
Loading

  • Create a cluster with --without-nodegroup option and create a node
    group after deploying Cilium.
  • Change the cluster name to test-cluster in the ENI getting started
    so it can use the same "Create a nodegroup" section as the EKS guide.

Ref: eksctl-io/eksctl#2022

Signed-off-by: Michi Mutsuzaki michi@isovalent.com

@michi-covalent michi-covalent requested a review from a team as a code owner May 26, 2020 16:42
@maintainer-s-little-helper
Copy link
Copy Markdown

maintainer-s-little-helper Bot commented May 26, 2020

Please set the appropriate release note label.

@michi-covalent michi-covalent added the release-note/misc This PR makes changes that have no direct user impact. label May 26, 2020
@coveralls
Copy link
Copy Markdown

coveralls commented May 26, 2020 •
edited
Loading

Coverage Status

Coverage decreased (-0.007%) to 36.941% when pulling 96bfe92 on pr/michi/eks into 507d9b8 on master.

@errordeveloper
Copy link
Copy Markdown
Contributor

errordeveloper commented May 26, 2020 •
edited
Loading

Hey @michi-covalent, just a couple of nits!

  1. Would you mind to use either long or short names for all flag please?

So either:

eksctl create cluster -n test-cluster -N 0 -M 2

Or:

eksctl create cluster --name test-cluster --nodes 0 --nodes-max 2

If it's best if you could make it consistent for the entire guide too.

  1. Perhaps we could explicitly create the nodegroup in separate step instead of creating one with 0 nodes and scaling up later. So overall the flow would be like this:
eksctl create cluster --name test-cluster --without-nodegroup
kubectl delet --namespace kube-system  daemonset/aws-node
helm install ...
eksctl create nodegroup --cluster test-cluster

What do you think?

@michi-covalent
Copy link
Copy Markdown
Contributor Author

michi-covalent commented May 26, 2020

@errordeveloper yeah i like the second option to create a cluster without nodegroup first. let me try what you suggested, and i'll update this pr accordingly.

@michi-covalent
Copy link
Copy Markdown
Contributor Author

michi-covalent commented May 26, 2020

i'm seeing warnings like these after creating a nodegroup. not sure if i got the same error before. trying scaling the cluster again to check.

level=info msg="Retrieved node information from kubernetes node" nodeName=ip-192-168-50-63.ec2.internal subsys=k8s
level=warning msg="Waiting for k8s node information" error="required IPv4 pod CIDR not present in node resource" subsys=k8s
level=info msg="Retrieved node information from kubernetes node" nodeName=ip-192-168-50-63.ec2.internal subsys=k8s
level=warning msg="Waiting for k8s node information" error="required IPv4 pod CIDR not present in node resource" subsys=k8s
level=info msg="Retrieved node information from kubernetes node" nodeName=ip-192-168-50-63.ec2.internal subsys=k8s
level=warning msg="Waiting for k8s node information" error="required IPv4 pod CIDR not present in node resource" subsys=k8s
level=info msg="Retrieved node information from kubernetes node" nodeName=ip-192-168-50-63.ec2.internal subsys=k8s
level=warning msg="Waiting for k8s node information" error="required IPv4 pod CIDR not present in node resource" subsys=k8s

@michi-covalent
Copy link
Copy Markdown
Contributor Author

michi-covalent commented May 26, 2020

discussion on slack: https://app.slack.com/client/T1MATJ4SZ/C1MATJ5U5

@aanm
Copy link
Copy Markdown
Member

aanm commented May 27, 2020

@michi-covalent I've marked the PR as draft since it looked like there's still some work being done

@aanm aanm marked this pull request as draft May 27, 2020 13:01
@michi-covalent michi-covalent changed the title doc: Specify --nodes-max in eksctl doc: Update the EKS getting started guide May 29, 2020
@michi-covalent michi-covalent marked this pull request as ready for review May 29, 2020 18:06
@michi-covalent
Copy link
Copy Markdown
Contributor Author

michi-covalent commented May 29, 2020

ready for review cc @errordeveloper

@michi-covalent
Copy link
Copy Markdown
Contributor Author

michi-covalent commented Jun 2, 2020

test-me-please

@aanm aanm requested a review from errordeveloper June 3, 2020 16:14
errordeveloper
errordeveloper approved these changes Jun 3, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@errordeveloper errordeveloper left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Overall, LGTM! Few nits, if you have a moment :)

errordeveloper
errordeveloper reviewed Jun 3, 2020
View reviewed changes
Comment thread Documentation/gettingstarted/k8s-install-eks.rst Outdated
errordeveloper
errordeveloper reviewed Jun 3, 2020
View reviewed changes
Comment thread Documentation/gettingstarted/aws-eni.rst Outdated
@michi-covalent
doc: Update the EKS getting started guide
96bfe92 
- Create a cluster with `--without-nodegroup` option and create a node
  group after deploying Cilium.
- Change the cluster name to `test-cluster` in the ENI getting started
  so it can use the same "Create a nodegroup" section as the EKS guide.

Ref: eksctl-io/eksctl#2022

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
@aanm aanm added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 4, 2020
@aanm aanm merged commit 5f1bec8 into master Jun 4, 2020
@aanm aanm deleted the pr/michi/eks branch June 4, 2020 12:25
@tklauser tklauser mentioned this pull request Jun 4, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@errordeveloper errordeveloper errordeveloper approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@michi-covalent @coveralls @errordeveloper @aanm @tklauser @borkmann