Charts: [stable/minio] minio.fullname is incorrect

Created on 23 Nov 2018  Â·  11Comments  Â·  Source: helm/charts

Hi,

It seems there is something missing in the implementation of minio.fullname at this line: https://github.com/helm/charts/blob/master/stable/minio/templates/_helpers.tpl#L22

Basically, it says that it will use only the release name while typically, people use release name followed by name like in the generated helpers of helm create:

{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" -}}

All 11 comments

@victornoel could you please send and example when this current implementation fails?

@victornoel Thanks for reporting this issue. Looks like this bug was introduced in this PR: https://github.com/helm/charts/pull/8349

I've added a fix for this here: https://github.com/helm/charts/pull/9611

@obedmr I've added a comment about this here: https://github.com/helm/charts/pull/8349#issuecomment-442595444

@wlan0 ooh, I see it, thanks for the catch, will test it. Because it's being put back as it was before and it was putting the wrong command in NOTES.

@wlan0 @obedmr thanks!

@victornoel Looked into this and concluded that @obedmr's first change was indeed correct. The convention you mentioned isn't a standard AFAICT.

I'm closing my PR: https://github.com/helm/charts/pull/9611

Thanks @obedmr!

@wlan0 @obedmr sorry, but I think this is a mistake (to leave it like that), for 2 reasons:

  1. First, when minio is used as a subchart, its naming is horrible, example (simple chart with just the dependency defined, no alias):
➜  k8s git:(master) ✗ helm install test/ --namespace test
NAME:   named-dog
LAST DEPLOYED: Tue Dec  4 08:33:59 2018
NAMESPACE: test
STATUS: DEPLOYED

RESOURCES:
==> v1beta2/Deployment
NAME       AGE
named-dog  0s

==> v1/Pod(related)

NAME                        READY  STATUS   RESTARTS  AGE
named-dog-77fd795bfd-855nd  0/1    Pending  0         0s

==> v1/Secret

NAME       AGE
named-dog  1s

==> v1/ConfigMap
named-dog  1s

==> v1/PersistentVolumeClaim
named-dog  1s

==> v1/Service
named-dog  1s

Why is minio named with my release name? How is it readable? How do I know this is related to minio?

  1. Second, you can't have two minio as subchart because of this (two dependencies to minio with different alias):
➜  k8s git:(master) ✗ helm install test/ --namespace test
Error: release luminous-saola failed: secrets "luminous-saola" already exists

Both are named in the same way, conflict arises.

Finally, I think if there was a bug and the solution was this naming, this is an incorrect fix for the original bug. Everybody is succeeding using this naming, I don't see why minio couldn't.

I think it IS a correct practice for reusable charts to be named like this, if not they are unusable. In the same way that you usually prefix your helpers with the name of the chart.

I'm agree with @victornoel.

I use minio too as subchart and others subcharts.
When I list resources (Pods, Services, Secrets, Deployments, PersistentVolumeClaim, ConfigMap), minio resources are listed with release name as said @victornoel .
However mongodb subchart, for example, enriches release name with "mongodb" suffix.

I put an example below of deployed services on my kubernete.
As you can see, the first one is minio's service, only named with release name "doki".

==> v1/Service
doki                    1m
doki-mongodb            1m
doki-rabbitmq-headless  1m
doki-rabbitmq           1m
doki-api                1m

Some user will have some trouble if they don't fill property nameOverride in values.yaml file, they will have name collision during service creation as I did when deploying my services.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

@wlan0 any way to get back on your decision? :)

I suppose this was finally solved by #11400

Was this page helpful?
0 / 5 - 0 ratings