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

Add multi-cloud support (#634) #973

Merged
merged 11 commits into from
Dec 18, 2023
Merged

Conversation

mparada-suva
Copy link
Contributor

@mparada-suva mparada-suva commented Dec 18, 2023

What does this PR do?

If you modified files in the ./charts/jenkins/ directory, please also include the following:

Submitter checklist

Preview Give feedback

Special notes for your reviewer

I did not do the tasks in "Submitter checklist" because I want to gather feedback on my changes. If I'm going in the right direction I will be happy to do all those tasks!

I tried to do the additionalClouds feature the same way the additionalAgents feature was built in: by looping through the items and merging to the main template. I also stored the state before the loop and restored it after the loop ends.

@mparada-suva mparada-suva requested a review from a team as a code owner December 18, 2023 10:22
@@ -206,6 +207,94 @@ jenkins:
{{- end }}
{{- end }}
{{- end }}
{{- if .Values.additionalClouds }}
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

Thanks, looks great, can you add a changelog entry please

@mparada-suva
Copy link
Contributor Author

mparada-suva commented Dec 18, 2023

Wow! This review was fast ⚡ Thanks!

@mparada-suva
Copy link
Contributor Author

I see now some IDs in the tests are failing. And the other error is regarding the jenkins-agent image tag. I'll fix those and push the changes.

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

LGTM

@timja timja merged commit 346df94 into jenkinsci:main Dec 18, 2023
5 checks passed
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.

Add support for adding more clouds via values.yaml
2 participants