KEP-2307: Graduate JobTrackingWithFinalizers to GA #3482

alculquicondor · 2022-08-29T17:52:41Z

One-line PR description: Add GA and deprecation notes

Issue link: Job tracking without lingering Pods #2307

Other comments:
- Add metric job_pod_tracking_finalizer.
- Add legacy to finalizers migration under MigrateJobLegacyTracking feature gate.
- Add known failure modes.

alculquicondor · 2022-08-29T17:54:35Z

/assign @soltysh
cc @ahg-g @mimowo

alculquicondor · 2022-08-29T17:55:14Z

FYI @liggitt

ahg-g · 2022-08-29T19:30:45Z

keps/sig-apps/2307-job-tracking-without-lingering-pods/README.md

+the job controller migrates jobs with legacy tracking to tracking with finalizers.
+
+If a Job doesn't have the annotation `batch.kubernetes.io/job-completion`, it
+means that is not currently tracked with finalizers. The job controller starts


Suggested change

means that is not currently tracked with finalizers. The job controller starts

means that it is not currently tracked with finalizers. The job controller starts

ahg-g · 2022-08-29T19:34:35Z

keps/sig-apps/2307-job-tracking-without-lingering-pods/README.md

+- For the remaining Jobs, the Job controller already accounted most of the
+  finished Pods in the status. The controller might leave some
+  finished Pods unaccounted, if they finish before the controller has a chance
+  to add a finalizer. This situation is no worse that the legacy tracking


Suggested change

to add a finalizer. This situation is no worse that the legacy tracking

to add a finalizer. This situation is no worse than the legacy tracking

ahg-g · 2022-08-29T20:49:35Z

keps/sig-apps/2307-job-tracking-without-lingering-pods/README.md

+
+In 1.26:
+
+- Declare deprecation of annotation `batch.kubernetes.io/job-completion` in


just curious, are we expecting anyone to use/rely on this annotation?

You never know

👍 on deprecating and announcing that fact, we've been bitten in the past several times on alpha things that users started heavily rely on 😞

ahg-g · 2022-08-29T20:55:42Z

keps/sig-apps/2307-job-tracking-without-lingering-pods/README.md

+
+## Migrating Jobs with legacy tracking
+
+Starting in 1.26, when the feature gate `MigrateJobLegacyTracking` is enabled,


this is a new feature gate, but it is not described before. Are we currently not migrating legacy jobs and they use continue to use the legacy path?

Worth adding the reason for this which is to facilitate the migration of jobs that may run across two upgrades. It looks like an overkill since jobs are not like deployments in that they don't live forever, but there are probably cases where people upgrade their clusters back to back.

Added notes

keps/sig-apps/2307-job-tracking-without-lingering-pods/README.md

soltysh · 2022-09-13T20:34:51Z

keps/sig-apps/2307-job-tracking-without-lingering-pods/README.md

+
+In 1.26:
+
+- Declare deprecation of annotation `batch.kubernetes.io/job-completion` in


👍 on deprecating and announcing that fact, we've been bitten in the past several times on alpha things that users started heavily rely on 😞

soltysh · 2022-09-13T20:40:37Z

keps/sig-apps/2307-job-tracking-without-lingering-pods/README.md

+   `.status.phase=(Pending/Running)`.
+2. Ignore pods with `.status.phase=(Complete/Failed)` that don't have the `batch.kubernetes.io/job-completion`.
+   They are considered to be already counted in `.status.(failed/succeeded)`.
+3. Add the annotation `batch.kubernetes.io/job-completion`.


This bit is probably the only one which I have some doubts around. Can you estimate how many jobs might be using the old mechanism assuming both cases:

with the feature on by default in 1.25.

with the feature off in 1.25.

From my quick calculations, the 2nd case will be the most impactful, since that's basically all the jobs using the old mechanism that need to be migrated. Is there something we can do here to make the pain to users to be smaller? Maybe advise them to run 1.25 with the feature on before the upgrade? Would that kind of approach make it more reliable?

Should we allow users to disable MigrateJobLegacyTracking at all?

Or maybe we should do it gradually, iow. new jobs should use only the new tracking, but leave the old ones as they where?

In general, I have no way to know.

For case 1, and assuming that admins are not doing "binge upgrading", it should be close to zero. There are 4 months between releases, which should be plenty of time for even the slowest jobs to finish.

Case 2 can be directly impacted.

I'm thinking that the recommendation should be either of these:

If JobTrackingWithFinalizers was enabled in 1.25, enable MigrateJobLegacyTracking in 1.26.

If the feature was disabled in 1.24, disable MigrateJobLegacyTracking in 1.26. They remaining jobs will be migrated in 1.27.

We could also never introduce the code for migration and document this:
If a Job that was created in 1.25- with JobTrackingWithFinalizers disabled, and it is still running in 1.26, the job controller would recreate all its pods upon upgrade to 1.27.

For people that had JobTrackingWithFinalizers enabled in 1.25, that gives them 2 releases of buffer for their jobs to finish.

I think the migration code is an overkill, the code is too complex as is and this will only increase sources of bugs/errors and misconfigurations. I think it is sufficient to have a release note indicating that upgrading clusters back to back through two releases may disrupt running jobs.

Totally agree with Abdullah about that migration being overkill (and potential source of unwanted and unexpected problems). All we should do is to ensure good communication about these upcoming changes.

wojtek-t · 2022-09-14T13:18:32Z

/assign

@alculquicondor - please ping me on Slack once you have SIG lgtm

soltysh

One suggestion, otherwise
/lgtm
/approve

soltysh · 2022-09-22T10:26:22Z

keps/sig-apps/2307-job-tracking-without-lingering-pods/README.md

+      [#111646](https://github.com/kubernetes/kubernetes/pull/111646)).
+      In newer versions, this can still happen if there is a buggy webhook
+      that prevents pod updates to remove finalizers.
+    - Testing: Discovered bugs are covered by unit and integration tests.


I'd add here the problem you described at the beginning, ie. the one that we recommend folks running with JobTrackingWithFinalizers in 1.25 and 1.26 to ensure all jobs with batch.kubernetes.io/job-completion are completed prior to upgrade to 1.27. I doubt we'll be updating the KEP then, so let's add it right away.

On top of that, I'd suggest writing a blog post for 1.26 release describing this and recommendations.

Good ideas. Added here and in the graduation criteria.

alculquicondor · 2022-09-22T14:57:29Z

@wojtek-t this is ready for PRR.

wojtek-t · 2022-09-23T07:27:56Z

This is great!

/lgtm
/approve PRR

k8s-ci-robot · 2022-09-23T07:28:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, soltysh, wojtek-t

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~keps/prod-readiness/OWNERS~~ [wojtek-t]
~~keps/sig-apps/OWNERS~~ [soltysh]

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 added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory labels Aug 29, 2022

k8s-ci-robot requested review from kow3ns and soltysh August 29, 2022 17:53

k8s-ci-robot added the sig/apps Categorizes an issue or PR as relevant to SIG Apps. label Aug 29, 2022

alculquicondor force-pushed the job-tracking-ga branch from 486bb61 to 01fe5ea Compare August 29, 2022 17:54

Add GA and deprecation notes to JobTrackingWithFinalizers

7823232

alculquicondor force-pushed the job-tracking-ga branch from 01fe5ea to 7823232 Compare August 29, 2022 17:54

k8s-ci-robot assigned soltysh Aug 29, 2022

ahg-g reviewed Aug 29, 2022

View reviewed changes

clarify MigrateJobLegacyTracking

5887977

soltysh reviewed Sep 13, 2022

View reviewed changes

alculquicondor mentioned this pull request Sep 13, 2022

Job tracking without lingering Pods #2307

Closed

12 tasks

k8s-ci-robot assigned wojtek-t Sep 14, 2022

Simplify

6e0acd6

alculquicondor force-pushed the job-tracking-ga branch from 4adc040 to 6e0acd6 Compare September 15, 2022 15:22

kikisdeliveryservice changed the title ~~2307: Graduate JobTrackingWithFinalizers to GA~~ KEP-2307: Graduate JobTrackingWithFinalizers to GA Sep 16, 2022

soltysh approved these changes Sep 22, 2022

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 22, 2022

Add legacy job pods recreation to known failure

e5689ba

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 22, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 23, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 23, 2022

k8s-ci-robot merged commit a4b5f5c into kubernetes:master Sep 23, 2022

k8s-ci-robot added this to the v1.26 milestone Sep 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KEP-2307: Graduate JobTrackingWithFinalizers to GA #3482

KEP-2307: Graduate JobTrackingWithFinalizers to GA #3482

alculquicondor commented Aug 29, 2022

alculquicondor commented Aug 29, 2022

alculquicondor commented Aug 29, 2022

ahg-g Aug 29, 2022

alculquicondor Sep 9, 2022

ahg-g Aug 29, 2022

alculquicondor Sep 9, 2022

ahg-g Aug 29, 2022

alculquicondor Sep 9, 2022

soltysh Sep 13, 2022

wojtek-t Sep 23, 2022

ahg-g Aug 29, 2022

ahg-g Aug 29, 2022

alculquicondor Sep 9, 2022

soltysh Sep 13, 2022

soltysh Sep 13, 2022

soltysh Sep 13, 2022

soltysh Sep 13, 2022

alculquicondor Sep 13, 2022

ahg-g Sep 14, 2022 •

edited

Loading

alculquicondor Sep 14, 2022

soltysh Sep 22, 2022

wojtek-t commented Sep 14, 2022

soltysh left a comment

soltysh Sep 22, 2022

soltysh Sep 22, 2022

alculquicondor Sep 22, 2022

alculquicondor commented Sep 22, 2022

wojtek-t commented Sep 23, 2022

k8s-ci-robot commented Sep 23, 2022

	means that is not currently tracked with finalizers. The job controller starts
	means that it is not currently tracked with finalizers. The job controller starts

	to add a finalizer. This situation is no worse that the legacy tracking
	to add a finalizer. This situation is no worse than the legacy tracking


		In 1.26:

		- Declare deprecation of annotation `batch.kubernetes.io/job-completion` in


		## Migrating Jobs with legacy tracking

		Starting in 1.26, when the feature gate `MigrateJobLegacyTracking` is enabled,

KEP-2307: Graduate JobTrackingWithFinalizers to GA #3482

KEP-2307: Graduate JobTrackingWithFinalizers to GA #3482

Conversation

alculquicondor commented Aug 29, 2022

alculquicondor commented Aug 29, 2022

alculquicondor commented Aug 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahg-g Sep 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wojtek-t commented Sep 14, 2022

soltysh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alculquicondor commented Sep 22, 2022

wojtek-t commented Sep 23, 2022

k8s-ci-robot commented Sep 23, 2022

ahg-g Sep 14, 2022 •

edited

Loading