[stable/jenkins] Use StatefulSet instead of Deployment

[dylanpiergies]

What this PR does / why we need it:

Switch out the Jenkins master Deployment for a StatefulSet.

Which issue this PR fixes

Fixes the Jenkins element of #1863

Checklist

• DCO signed
• Chart Version bumped
• Variables are documented in the README.md
• Title of the PR starts with chart name (e.g. [stable/mychartname])

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dylanpiergies
To complete the pull request process, please assign mogaal
You can assign the PR to them by writing /assign @mogaal in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• stable/jenkins/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

Hi @dylanpiergies. Thanks for your PR.

I'm waiting for a helm member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[phisco]

Yes, please. Nobody would run Jenkins without persistence enabled, it is a stateful application and therefore should move to use a statefulset

[dylanpiergies]

I'm inclined to enhance it so that it can use volumeClaimTemplates as well, but one step at a time. ;)

[phisco]

I don't think it really make sense to have volumeclaimtemplates, as jenkins has no HA and there is no way to scale it to more than 1 replica. 🤷‍♂️

[dylanpiergies]

I don't think it really make sense to have volumeclaimtemplates, as jenkins has no HA and there is no way to scale it to more than 1 replica.

My thinking is that the difference in the way the lifecycle of the PV/PVC works with a volumeClaimTemplate can be useful. It would mean you're free to uninstall the chart without the Jenkins home volume being deleted.

[dylanpiergies]

@phisco Anyway, such discussions are precisely the reason I've kept it separate.

[wmcdona89]

@dylanpiergies thanks for the PR!

what would you think about making this an opt-in feature for now...perhaps via a master.statefulSet.enabled flag...and allow the chart to support using Deployment or StatefulSet. we could then consider making StatefulSet the default in a future major release.

[dylanpiergies]

@wmcdona89 Well, aside from the renamed values for the labels and annotations, this is a non-breaking change, at least theoretically. I was hoping to see what automation testing would occur prior to looking to fully assess the level of technical risk involved and whether any additional testing needs to be performed.

As for continuing to support Deployment, well on the one hand I kinda feel it's in bad taste to continue to support something that's plainly inappropriate (even arguably incorrect), as discussed on #1863. I definitely think of this change as more of a bug fix end of than a feature. On the other hand, yeah, it is quite a fundamental change and will probably raise alarm among the most risk-averse.

Personally, I think if the upgrade path from any sane configuration we can think of can be demonstrated to run smoothly, we should just cut over. Keeping support for Deployment will leave tech debt. Though if people want to take a softer approach, I'm willing to make the necessary changes to make the use of StatefulSet optional.

[mattfarina]

/ok-to-test

@wmcdona89 can you update from master to pull in the changes.

[dylanpiergies]

/assign mogaal

[torstenwalter]

/hold This chart will be moved to https://github.com/jenkinsci/helm-charts see for details #23562
Would be great if you could create the PR in the new location.