PriorityClassName is not set in redis-ha components

[zoispag]

Describe the bug

When setting global.priorityClassName and redis-ha.enabled: true the redis components (redis-ha-servers and redis-ha-proxys) do not set the same priorityClassName.

Related helm chart

argo-cd

Helm chart version

6.9.3

To Reproduce

Deploy HA ArgoCD with the following values.yml:

global:
  priorityClassName: system-cluster-critical

redis-ha:
  enabled: true

controller:
  replicas: 1

server:
  replicas: 2

repoServer:
  replicas: 2

applicationSet:
  replicas: 2

Expected behavior

Global priorityClassName should be also set to ha-redis components. In case of non ha-redis, redis deployment does get the priorityClassName as expected.

Screenshots

No response

Additional context

redis-ha is a dependency:

dependencies:
  - name: redis-ha
    version: 4.26.1
    repository: https://dandydeveloper.github.io/charts/
    condition: redis-ha.enabled

and has support for setting priorityClassName:

https://github.com/DandyDeveloper/charts/blob/0fe831e1c5171dd0217e1f543d40798f473a561f/charts/redis-ha/templates/redis-haproxy-deployment.yaml#L181-L183

https://github.com/DandyDeveloper/charts/blob/0fe831e1c5171dd0217e1f543d40798f473a561f/charts/redis-ha/templates/redis-ha-statefulset.yaml#L523-L525

[mkilchhofer]

I think this is outside of our control. Is a comment inside the table enough? Or shall we try to contribute to @DandyDeveloper's chart?

[zoispag]

I thought about contributing this myself but remembered that templating is not possible inside values file.

I think this is outside of our control. Is a comment inside the table enough? Or shall we try to contribute to @DandyDeveloper's chart?

I am not sure what contributing to @DandyDeveloper's chart would mean in this case.

It would be perfect if this was possible:

• when redis-ha.enabled=true and global.priorityClassName=system-cluster-critical
• then redis-ha.priorityClassName=system-cluster-critical and redis-ha.haproxy.priorityClassName=system-cluster-critical

But I am not sure how this can be achieved.

That being said, the best next thing is probably adding a reminder/note in README.md and/or in values.yaml, to include these values too.

[mkilchhofer]

So it seems that you are new to helms internals? Did you read https://helm.sh/docs/chart_template_guide/subcharts_and_globals/#global-chart-values ?
In summary:
.Values.global.xyz is a reserved section which is passed to all dependency charts inside a helm chart (also called subcharts).

What we can do is contributing the feature that the redis-ha chart is also respecting .Values.global.priorityClassName.

[mkilchhofer]

I filed a PR over there:

• feat: Add ability to define priorityClassName globally DandyDeveloper/charts#279

Let's see if @DandyDeveloper supports our use case 🤞

[zoispag]

I was not aware of the globals values namespace. I always thought it was just a naming convention among maintainers. TIL 💡

Thanks for sharing! 🙏🏼

And thanks for creating a PR to redis-ha chart! Let's see! 🤞🏼

[DandyDeveloper]

Sorry for the delay. I see no issue with this, especially with the benefit of this making it more compatible with other charts that depend on this as a dep.

Working through the PR.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.