-
Notifications
You must be signed in to change notification settings - Fork 423
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 Readme for simplified Kops 1.10 instructions #135
Conversation
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.
Can we change one thing?
README.md
Outdated
If the cluster already exists, roll the cluster with `kops rolling-update cluster ${CLUSTER_NAME}` in order to recreate the master nodes. | ||
4. Update the Authenticator DaemonSet's state and output volumes to both use `/srv/kubernetes/aws-iam-authenticator/` for their `hostPath`s. | ||
5. Apply the DaemonSet and ConfigMap resource manifests to launch the Authenticator server on the cluster. | ||
|
||
*Note:* Certain Kops commands will overwrite the `ExecCredential` in kubeconfig so it may need to be restored manually. See [kubernetes/kops#5051](https://github.com/kubernetes/kops/issues/5051) for more information. |
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.
We should probably change this to say something like "If you are using Kops <1.10 …" instead of "Certain Kops" since the 1.10 Kops fixes this bug by supporting client-go
7.0
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.
It's kind of strange to provide instructions that require >=1.10 and then give a warning about behavior when using <1.10. Maybe we just remove the warning altogether if we're not going to provide instructions for <1.10 ?
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.
That's a fair point. Removing is probably the better option.
5e5b5f8
to
6568a4a
Compare
Updated to remove outdated note |
Hey @rifelpet , I had currently tested to change to aws-authenticator on one of my clusters. You don't need to do a |
Sorry this is not correct, all deployments and folders are created on the master, but the certificates are only inserted on instance launch. So you need the But then Kops have a problem, they don't display a "needs update" message, so you can only deploy the master again with Sorry for my mistake. |
@dhemeier Thanks for the heads up, I've updated the |
README.md
Outdated
|
||
2. Apply the changes with `kops update cluster ${CLUSTER_NAME} --yes`. | ||
If the cluster already exists, roll the cluster with `kops rolling-update cluster ${CLUSTER_NAME} --force --yes` in order to recreate the master nodes. |
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 only the master nodes need to be rolled, then use kops rolling-update cluster ${CLUSTER_NAME} --instance-group ${MASTER_INSTANCE_GROUP_NAME} --force --yes
to prevent unnecessary recycling of the nodes.
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.
More generically one could also just do kops rolling-update cluster ${CLUSTER_NAME} --instance-group-roles=Master --force --yes
According to the kops docs a ConfigMap also needs to be created. Without this, the cluster will not validate and the aws-iam-authenticator Pod will not start. |
Perhaps it would be better to maintain one set of authoritative instructions in the kops git and just link to them from this README? |
@somcsel Agreed. I've updated this PR to just point to the kops documentation. I added some additional instructions over there so hopefully it should be sufficient on its own. |
@christopherhein is this ok to merge? |
/lgtm @nckturner I'm good with these updates. |
Is there any way we can get this merged? CC @nckturner |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nckturner, rifelpet The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Kops 1.10 was released 2 days ago which adds support for managing the AWS IAM Authenticator via the cluster spec. This greatly simplifies the install instructions.
It does require Kops 1.10 and Kubernetes 1.10, so if theres a desire to maintain both sets of instructions I can update this PR to do so.