[incubator/percona-xtradb-cluster] New Chart! #2995

paulczar · 2017-12-11T23:38:42Z

This new chart expands on the percona chart to include xtradb clustering.

Thank you for contributing to kubernetes/charts. Before you submit this PR we'd like to
make sure you are aware of our technical requirements and best practices:

For a quick overview across what we will look at reviewing your PR, please read
our review guidelines:

https://github.com/kubernetes/charts/blob/master/REVIEW_GUIDELINES.md

Following our best practices right from the start will accelerate the review process and
help get your PR merged quicker.

When updates to your PR are requested, please add new commits and do not squash the
history. This will make it easier to identify new changes. The PR will be squashed
anyways when it is merged. Thanks.

unguiculus · 2017-12-12T04:42:45Z

Thanks for the PR. So, why did you create a new chart instead of extending the Percona chart?

paulczar · 2017-12-12T15:12:25Z

@unguiculus I created a new chart because:

It starts from a different base image
it uses statefulset rather than deployment
A person who wants a single mysql may not want the extra xtradb cluster packages installed as they may add bugs/vulns.
The overall lifecycle is different and the charts may diverge even further.

paulczar · 2017-12-12T15:22:01Z

hrmmm the helm lint works fine locally, I can't replicate the error (with helm 2.7.2) shown in the circleci test above.

$ helm version
Client: &version.Version{SemVer:"v2.7.2", GitCommit:"8478fb4fc723885b155c924d1c8c410b7a9444e6", GitTreeState:"clean"}
Server: &version.Version{SemVer:"v2.7.2", GitCommit:"8478fb4fc723885b155c924d1c8c410b7a9444e6", GitTreeState:"clean"}
$ helm lint incubator/percona-xtradb-cluster
==> Linting incubator/percona-xtradb-cluster
Lint OK

1 chart(s) linted, no failures

unguiculus · 2017-12-15T10:00:11Z

incubator/percona-xtradb-cluster/templates/_helpers.tpl

+Expand the name of the chart.
+We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec).
+*/}}
+{{- define "percona.name" -}}


Folow the chart name: percona-xtradb-cluster.name

unguiculus · 2017-12-15T10:01:54Z

incubator/percona-xtradb-cluster/templates/config-map_mysql-config.yaml

+metadata:
+  name: {{ template "percona.fullname" . }}-config-files
+  labels:
+    app: {{ template "percona.fullname" . }}


app label should be {{ template "percona.name" . }}. Update everywhere.

unguiculus · 2017-12-15T10:02:18Z

incubator/percona-xtradb-cluster/templates/service.yaml

+  ports:
+  - port: 9104
+  selector:
+    app: {{ template "percona.fullname" . }}


Add release label to selector.

unguiculus · 2017-12-15T10:02:36Z

incubator/percona-xtradb-cluster/templates/service.yaml

+  - name: state-snap
+    port: 4444
+  selector:
+    app: {{ template "percona.fullname" . }}


Add release label to selector.

unguiculus · 2017-12-15T10:02:42Z

incubator/percona-xtradb-cluster/templates/service.yaml

+    port: 3306
+    targetPort: mysql
+  selector:
+    app: {{ template "percona.fullname" . }}


Add release label to selector.

unguiculus · 2017-12-15T10:03:27Z

incubator/percona-xtradb-cluster/templates/statefulset.yaml

+  template:
+    metadata:
+      labels:
+        app: {{ template "percona.fullname" . }}


Add release label to selector.

unguiculus · 2017-12-15T10:04:32Z

incubator/percona-xtradb-cluster/values.yaml

+
+## percona image and version
+## ref: https://hub.docker.com/r/percona/percona-xtradb-cluster/tags/
+imageRepo: "percona/percona-xtradb-cluster"


User this:

image: repository: tag: pullPolicy:

unguiculus · 2017-12-15T10:05:32Z

incubator/percona-xtradb-cluster/values.yaml

+## Configure resource requests and limits
+## ref: http://kubernetes.io/docs/user-guide/compute-resources/
+##
+resources:


Don't specify defaults:

resources: {} # requests: # memory: 256Mi # cpu: 100m

unguiculus · 2017-12-15T10:05:52Z

incubator/percona-xtradb-cluster/values.yaml

+## It's recommended to change this to 'Always' if the image tag is 'latest'
+## ref: http://kubernetes.io/docs/user-guide/images/#updating-images
+##
+imagePullPolicy: Always


Should be IfNotPresent

unguiculus · 2017-12-15T10:07:34Z

incubator/percona-xtradb-cluster/values.yaml

+
+## Specify password for root user
+##
+mysqlRootPassword: not-a-secure-password


Passwords are auto-generated if not specified. However, this will never be the case, since you add a default here. Default should be auto-generated.

paulczar · 2018-01-03T19:34:00Z

@unguiculus sorry for the slow response, vacation and all that. this update should address your comments.

This new chart expands on the percona chart to include xtradb clustering.

paulczar · 2018-01-08T15:45:34Z

