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

Add secure DaemonSet instructions #4037

Merged
merged 1 commit into from
Nov 14, 2018
Merged

Add secure DaemonSet instructions #4037

merged 1 commit into from
Nov 14, 2018

Conversation

jseldess
Copy link
Contributor

Also restructure the content into steps.

Fixes #3318.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@jseldess
Copy link
Contributor Author


v2.1/kubernetes-performance.md, line 437 at r1 (raw file):

    ~~~

6. Initialize the cluster, specifying any one of the pod names:

@a-robinson, can you help me tweak this for a secure deployment? I'm not sure how to do it. For statefulsets, we have a distinct init file to use.

Also, if you see other ways the instructions need to be change for secure vs. insecure, please add those comments.

Copy link
Contributor

@a-robinson a-robinson left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


v2.1/kubernetes-performance.md, line 417 at r1 (raw file):

10.128.0.4,10.128.0.5,10.128.0.3

Is this meant to be the list that's already in the file (because it's not...)? Or an example? It's not clear to me.


v2.1/kubernetes-performance.md, line 437 at r1 (raw file):

Previously, jseldess (Jesse Seldess) wrote…

@a-robinson, can you help me tweak this for a secure deployment? I'm not sure how to do it. For statefulsets, we have a distinct init file to use.

Also, if you see other ways the instructions need to be change for secure vs. insecure, please add those comments.

First off, this should mention the need to approve certificate signing requests in the secure version.

For initialization, see the comment at the top of the YAML file:

# You will then have to approve certificate signing requests and initialize the
# cluster as described in the parent directory's README.md file. In order for
# the initialization step to work, note that you will need to change the
# address used by the cluster-init-secure.yaml file on the
# "--host=cockroachdb-0.cockroach" line from "cockroachdb-0.cockroach" to the
# address of one of your nodes.

In other words, you have to take the cloud/kubernetes/cluster-init-secure.yaml file, replace the line --host=cockroachdb-0.cockroach with --host=<one-of-your-addresses>, and then create the init job: kubectl create -f cluster-init-secure.yaml

We should run through this again using the docs to verify nothing else is missing. Are you up for it, or should I?

Copy link
Contributor Author

@jseldess jseldess left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


v2.1/kubernetes-performance.md, line 417 at r1 (raw file):

Previously, a-robinson (Alex Robinson) wrote…
10.128.0.4,10.128.0.5,10.128.0.3

Is this meant to be the list that's already in the file (because it's not...)? Or an example? It's not clear to me.

I'm not sure. I didn't change this content, and you wrote the original doc ;). I'll check against the "list in the provided file."


v2.1/kubernetes-performance.md, line 437 at r1 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

First off, this should mention the need to approve certificate signing requests in the secure version.

For initialization, see the comment at the top of the YAML file:

# You will then have to approve certificate signing requests and initialize the
# cluster as described in the parent directory's README.md file. In order for
# the initialization step to work, note that you will need to change the
# address used by the cluster-init-secure.yaml file on the
# "--host=cockroachdb-0.cockroach" line from "cockroachdb-0.cockroach" to the
# address of one of your nodes.

In other words, you have to take the cloud/kubernetes/cluster-init-secure.yaml file, replace the line --host=cockroachdb-0.cockroach with --host=<one-of-your-addresses>, and then create the init job: kubectl create -f cluster-init-secure.yaml

We should run through this again using the docs to verify nothing else is missing. Are you up for it, or should I?

I should've run through the doc on my own first. Sorry about that. I'll take a shot at it.

Copy link
Contributor Author

@jseldess jseldess left a comment

Choose a reason for hiding this comment

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

@a-robinson, I tried to test this out but wasn't able to figure out the steps for mounting a local ssd and the taints setting. But I did update the instructions to be more explicit, based on my understanding of the process. It'd be great if you could either run through the process on your own and leave notes here or run through the process with me.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

Copy link
Contributor

@a-robinson a-robinson left a comment

Choose a reason for hiding this comment

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

:lgtm: after your changes

Let's chat about the local SSDs and taints/tolerations after our other meeting this afternoon.

Reviewed 1 of 1 files at r2.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained


v2.1/kubernetes-performance.md, line 430 at r2 (raw file):

3. Modify the file wherever there is a `TODO` comment, for example:
    - Unless you want CockroachDB running on every machine in your Kubernetes cluster, pick out which nodes you want to run CockroachDB on using either [node labels](#node-labels) or [node taints](#node-taints) and then configure them and the `DaemonSet` configuration file appropriately as described in the relevant [Dedicated Nodes](#dedicated-nodes) section.

I'd add something like: "If you do want CockroachDB running on every machine, just remove the nodeSelector and tolerations sections.

Copy link
Contributor Author

@jseldess jseldess left a comment

Choose a reason for hiding this comment

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

Thanks, Alex. I have to head out early today, so we'll have to sync about SSDs and taints another day soon.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale)


v2.1/kubernetes-performance.md, line 430 at r2 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

I'd add something like: "If you do want CockroachDB running on every machine, just remove the nodeSelector and tolerations sections.

Done.

@jseldess jseldess changed the title [WIP] Add secure DaemonSet instructions Add secure DaemonSet instructions Nov 14, 2018
Copy link
Contributor

@a-robinson a-robinson left a comment

Choose a reason for hiding this comment

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

No worries, this is mergeable as is if you'd like.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale)

Also restructure the content into steps.

Fixes #3318.
@jseldess jseldess merged commit 5bb4765 into master Nov 14, 2018
@jseldess jseldess deleted the secure-DaemonSet branch November 14, 2018 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants