Ensure `ttlSecondsAfterFinished` is set on Job resources in static deploy manifests

[marshall007]

What this PR does / why we need it:

Job resources are not automatically pruned after completion which prevents in-place version upgrades when deploying from static manifests. See #7118.

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [x] Breaking change (fix or feature that would cause existing functionality to change)

Note: this is only a "breaking" change in the sense that the static deploy manifests will now assume capabilities of the API version batch/v1alpha1 are available (i.e. k8s >=1.12).

Which issue/s this PR fixes

fixes #7118

How Has This Been Tested?

This change is inline with the existing functionality of the helm chart. Please see #7118 for steps on how to reproduce the original issue.

Checklist:

• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [x] I've read the CONTRIBUTION guide
• [ ] I have added tests to cover my changes.
• [ ] All new and existing tests passed.

[k8s-ci-robot]

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
• If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
• Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: [email protected]

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. I understand the commands that are listed here.

[k8s-ci-robot]

Welcome @marshall007!

It looks like this is your first PR to kubernetes/ingress-nginx 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/ingress-nginx has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. :smiley:

[k8s-ci-robot]

Hi @marshall007. Thanks for your PR.

I'm waiting for a kubernetes 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.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: marshall007 To complete the pull request process, please assign bowei after the PR has been reviewed. You can assign the PR to them by writing /assign @bowei 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:

• OWNERS

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

[rikatz]

Hi, thanks for your PR.

I'm not sure right now we can add this without breaking users, and let me explain: this feature has been in Alpha from v1.12 to v1.21, meaning that its feature gate was defaulted to false until 3 months ago -> https://github.com/kubernetes/kubernetes/commit/880bbdad2346be9eaf73a3ebef76692374a70f2a

We support k8s >= v1.16, meaning that anyone deploying ingress nginx in versions previous to v1.21 will get an error because this field is not supported (or worst, will have a no-op)

We can discuss about supporting this specifically in a future release, but then make clear to users that the new release will be supported from v1.21 forward.

[marshall007]

@rikatz thanks for the explanation, makes total sense! I didn't realize just how recently the TTLAfterFinished feature gate was promoted to beta.

... anyone deploying ingress nginx in versions previous to v1.21 will get an error because this field is not supported (or worst, will have a no-op)

For clarification and my own understanding, is it correct that versions <1.21 would see an error here? My understanding was that, for native resources, when an unknown field is specified it is ignored/dropped.

I agree with you that in general the no-op behavior is bad, but in this very specific instance it may be desirable in the sense that we could improve the UX of static manifest deployments on >=1.21 (or those with the TTLAfterFinished gate manually enabled) without modifying the behavior at all on previous versions.

---

It's not ideal, but perhaps another option worth considering in the immediate term would be to not include (or at least use more stable) version labels on the admission-create/admission-patch jobs. Then we wouldn't attempt to patch these job resources that are not allowed to be patched between minor version upgrades. Based on my understanding, there would be zero implications if these jobs were only run on the first deployment.

[tao12345666333]

@rikatz This is very similar to #7145, we can discuss it at next community meetings. About our compatibility matrix.

[rikatz]

/area helm

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[rikatz]

Just so we wont miss this.

https://github.com/kubernetes/enhancements/blob/master/keps/sig-apps/592-ttl-after-finish/kep.yaml

ttlAfterFinish will be GA'd in v1.23 and it's BEta (enabled by default) since v1.21.

I guess that we can, as soon as v1.23 is released merge this one as well and maybe use something like https://github.com/kubernetes/ingress-nginx/blob/legacy/charts/ingress-nginx/templates/controller-ingressclass.yaml#L1 to proper add or remove this directive

[rikatz]

/lifecycle active

[rikatz]

OK, not that this is stable in v1.23 I think we can move forward to it.

So let's "start from scratch" here.

I'm all about accepting this PR, I just want to make sure users will have the right behavior. It's beta in v1.21 and stable in v1.23 (https://github.com/kubernetes/enhancements/blob/master/keps/sig-apps/592-ttl-after-finish/kep.yaml#L37) so we need to make sure this feature is used only from >=v1.21

I've seen some other PR here (https://github.com/kubernetes/ingress-nginx/pull/8000) targeting specific versions. Is this something we should work together? Is this something specific that we can deal in helm chart templates only with the apiVersion helper?

Let's make this get merged before the end of january :)

[rikatz]

@marshall007 ping

[rikatz]

/assign

[k8s-ci-robot]

@marshall007: PR needs rebase.

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.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[rikatz]

/lifecycle active @marshall007 please rebase and we can release this now :) v1.24 is right there and we can support it

[rikatz]

/check-cla /retest

[k8s-ci-robot]

@marshall007: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-ingress-nginx-boilerplate	d75d24e65ef5a63b667b1aed0e34da802d72edcc	link	true	/test pull-ingress-nginx-boilerplate

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Reopen this PR with /reopen
• Mark this PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

[k8s-ci-robot]

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Reopen this PR with /reopen
• Mark this PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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.