jfrog / charts

JFrog official Helm Charts
https://jfrog.com/integration/helm-repository/
Apache License 2.0
259 stars 448 forks source link

Shouldn't fullname be used for the new artifactory-unified-secret? #1882

Closed baselbmz closed 6 months ago

baselbmz commented 6 months ago

I have noticed that in the stable/artifactory/templates/artifactory-unified-secret.yaml we are using name instead of fullname.

  name: {{ template "artifactory.name" . }}-unified-secret

I was wondering if fullname should have been used here? or is there a good reason for using name instead?

I have 2 instances of artifactory installed on the same namesapce, and while doing an upgrade, I got a conflict because both releases are trying to create the same object.

Is this a BUG REPORT or FEATURE REQUEST? (choose one): This is a question at the moment, it could be seen as a BUG REPORT if the chart was supposed to support 2 releases on the same namespace

Version of Helm and Kubernetes: Kubernetes: v1.28.7-gke.1226000 Helm: v3.14.4

Which chart: jfrog/artifactory --version 107.84.10

Which product license (Enterprise/Pro/oss): oss

JFrog support reference (if already raised with support team): N/A

What happened: 2 helm releases on the same namespace fight over the object artifactory-unified-secret. We use fullnameOverride to avoid such thing, but this is not used for this object :(

What you expected to happen: if the chart was supposed to support 2 releases on the same namespace then fullname should be used for artifactory-unified-secret

How to reproduce it (as minimally and precisely as possible): try to deploy 2 releases on the same namespace while using fullnameOverride (but not using nameOverride)

Anything else we need to know: A work around could be to use nameOverride

oumkale commented 6 months ago

Hi @baselbmz,

Thank you for taking this point, we will be taking this up internally and It will be fixed in future 7.84.x release

chukka commented 6 months ago

@baselbmz Please try 107.84.11 chart for the fix and share feedback

baselbmz commented 6 months ago

Thanks @chukka, we have tested the new release and the issue got fixed