You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2022/12/22 01:57:52 UTC
[GitHub] [couchdb-helm] yekibud opened a new pull request, #105: Support existing persistent volume
yekibud opened a new pull request, #105:
URL: https://github.com/apache/couchdb-helm/pull/105
<!--
Thank you for contributing to couchdb-helm. Before you submit this PR we'd like to
make sure you are aware of the chart technical requirements and best practices:
* https://github.com/helm/charts/blob/master/CONTRIBUTING.md#technical-requirements
* https://github.com/helm/helm/tree/master/docs/chart_best_practices
For a quick overview across what we will look at reviewing your PR, please read
our review guidelines:
* https://github.com/helm/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.
Please make sure you test your changes before you push them.
-->
#### What this PR does / why we need it:
Statefulset volumeClaimTemplates create new volumes and PersistentVolumeClaims which cannot be mutated. This PR adds the ability to declare a persistentVolume as existing and provide the required PV and PVC data for binding a volume to the statefulset. This is especially useful for restoring backup snapshots during disaster recovery efforts, but could also be used for initial data migrations or other scenarios where users would like to deploy this chart with existing CouchDB data.
#### Which issue this PR fixes
No related issue tracker.
#### Special notes for your reviewer:
* I noticed [my previous merged PR](https://github.com/apache/couchdb-helm/pull/102)'s version bump isn't being listed by `helm search repo` for the `https://apache.github.io/couchdb-helm` repo, but it _does_ appear to be listed on [the releases page](url) for this project on github.com. Not sure if there's something I'm doing wrong or something needs to be fixed.
* I'm still getting the error I got from my previous PR running `make test`.
Error: Error linting and installing charts: Error identifying merge base: Error running process: exit status 128
This seems like an unrelated/existing issue, but please let me know if I'm doing something wrong here, as well.
#### Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.
- [x] Chart Version bumped
- [ ] e2e tests pass
- [x] Variables are documented in the README.md
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-helm] yekibud commented on pull request #105: Support existing persistent volume
Posted by "yekibud (via GitHub)" <gi...@apache.org>.
yekibud commented on PR #105:
URL: https://github.com/apache/couchdb-helm/pull/105#issuecomment-1442528964
> thanks for the PR @yekibud. The changes look mostly good to me - just a couple of queries / suggestions.
>
> On the issue of why releases aren't showing up in the helm repo index, I believe this is due to the lingering `master` branch defining the github pages location. We moved the default branch to `main` some time ago, so I've removed the `master` branch and hopefully this will fix the issue, though it may take a while to propagate.
Hi @willholley . I made the changes you suggested. Please let me know if you have any further feedback or if it's possible to approve/merge. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-helm] willholley commented on pull request #105: Support existing persistent volume
Posted by "willholley (via GitHub)" <gi...@apache.org>.
willholley commented on PR #105:
URL: https://github.com/apache/couchdb-helm/pull/105#issuecomment-1479556001
thanks @yekibud - lgtm. It just needs a version bump/rebase and then I can merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-helm] willholley commented on a diff in pull request #105: Support existing persistent volume
Posted by GitBox <gi...@apache.org>.
willholley commented on code in PR #105:
URL: https://github.com/apache/couchdb-helm/pull/105#discussion_r1063630994
##########
docs/index.yaml:
##########
@@ -1,6 +1,30 @@
apiVersion: v1
entries:
couchdb:
+ - apiVersion: v1
+ appVersion: 3.2.1
+ created: "2022-12-20T17:53:44.680236109-08:00"
+ description: A database featuring seamless multi-master sync, that scales from
+ big data to mobile, with an intuitive HTTP/JSON API and designed for reliability.
+ digest: 5a66b7584d72bb2ac9c30e65d751ee374adbea2fe73dac20a058fb20bf1476e2
+ home: https://couchdb.apache.org/
+ icon: http://couchdb.apache.org/CouchDB-visual-identity/logo/CouchDB-couch-symbol.svg
+ keywords:
+ - couchdb
+ - database
+ - nosql
+ maintainers:
+ - email: kocolosk@apache.org
+ name: kocolosk
+ - email: willholley@apache.org
+ name: willholley
+ name: couchdb
+ sources:
+ - https://github.com/apache/couchdb-helm
+ - https://github.com/apache/couchdb-docker
+ urls:
+ - couchdb-3.6.4.tgz
+ version: 3.6.4
- apiVersion: v1
appVersion: 3.2.1
created: "2022-11-16T10:04:19.187911144-08:00"
Review Comment:
This doesn't need to be manually updated - it's now generated automatically on release. We need to update the contributing guide / remove the docs folder...
##########
couchdb/Chart.yaml:
##########
@@ -1,6 +1,6 @@
apiVersion: v1
name: couchdb
-version: 3.6.3
+version: 3.6.4
Review Comment:
Can you rebase / re-bump the version please?
##########
couchdb/templates/persistentvolumeclaim.yaml:
##########
@@ -0,0 +1,29 @@
+{{- if and .Values.persistentVolume.enabled .Values.persistentVolume.existing -}}
+apiVersion: v1
+kind: PersistentVolumeClaim
+metadata:
+ name: {{ .Values.persistentVolume.claimName }}
+ labels:
+ app: {{ template "couchdb.name" . }}
+ release: {{ .Release.Name }}
+ {{- with .Values.persistentVolume.annotations }}
+ annotations:
+ {{- toYaml . | nindent 10 }}
Review Comment:
nindent value here is probably 6 rather than 10? I assume it is 10 since it was copied from the statefulset yaml?
It feels like there's some potential for the persistentvolumeclaim.yaml and statefulset.yaml to get out of sync - perhaps we could extract a helper function for the common yaml?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-helm] yekibud commented on a diff in pull request #105: Support existing persistent volume
Posted by GitBox <gi...@apache.org>.
yekibud commented on code in PR #105:
URL: https://github.com/apache/couchdb-helm/pull/105#discussion_r1070711871
##########
couchdb/README.md:
##########
@@ -164,63 +164,67 @@ required options to set:
A variety of other parameters are also configurable. See the comments in the
`values.yaml` file for further details:
-| Parameter | Default |
-|--------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| `adminUsername` | admin |
-| `adminPassword` | auto-generated |
-| `adminHash` | |
-| `cookieAuthSecret` | auto-generated |
-| `image.repository` | couchdb |
-| `image.tag` | 3.2.1 |
-| `image.pullPolicy` | IfNotPresent |
-| `searchImage.repository` | kocolosk/couchdb-search |
-| `searchImage.tag` | 0.1.0 |
-| `searchImage.pullPolicy` | IfNotPresent |
-| `initImage.repository` | busybox |
-| `initImage.tag` | latest |
-| `initImage.pullPolicy` | Always |
-| `ingress.enabled` | false |
-| `ingress.hosts` | chart-example.local |
-| `ingress.annotations` | |
-| `ingress.path` | / |
-| `ingress.tls` | |
-| `persistentVolume.accessModes` | ReadWriteOnce |
-| `persistentVolume.storageClass` | Default for the Kube cluster |
-| `persistentVolume.annotations` | {} |
-| `podManagementPolicy` | Parallel |
-| `affinity` | |
-| `topologySpreadConstraints` | |
-| `annotations` | |
-| `tolerations` | |
-| `resources` | |
-| `service.annotations` | |
-| `service.enabled` | true |
-| `service.type` | ClusterIP |
-| `service.externalPort` | 5984 |
-| `dns.clusterDomainSuffix` | cluster.local |
-| `networkPolicy.enabled` | true |
-| `serviceAccount.enabled` | true |
-| `serviceAccount.create` | true |
-| `serviceAccount.imagePullSecrets` | |
-| `sidecars` | {} |
-| `livenessProbe.enabled` | true |
-| `livenessProbe.failureThreshold` | 3 |
-| `livenessProbe.initialDelaySeconds` | 0 |
-| `livenessProbe.periodSeconds` | 10 |
-| `livenessProbe.successThreshold` | 1 |
-| `livenessProbe.timeoutSeconds` | 1 |
-| `readinessProbe.enabled` | true |
-| `readinessProbe.failureThreshold` | 3 |
-| `readinessProbe.initialDelaySeconds` | 0 |
-| `readinessProbe.periodSeconds` | 10 |
-| `readinessProbe.successThreshold` | 1 |
-| `readinessProbe.timeoutSeconds` | 1 |
-| `prometheusPort.enabled` | false |
-| `prometheusPort.port` | 17896 |
-| `prometheusPort.bind_address` | 0.0.0.0 |
-| `placementConfig.enabled` | false |
-| `placementConfig.image.repository` | caligrafix/couchdb-autoscaler-placement-manager |
-| `placementConfig.image.tag` | 0.1.0 |
+| Parameter | Default |
+| -------------------------------------- | ------------------------------------------------ |
+| `adminUsername` | admin |
+| `adminPassword` | auto-generated |
+| `adminHash` | |
+| `cookieAuthSecret` | auto-generated |
+| `image.repository` | couchdb |
+| `image.tag` | 3.2.1 |
+| `image.pullPolicy` | IfNotPresent |
+| `searchImage.repository` | kocolosk/couchdb-search |
+| `searchImage.tag` | 0.1.0 |
+| `searchImage.pullPolicy` | IfNotPresent |
+| `initImage.repository` | busybox |
+| `initImage.tag` | latest |
+| `initImage.pullPolicy` | Always |
+| `ingress.enabled` | false |
+| `ingress.hosts` | chart-example.local |
+| `ingress.annotations` | |
+| `ingress.path` | / |
+| `ingress.tls` | |
+| `persistentVolume.accessModes` | ReadWriteOnce |
+| `persistentVolume.storageClass` | Default for the Kube cluster |
+| `persistentVolume.annotations` | {} |
+| `persistentVolume.existing` | false |
Review Comment:
Unfortunately [this recently merged PR](https://github.com/apache/couchdb-helm/pull/106) added unnecessary and excessive spacing to a table in the README, in addition to obscuring the diff. The changes in the current PR are the additions of lines #190-193.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-helm] yekibud commented on a diff in pull request #105: Support existing persistent volume
Posted by GitBox <gi...@apache.org>.
yekibud commented on code in PR #105:
URL: https://github.com/apache/couchdb-helm/pull/105#discussion_r1070711871
##########
couchdb/README.md:
##########
@@ -164,63 +164,67 @@ required options to set:
A variety of other parameters are also configurable. See the comments in the
`values.yaml` file for further details:
-| Parameter | Default |
-|--------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| `adminUsername` | admin |
-| `adminPassword` | auto-generated |
-| `adminHash` | |
-| `cookieAuthSecret` | auto-generated |
-| `image.repository` | couchdb |
-| `image.tag` | 3.2.1 |
-| `image.pullPolicy` | IfNotPresent |
-| `searchImage.repository` | kocolosk/couchdb-search |
-| `searchImage.tag` | 0.1.0 |
-| `searchImage.pullPolicy` | IfNotPresent |
-| `initImage.repository` | busybox |
-| `initImage.tag` | latest |
-| `initImage.pullPolicy` | Always |
-| `ingress.enabled` | false |
-| `ingress.hosts` | chart-example.local |
-| `ingress.annotations` | |
-| `ingress.path` | / |
-| `ingress.tls` | |
-| `persistentVolume.accessModes` | ReadWriteOnce |
-| `persistentVolume.storageClass` | Default for the Kube cluster |
-| `persistentVolume.annotations` | {} |
-| `podManagementPolicy` | Parallel |
-| `affinity` | |
-| `topologySpreadConstraints` | |
-| `annotations` | |
-| `tolerations` | |
-| `resources` | |
-| `service.annotations` | |
-| `service.enabled` | true |
-| `service.type` | ClusterIP |
-| `service.externalPort` | 5984 |
-| `dns.clusterDomainSuffix` | cluster.local |
-| `networkPolicy.enabled` | true |
-| `serviceAccount.enabled` | true |
-| `serviceAccount.create` | true |
-| `serviceAccount.imagePullSecrets` | |
-| `sidecars` | {} |
-| `livenessProbe.enabled` | true |
-| `livenessProbe.failureThreshold` | 3 |
-| `livenessProbe.initialDelaySeconds` | 0 |
-| `livenessProbe.periodSeconds` | 10 |
-| `livenessProbe.successThreshold` | 1 |
-| `livenessProbe.timeoutSeconds` | 1 |
-| `readinessProbe.enabled` | true |
-| `readinessProbe.failureThreshold` | 3 |
-| `readinessProbe.initialDelaySeconds` | 0 |
-| `readinessProbe.periodSeconds` | 10 |
-| `readinessProbe.successThreshold` | 1 |
-| `readinessProbe.timeoutSeconds` | 1 |
-| `prometheusPort.enabled` | false |
-| `prometheusPort.port` | 17896 |
-| `prometheusPort.bind_address` | 0.0.0.0 |
-| `placementConfig.enabled` | false |
-| `placementConfig.image.repository` | caligrafix/couchdb-autoscaler-placement-manager |
-| `placementConfig.image.tag` | 0.1.0 |
+| Parameter | Default |
+| -------------------------------------- | ------------------------------------------------ |
+| `adminUsername` | admin |
+| `adminPassword` | auto-generated |
+| `adminHash` | |
+| `cookieAuthSecret` | auto-generated |
+| `image.repository` | couchdb |
+| `image.tag` | 3.2.1 |
+| `image.pullPolicy` | IfNotPresent |
+| `searchImage.repository` | kocolosk/couchdb-search |
+| `searchImage.tag` | 0.1.0 |
+| `searchImage.pullPolicy` | IfNotPresent |
+| `initImage.repository` | busybox |
+| `initImage.tag` | latest |
+| `initImage.pullPolicy` | Always |
+| `ingress.enabled` | false |
+| `ingress.hosts` | chart-example.local |
+| `ingress.annotations` | |
+| `ingress.path` | / |
+| `ingress.tls` | |
+| `persistentVolume.accessModes` | ReadWriteOnce |
+| `persistentVolume.storageClass` | Default for the Kube cluster |
+| `persistentVolume.annotations` | {} |
+| `persistentVolume.existing` | false |
Review Comment:
Unfortunately [this PR](https://github.com/apache/couchdb-helm/pull/106) added unnecessary and excessive spacing to a table in the README, in addition to obscuring the diff. The changes in this PR are the additions of lines #190-193.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-helm] yekibud commented on pull request #105: Support existing persistent volume
Posted by "yekibud (via GitHub)" <gi...@apache.org>.
yekibud commented on PR #105:
URL: https://github.com/apache/couchdb-helm/pull/105#issuecomment-1485494744
> thanks @yekibud - lgtm. It just needs a version bump/rebase and then I can merge.
@willholley Great! Bumped and resolved conflicts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-helm] willholley commented on pull request #105: Support existing persistent volume
Posted by "willholley (via GitHub)" <gi...@apache.org>.
willholley commented on PR #105:
URL: https://github.com/apache/couchdb-helm/pull/105#issuecomment-1459772347
thanks @yekibud - no need to update the docs / tarball as these are generated automatically
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-helm] willholley merged pull request #105: Support existing persistent volume
Posted by "willholley (via GitHub)" <gi...@apache.org>.
willholley merged PR #105:
URL: https://github.com/apache/couchdb-helm/pull/105
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-helm] yekibud commented on a diff in pull request #105: Support existing persistent volume
Posted by GitBox <gi...@apache.org>.
yekibud commented on code in PR #105:
URL: https://github.com/apache/couchdb-helm/pull/105#discussion_r1070712025
##########
docs/index.yaml:
##########
@@ -1,6 +1,30 @@
apiVersion: v1
entries:
couchdb:
+ - apiVersion: v1
+ appVersion: 3.2.1
+ created: "2022-12-20T17:53:44.680236109-08:00"
+ description: A database featuring seamless multi-master sync, that scales from
+ big data to mobile, with an intuitive HTTP/JSON API and designed for reliability.
+ digest: 5a66b7584d72bb2ac9c30e65d751ee374adbea2fe73dac20a058fb20bf1476e2
+ home: https://couchdb.apache.org/
+ icon: http://couchdb.apache.org/CouchDB-visual-identity/logo/CouchDB-couch-symbol.svg
+ keywords:
+ - couchdb
+ - database
+ - nosql
+ maintainers:
+ - email: kocolosk@apache.org
+ name: kocolosk
+ - email: willholley@apache.org
+ name: willholley
+ name: couchdb
+ sources:
+ - https://github.com/apache/couchdb-helm
+ - https://github.com/apache/couchdb-docker
+ urls:
+ - couchdb-3.6.4.tgz
+ version: 3.6.4
- apiVersion: v1
appVersion: 3.2.1
created: "2022-11-16T10:04:19.187911144-08:00"
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-helm] yekibud commented on a diff in pull request #105: Support existing persistent volume
Posted by GitBox <gi...@apache.org>.
yekibud commented on code in PR #105:
URL: https://github.com/apache/couchdb-helm/pull/105#discussion_r1071513684
##########
couchdb/templates/persistentvolumeclaim.yaml:
##########
@@ -0,0 +1,29 @@
+{{- if and .Values.persistentVolume.enabled .Values.persistentVolume.existing -}}
+apiVersion: v1
+kind: PersistentVolumeClaim
+metadata:
+ name: {{ .Values.persistentVolume.claimName }}
+ labels:
+ app: {{ template "couchdb.name" . }}
+ release: {{ .Release.Name }}
+ {{- with .Values.persistentVolume.annotations }}
+ annotations:
+ {{- toYaml . | nindent 10 }}
Review Comment:
Good catch, thanks. Moved these repeated sections to a couple helper functions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-helm] yekibud commented on pull request #105: Support existing persistent volume
Posted by "yekibud (via GitHub)" <gi...@apache.org>.
yekibud commented on PR #105:
URL: https://github.com/apache/couchdb-helm/pull/105#issuecomment-1478762488
> thanks @yekibud - no need to update the docs / tarball as these are generated automatically
@willholley sorry - I've been using this branch in production and needed it for updating charts. I guess I'd have to point to my own pages/repo in order to do that with the new pattern? In any case, I just kept the `docs` folder in a fork of my PR branch and removed it from this PR.
Also note that I've added a few more commits to support an array of `persistentVolume.existingClaims` for cluster scenarios. Let me know what you think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org