add support to minio bootstrap #2061

yagonobre · 2017-09-07T04:02:23Z

No description provided.

k8s-ci-robot · 2017-09-07T04:02:31Z

Hi @yagonobre. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

yagonobre · 2017-09-07T04:03:04Z

@nitisht can you check this too?

nitisht · 2017-09-07T04:08:00Z

stable/minio/README.md

@@ -90,6 +90,10 @@ The following tables lists the configurable parameters of the Minio chart and th
 | `persistence.storageClass` | Type of persistent volume claim     | `generic`                                               |
 | `persistence.accessMode`   | ReadWriteOnce or ReadOnly           | `ReadWriteOnce`                                         |
 | `resources`                | CPU/Memory resource requests/limits | Memory: `256Mi`, CPU: `100m`                            |
+| `defaultBucket.enabled`    | If true, a bucket will be create after minio


s/create/created

nitisht · 2017-09-07T04:09:09Z

stable/minio/templates/post-install-create-bucket-pod.yaml

+    command: ["/bin/sh", "-c",
+    "/usr/bin/mc config host add myminio http://$MINIO_ENDPOINT:9000 $MINIO_ACCESS_KEY $MINIO_SECRET_KEY;",
+    "/usr/bin/mc rm -r --force myminio/{{ .Values.defaultBucket.name }};",
+    "/usr/bin/mc mb myminio//{{ .Values.defaultBucket.name }};",


myminio/ instead of myminio//

nitisht · 2017-09-08T05:34:58Z

Thank you for the PR @yagonobre, works as expected. There is one issue I can see - after deleting a release via helm delete, the create-bucket pod still does not get deleted, any ideas what may be causing this?

yagonobre · 2017-09-08T15:57:22Z

I run helm delete with --purge the pod stay in completed status

scottrigby · 2017-09-13T19:47:12Z

👀

scottrigby

Otherwise this looks good

scottrigby · 2017-09-13T20:09:47Z

stable/minio/values.yaml

@@ -54,6 +54,17 @@ resources:
    memory: 256Mi
    cpu: 250m

+## Create a bucket after minio install


@yagonobre Let's change this documentation to be consistent with the rest:

## Create a bucket after minio install ## defaultBucket: enabled: false ## If enabled, must be a string with length > 0 # name: example ## Can be one of none|download|upload|public # policy: download

If there is a direct reference to these options, you may also want to add a ## ref: LINK (I'm not familiar enough with Minio yet to know this info, and didn't see it in the docs after a few moments looking in https://docs.minio.io/).

I think that dont exist a direct link
@nitisht, have you info about?

Default bucket just gives an option to users who want to have a bucket created when they deploy Minio, so there is no doc link for this. I will make a note to add a link here, if we add a doc on docs.minio.io

Thanks @nitisht - OK great 👍

@yagonobre This default in the values file should still be updated to this (which adds documentation as comments, and is more DRY):

+## Create a bucket after minio install +## +defaultBucket: + enabled: false + ## If enabled, must be a string with length > 0 + # name: example + ## Can be one of none|download|upload|public + # policy: download +

Instead of what you have in the PR now (which is redundant):

+## Create a bucket after minio install +##defaultBucket +## enabled: true +## name: bucketName +## policy: "none|download|upload|public" +## +defaultBucket: + enabled: false + name: + policy: "download" +

Does that make sense?

scottrigby · 2017-09-13T20:11:30Z

stable/minio/README.md

@@ -90,6 +90,10 @@ The following tables lists the configurable parameters of the Minio chart and th
 | `persistence.storageClass` | Type of persistent volume claim     | `generic`                                               |
 | `persistence.accessMode`   | ReadWriteOnce or ReadOnly           | `ReadWriteOnce`                                         |
 | `resources`                | CPU/Memory resource requests/limits | Memory: `256Mi`, CPU: `100m`                            |
+| `defaultBucket.enabled`    | If true, a bucket will be created after minio
+install | `false` |
+| `defaultBucket.name`       | Bucket name | ` ` |


The default should be nil

scottrigby · 2017-09-13T20:12:01Z

stable/minio/README.md

+| `defaultBucket.enabled`    | If true, a bucket will be created after minio
+install | `false` |
+| `defaultBucket.name`       | Bucket name | ` ` |
+| `defaultBucket.policy` | Bucket policy | `Download` |


The default should be download (lowercase).

scottrigby

LGTM

viglesiasce · 2017-09-19T17:55:43Z

/ok-to-test

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

yagonobre mentioned this pull request Sep 7, 2017

[Dockerfile] Add support to create a bucket on startup minio/minio#4882

Closed

nitisht reviewed Sep 7, 2017

View reviewed changes

yagonobre force-pushed the minio-bootstrap branch 3 times, most recently from 5371b47 to ec9031d Compare September 7, 2017 18:54

yagonobre force-pushed the minio-bootstrap branch from ec9031d to 89a9a05 Compare September 11, 2017 14:03

yagonobre mentioned this pull request Sep 13, 2017

[HELM] use minio on teresa chart luizalabs/teresa#314

Merged

scottrigby suggested changes Sep 13, 2017

View reviewed changes

yagonobre force-pushed the minio-bootstrap branch from 89a9a05 to 6069a91 Compare September 13, 2017 20:19

add support to minio bootstrap

f12ac1b

yagonobre force-pushed the minio-bootstrap branch from 6069a91 to f12ac1b Compare September 15, 2017 16:22

scottrigby approved these changes Sep 15, 2017

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 Sep 19, 2017

viglesiasce merged commit 770e28c into helm:master Sep 19, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support to minio bootstrap #2061

add support to minio bootstrap #2061

yagonobre commented Sep 7, 2017

k8s-ci-robot commented Sep 7, 2017

yagonobre commented Sep 7, 2017

nitisht Sep 7, 2017

nitisht Sep 7, 2017

nitisht commented Sep 8, 2017

yagonobre commented Sep 8, 2017

scottrigby commented Sep 13, 2017

scottrigby left a comment •

edited

Loading

scottrigby Sep 13, 2017

yagonobre Sep 13, 2017

nitisht Sep 14, 2017

scottrigby Sep 14, 2017

yagonobre Sep 15, 2017

scottrigby Sep 13, 2017

yagonobre Sep 13, 2017

scottrigby Sep 13, 2017

yagonobre Sep 13, 2017

scottrigby left a comment

viglesiasce commented Sep 19, 2017

add support to minio bootstrap #2061

add support to minio bootstrap #2061

Conversation

yagonobre commented Sep 7, 2017

k8s-ci-robot commented Sep 7, 2017

yagonobre commented Sep 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nitisht commented Sep 8, 2017

yagonobre commented Sep 8, 2017

scottrigby commented Sep 13, 2017

scottrigby left a comment • edited Loading

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

scottrigby left a comment

Choose a reason for hiding this comment

viglesiasce commented Sep 19, 2017

scottrigby left a comment •

edited

Loading