search

Unleash / helm-charts

Contains helm-charts for Unleash
Apache License 2.0
40 stars 54 forks source link

Edge Container name issue #153

Closed talevy-runai closed 2 weeks ago

talevy-runai commented 3 weeks ago

Describe the bug

I'm trying to use the edge chart as a dependency, In our Values file, I Added a section called "unleashEdge" and used the chart as described:

  - name: unleash-edge
    alias: unleashEdge
    repository: https://docs.getunleash.io/helm-charts
    version: 2.6.1
    condition: unleashEdge.enabled

The issue starts when I use the nameOverride parameter to use "unleash-edge" form for all of the resources created. e.g:

apiVersion: v1
kind: ServiceAccount
metadata:
  name: release-name-unleash-edge
  labels:
    helm.sh/chart: unleashEdge-2.6.1
    app.kubernetes.io/name: unleash-edge
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/version: "v19.1.3"
    app.kubernetes.io/managed-by: Helm

the issue happens to cause you are using the .Chart.Name as the container name, CamelCase is not allowed as container name, as a result, I can't deploy my edge https://github.com/Unleash/helm-charts/blob/main/charts/unleash/templates/deployment.yaml#L32

Suggestion: use constant "edge" as a container name or at least use the "nameOverride" parameter so it will be possible to override it https://github.com/Unleash/helm-charts/blob/main/charts/unleash/templates/_helpers.tpl#L17

I really appreciate any help you can provide.

Steps to reproduce the bug

Import the edge as described

  - name: unleash-edge
    alias: unleashEdge
    repository: https://docs.getunleash.io/helm-charts
    version: 2.6.1
    condition: unleashEdge.enabled

run helm template .

Expected behavior

No response

Logs, error output, etc.

No response

Screenshots

No response

Additional context

No response

Unleash version

2.6.1

Subscription type

Pro

Hosting type

Hosted by Unleash

SDK information (language and version)

No response

talevy-runai commented 3 weeks ago

BDW it is relevant to the server and proxy deployments as well

chriswk commented 2 weeks ago

Hi @talevy-runai I made #154 to add support for the nameOverride. Thanks for reporting this, we'll get the PR merged this week, so you can start using nameOverride.

talevy-runai commented 2 weeks ago

Hi @talevy-runai I made #154 to add support for the nameOverride. Thanks for reporting this, we'll get the PR merged this week, so you can start using nameOverride.

Thank you for the quick response! I'm waiting for the release (: