Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bitnami/common] _affinities namespace #12668

Closed
dosmanak opened this issue Sep 26, 2022 · 7 comments · Fixed by #12932
Closed

[bitnami/common] _affinities namespace #12668

dosmanak opened this issue Sep 26, 2022 · 7 comments · Fixed by #12932
Assignees
Labels
common solved tech-issues The user has a technical issue about an application

Comments

@dosmanak
Copy link
Contributor

Name and Version

bitnami/common 1.16.0

What steps will reproduce the bug?

related to #10804

Hello. I struggle with the helmchart inside kustomize. The namespace transformer can't reach this value and change it accordingly.

Here is the question: Why even have the podAffinityTerm populated with namespaces list with this single value? It seems to me this setup has the same effect as not defined at all.

Are you using any custom parameters or values?

No response

What is the expected behavior?

The podAffinityTerm applies to namespace of redis deployment as in case where namespace is not specified in the podAffinityTerm docs.

   namespaces	<[]string>
     namespaces specifies a static list of namespace names that the term applies
     to. The term is applied to the union of the namespaces listed in this field
     and the ones selected by namespaceSelector. null or empty namespaces list
     and null namespaceSelector means "this pod's namespace"

What do you see instead?

If the resource is created for different kubectl context than the current. The current namespace is injected into podAffinityTerm.namespaces which does not correspond with the namespace of pod.

Additional information

I suggest the template

https://github.com/bitnami/charts/blob/master/bitnami/common/templates/_affinities.tpl#L65

should not contain

    namespaces:
      - {{ include "common.names.namespace" .context | quote }}
@dosmanak dosmanak added the tech-issues The user has a technical issue about an application label Sep 26, 2022
@bitnami-bot bitnami-bot added this to Triage in Support Sep 26, 2022
@github-actions github-actions bot added the triage Triage is needed label Sep 26, 2022
@fmulero
Copy link
Collaborator

fmulero commented Sep 26, 2022

Thanks a lot for sharing your concerns.

Sorry but I don't fully understand this:

If the resource is created for different kubectl context than the current. The current namespace is injected into podAffinityTerm.namespaces which does not correspond with the namespace of pod.

If the chart is released in a particular namespace (helm install -n test ....), that namespace should be injected in podAffinityTerm.namespaces. Have you find any issue? Could you provide the steps to reproduce it?

@github-actions github-actions bot moved this from Triage to Pending in Support Sep 26, 2022
@dosmanak
Copy link
Contributor Author

As I stated before, I am using the helm chart with kustomize. The flag you mention should be handed to helm binary here
https://github.com/kubernetes-sigs/kustomize/blob/master/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go#L275

In kustomize there is namespace transformer that can override the namespace in lower layers of kustomize hierarchy.

Unfortunately the kustomize can not modify helm parameteters specified in helmchart generator and patching the final resource is very troublesome here.

I doubt it will be resolved quickly in kustomize (see long term helm support issue).

From my description in this issue, the current behavior of AffinityTerm is indentical as if namespaces were not declared at all and so if you omit it, that would be helpful for me :-).

@bitnami-bot bitnami-bot moved this from Pending to Triage in Support Sep 26, 2022
@fmulero
Copy link
Collaborator

fmulero commented Sep 27, 2022

OK, I got it. Could you open a PR linking this issue? I think that's the best option to include the changes, our capacity is limited and we couldn't give you an ETA.

@github-actions github-actions bot moved this from Triage to Pending in Support Sep 27, 2022
dosmanak added a commit to dosmanak/bitnami-charts that referenced this issue Oct 12, 2022
the current context namespace defined is identical as not defined at all

> null or empty namespaces list and null namespaceSelector means "this pod's namespace"

docs: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#podaffinityterm-v1-core
issue: bitnami#12668
dosmanak added a commit to dosmanak/bitnami-charts that referenced this issue Oct 12, 2022
the current context namespace defined is identical as not defined at all

> null or empty namespaces list and null namespaceSelector means "this pod's namespace"

docs: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#podaffinityterm-v1-core
issue: bitnami#12668
dosmanak added a commit to dosmanak/bitnami-charts that referenced this issue Oct 12, 2022
the current context namespace defined is identical as not defined at all

> null or empty namespaces list and null namespaceSelector means "this pod's namespace"

docs: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#podaffinityterm-v1-core
issue: bitnami#12668
Signed-off-by: Petr Studeny <[email protected]>
@dosmanak
Copy link
Contributor Author

OK, I got it. Could you open a PR linking this issue? I think that's the best option to include the changes, our capacity is limited and we couldn't give you an ETA.

Hi. I was ill. I pushed the PR I hope I will handle all the bureaucracy.

@bitnami-bot bitnami-bot moved this from Pending to Triage in Support Oct 12, 2022
@fmulero
Copy link
Collaborator

fmulero commented Oct 13, 2022

Thanks a lot Petr! We will come back soon with the review

@github-actions github-actions bot moved this from Triage to Pending in Support Oct 13, 2022
@fmulero fmulero moved this from Pending to In progress in Support Oct 13, 2022
@github-actions github-actions bot added in-progress and removed triage Triage is needed labels Oct 13, 2022
@carrodher carrodher moved this from In progress to Pending in Support Oct 20, 2022
@github-actions
Copy link

This Issue has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thanks for the feedback.

@github-actions github-actions bot added the stale 15 days without activity label Oct 31, 2022
@dosmanak
Copy link
Contributor Author

Copy paste from the pull request:
carrodher moved this from Pending to In progress in Support yesterday

@github-actions github-actions bot moved this from Pending to In progress in Support Oct 31, 2022
@fmulero fmulero removed the stale 15 days without activity label Oct 31, 2022
fmulero pushed a commit that referenced this issue Oct 31, 2022
#12932)

the current context namespace defined is identical as not defined at all

> null or empty namespaces list and null namespaceSelector means "this pod's namespace"

docs: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#podaffinityterm-v1-core
issue: #12668
Signed-off-by: Petr Studeny <[email protected]>

Signed-off-by: Petr Studeny <[email protected]>
Signed-off-by: Carlos Rodríguez Hernández <[email protected]>
Co-authored-by: Carlos Rodríguez Hernández <[email protected]>
@bitnami-bot bitnami-bot moved this from In progress to Solved in Support Oct 31, 2022
fire-ant pushed a commit to fire-ant/charts that referenced this issue Nov 6, 2022
bitnami#12932)

the current context namespace defined is identical as not defined at all

> null or empty namespaces list and null namespaceSelector means "this pod's namespace"

docs: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#podaffinityterm-v1-core
issue: bitnami#12668
Signed-off-by: Petr Studeny <[email protected]>

Signed-off-by: Petr Studeny <[email protected]>
Signed-off-by: Carlos Rodríguez Hernández <[email protected]>
Co-authored-by: Carlos Rodríguez Hernández <[email protected]>
@fmulero fmulero removed this from Solved in Support Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common solved tech-issues The user has a technical issue about an application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants