Skip to content

chore: support 'imagePullSecrets' propagation#8

Closed
heyvister1 wants to merge 1 commit intoMellanox:masterfrom
heyvister1:fix-image-pull-secret
Closed

chore: support 'imagePullSecrets' propagation#8
heyvister1 wants to merge 1 commit intoMellanox:masterfrom
heyvister1:fix-image-pull-secret

Conversation

@heyvister1
Copy link

Added imagePullSecrets propagation helper function, to support the following options:

  1. Current chart's .Values.imagePullSecrets
  2. Root chart .Values.global.imagePullSecrets

@heyvister1 heyvister1 force-pushed the fix-image-pull-secret branch from dd28c75 to 6bd7295 Compare July 6, 2025 19:47
Signed-off-by: Ido Heyvi <iheyvi@nvidia.com>
@heyvister1 heyvister1 requested a review from almaslennikov July 6, 2025 19:47
Copy link
Collaborator

@e0ne e0ne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should we do this? we can propagate imagePullSecrets for this chart like we do for sriov operator in NVIDIA Network Operator project

@heyvister1
Copy link
Author

heyvister1 commented Jul 7, 2025

Why should we do this? we can propagate imagePullSecrets for this chart like we do for sriov operator in NVIDIA Network Operator project

The motivation was to refrain of declaring imagePullSecrects: ["foo-secret"] multiple time in network-operator's values.yaml. And declare it once, globally. Otherwise, users will need to fill in its content per each sub-chart

@heyvister1 heyvister1 requested a review from e0ne July 7, 2025 08:00
@e0ne
Copy link
Collaborator

e0ne commented Jul 7, 2025

Why should we do this? we can propagate imagePullSecrets for this chart like we do for sriov operator in NVIDIA Network Operator project

The motivation was to refrain of declaring imagePullSecrects: ["foo-secret"] multiple time in network-operator's values.yaml. And declare it once, globally. Otherwise, users will need to fill in its content per each sub-chart

I really don't like the idea to change upstream API

@heyvister1
Copy link
Author

Why should we do this? we can propagate imagePullSecrets for this chart like we do for sriov operator in NVIDIA Network Operator project

The motivation was to refrain of declaring imagePullSecrects: ["foo-secret"] multiple time in network-operator's values.yaml. And declare it once, globally. Otherwise, users will need to fill in its content per each sub-chart

I really don't like the idea to change upstream API

I can suggest this fix for upstream

@adrianchiris
Copy link

id suggest to do the opposite. having this skew is not good practice.

i.e suggest changes upstream and let them be synced back to this fork.

@almaslennikov
Copy link

almaslennikov commented Jul 7, 2025

I agree with @adrianchiris. I think that we should go with the same approach that we have for the SR-IOV operator for now and let's try to push this PR upstream first before doing any changes in the fork.

@heyvister1
Copy link
Author

Will wait until this change is merged by kubernetes-sigs#2191

@heyvister1 heyvister1 closed this Jul 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants