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

k8s: Support StatefulSet and PersistentVolume in Helm chart #2248

Merged
merged 4 commits into from Nov 16, 2016

Conversation

enisoc
Copy link
Member

@enisoc enisoc commented Nov 15, 2016

@thompsonja

Note that this only works in a k8s 1.4 cluster with alpha features enabled (i.e. not in a production GKE cluster). After k8s 1.5, we can update this to use the beta StatefulSet.

The volume init commands that needed to be run as root are now
separated into an init-container, so the main container can directly run
as the vitess user (uid 999).
@thompsonja-bot
Copy link

Reviewed 1 of 1 files at r1, 3 of 4 files at r2, 1 of 3 files at r3, 1 of 2 files at r4.
Review status: 6 of 7 files reviewed at latest revision, 2 unresolved discussions.


helm/vitess/templates/_vtctld.tpl, line 62 at r2 (raw file):

          resources:
{{ toYaml (.resources | default $0.resources) | indent 12 }}
          securityContext:

What is the purpose of this?


helm/vitess/templates/_vtctld.tpl, line 69 at r2 (raw file):

            - |
              set -ex
              eval exec /vt/bin/vtctld $(cat <<END_OF_COMMAND

Why eval exec?


Comments from Reviewable

@thompsonja-bot
Copy link

Reviewed 1 of 2 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

@enisoc
Copy link
Member Author

enisoc commented Nov 16, 2016

The securityContext tells k8s to launch the container as user "vitess" (created in the Dockerfile), so we don't need the su command anymore.

The $(cat <<HEREDOC) subshell is there to put all those flags on one line. It avoids the need to end each line with \, which we've seen is error-prone and frustrating to debug.

After being put on one line, however, it all gets treated as a literal string, so things like -flag="quoted value" don't get transformed through the usual shell semantics into the literal argv element -flag=quoted value. The eval tells the shell to take another pass at doing these transformations.

@thompsonja-bot
Copy link

thompsonja-bot commented Nov 16, 2016

LGTM.

I hadn't notice the quotes change, eval makes total sense :)


Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

Approved with PullApprove

@thompsonja-bot
Copy link

:lgtm:


Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

@enisoc enisoc merged commit 5dd1821 into vitessio:master Nov 16, 2016
@enisoc enisoc deleted the stateful-set branch November 16, 2016 23:48
frouioui pushed a commit to planetscale/vitess that referenced this pull request Nov 21, 2023
Signed-off-by: Max Englander <max@planetscale.com>
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

4 participants