ugh sorry I just realized I squashed my history... bad habits from previous projects :(

unguiculus · 2018-01-11T19:22:48Z

incubator/percona-xtradb-cluster/Chart.yaml

+- https://github.com/kubernetes/charts
+- https://github.com/percona-lab/percona-docker/
+maintainers:
+- name: Paul Czarkowski


Use Github username

unguiculus · 2018-01-11T19:23:06Z

incubator/percona-xtradb-cluster/Chart.yaml

+- https://github.com/percona-lab/percona-docker/
+maintainers:
+- name: Paul Czarkowski
+  email: username.taken@gmail.com


unguiculus · 2018-01-11T19:23:41Z

incubator/percona-xtradb-cluster/Chart.yaml

@@ -0,0 +1,21 @@
+name: percona-xtradb-cluster
+version: 0.0.1


Add appVersion

paulczar · 2018-01-19T21:56:51Z

/assign @unguiculus

unguiculus · 2018-01-27T13:15:34Z

incubator/percona-xtradb-cluster/templates/statefulset.yaml

+      - name: database
+        image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
+        imagePullPolicy: {{ .Values.image.pullPolicy | quote }}
+        command:


Use a consistent list indentation style. It looks like everywhere else you did not indent.

good catch! fixed.

unguiculus · 2018-01-27T13:16:37Z

/ok-to-test

unguiculus · 2018-01-27T13:36:16Z

/lgtm

unguiculus · 2018-01-27T13:36:34Z

/lgtm cancel

unguiculus · 2018-01-30T05:03:06Z

I'd suggest you move the chart to stable before we merge it.

paulczar · 2018-01-31T07:07:21Z

moved to stable 😍

unguiculus · 2018-01-31T07:47:54Z

Fix any references to incubator.

unguiculus · 2018-02-02T12:19:11Z

/lgtm

k8s-ci-robot · 2018-02-02T12:19:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: paulczar, unguiculus

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~OWNERS~~ [unguiculus]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 11, 2017

unguiculus self-assigned this Dec 12, 2017

paulczar force-pushed the percona-xtradb-cluster branch from 690c4c0 to 387aae2 Compare December 12, 2017 15:08

paulczar force-pushed the percona-xtradb-cluster branch 2 times, most recently from bd07f60 to a0b99ab Compare December 12, 2017 22:46

unguiculus suggested changes Dec 15, 2017

View reviewed changes

paulczar force-pushed the percona-xtradb-cluster branch 2 times, most recently from cb49f35 to 536ea15 Compare January 3, 2018 19:33

[incubator/percona-xtradb-cluster] New Chart!

356b237

This new chart expands on the percona chart to include xtradb clustering.

paulczar force-pushed the percona-xtradb-cluster branch from 536ea15 to 356b237 Compare January 4, 2018 17:28

fix documentation for values

15cc1de

unguiculus suggested changes Jan 11, 2018

View reviewed changes

paulczar added 2 commits January 16, 2018 12:54

fix Chart.yaml values

a6d2628

fix a few old references from README

f7a59c7

unguiculus suggested changes Jan 27, 2018

View reviewed changes

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 27, 2018

k8s-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 27, 2018

k8s-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 27, 2018

k8s-ci-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 27, 2018

fix indenting to match rest of chart

3c5b00e

move percona-xtradb-cluster from incubator to stable

f021ab5

remove references to incubator

68df0ab

unguiculus approved these changes Feb 2, 2018

View reviewed changes

k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 2, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 2, 2018

k8s-ci-robot merged commit 0205ce7 into helm:master Feb 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[incubator/percona-xtradb-cluster] New Chart! #2995

[incubator/percona-xtradb-cluster] New Chart! #2995

paulczar commented Dec 11, 2017

unguiculus commented Dec 12, 2017 •

edited

Loading

paulczar commented Dec 12, 2017

paulczar commented Dec 12, 2017

unguiculus Dec 15, 2017

unguiculus Dec 15, 2017

unguiculus Dec 15, 2017

unguiculus Dec 15, 2017

unguiculus Dec 15, 2017

unguiculus Dec 15, 2017

unguiculus Dec 15, 2017

unguiculus Dec 15, 2017

unguiculus Dec 15, 2017

unguiculus Dec 15, 2017

paulczar commented Jan 3, 2018

paulczar commented Jan 8, 2018

unguiculus Jan 11, 2018

paulczar Jan 16, 2018

unguiculus Jan 11, 2018

paulczar Jan 16, 2018

unguiculus Jan 11, 2018

paulczar Jan 16, 2018

paulczar commented Jan 19, 2018

unguiculus Jan 27, 2018

paulczar Jan 30, 2018

unguiculus commented Jan 27, 2018

unguiculus commented Jan 27, 2018

unguiculus commented Jan 27, 2018

unguiculus commented Jan 30, 2018

paulczar commented Jan 31, 2018

unguiculus commented Jan 31, 2018

unguiculus commented Feb 2, 2018

k8s-ci-robot commented Feb 2, 2018

[incubator/percona-xtradb-cluster] New Chart! #2995

[incubator/percona-xtradb-cluster] New Chart! #2995

Conversation

paulczar commented Dec 11, 2017

unguiculus commented Dec 12, 2017 • edited Loading

paulczar commented Dec 12, 2017

paulczar commented Dec 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulczar commented Jan 3, 2018

paulczar commented Jan 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulczar commented Jan 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unguiculus commented Jan 27, 2018

unguiculus commented Jan 27, 2018

unguiculus commented Jan 27, 2018

unguiculus commented Jan 30, 2018

paulczar commented Jan 31, 2018

unguiculus commented Jan 31, 2018

unguiculus commented Feb 2, 2018

k8s-ci-robot commented Feb 2, 2018

unguiculus commented Dec 12, 2017 •

edited

Loading