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

Added namespace creation to docker-registry-secrets Helm chart #3727

Conversation

Giridharan002
Copy link

Fixes issue where docker-registry-secrets Helm release fails to install
due to missing namespace. This commit adds a namespace field to the
values.yaml file with create set to true and name set to
jenkins-agents to create the namespace before installing the Helm chart.

Additionally, the imageCredentials.namespaces[1] value is updated to include the jenkins-agents namespace.

This resolves the issue and allows for successful installation of the docker-registry-secrets Helm chart.

Issue: #2278

@Giridharan002 Giridharan002 requested a review from a team March 23, 2023 17:35
@dduportal
Copy link
Contributor

Hi @Giridharan002 , it is really nice of you to propose a contribution.

However we cannot accept it for numerous reasons:

  • First, a pull request "out of the wild" is usually not the best place to start. Because a Pull Request is usually the "How", we require to agree on the "What" in the issue you would want to contribute to. We would want you to describe your solution (and how did you test it) as part of the issue Missing required namespaces for release docker-registry-secrets #2278.

  • Second, the change you are proposing is not valid for helmfile:

in clusters/doks.yaml: failed to read doks.yaml: reading document at index 1: yaml: unmarshal errors:
  line 18: cannot unmarshal !!map into string
  line 22: field version not found in type state.helmStateAlias
  line 29: field secrets not found in type state.helmStateAlias

You might want to check the README (https://github.com/jenkins-infra/kubernetes-management#how-to-debug-deployments) and test with the helmfile command line beforehand.

It looks like you are mixing helm and helmfile concept: I recommend you to check https://helmfile.readthedocs.io/en/latest/#getting-started in order to get a better understanding.

  • Finally, why would you want to only apply the change on only one of our clusters, while other clusters would be the target.

Under these elements, I'm gonna close this pull request until we can agree on a solution on the issue before writing code.

I'm sorry for not being able to accept your contribution, but the infrastructure might not be the easiest way to get started on contributing to an open source.

If you are still interested, you are welcome to contact us on the IRC channel #jenkins-infra (https://www.jenkins.io/chat/) to discuss a bit more (prefer EU working hours).

@dduportal dduportal closed this Mar 23, 2023
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.

2 participants