Skip to content

Commit

Permalink
Add PRR
Browse files Browse the repository at this point in the history 
Signed-off-by: Aldo Culquicondor <acondor@google.com>
  • Loading branch information
@alculquicondor
alculquicondor committed Jan 27, 2021
1 parent 54f33e9 commit 4164b63
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 154 deletions.
3 changes: 3 additions & 0 deletions keps/prod-readiness/sig-apps/2307.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kep-number: 2307
alpha:
approver: "@wojtek-t"
230 changes: 77 additions & 153 deletions keps/sig-apps/2307-job-tracking-wihout-lingering-pods/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ SIG Architecture for cross-cutting KEPs).
- [Pod adoption](#pod-adoption)
- [Test Plan](#test-plan)
- [Graduation Criteria](#graduation-criteria)
- [Alpha](#alpha)
- [Alpha -&gt; Beta Graduation](#alpha---beta-graduation)
- [Beta -&gt; GA Graduation](#beta---ga-graduation)
- [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy)
- [Version Skew Strategy](#version-skew-strategy)
- [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire)
Expand All @@ -88,18 +91,17 @@ SIG Architecture for cross-cutting KEPs).
- [Implementation History](#implementation-history)
- [Drawbacks](#drawbacks)
- [Alternatives](#alternatives)
- [Infrastructure Needed (Optional)](#infrastructure-needed-optional)
<!-- /toc -->

## Release Signoff Checklist

Items marked with (R) are required *prior to targeting to a milestone / release*.

- [x] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR)
- [ ] (R) KEP approvers have approved the KEP status as `implementable`
- [x] (R) KEP approvers have approved the KEP status as `implementable`
- [x] (R) Design details are appropriately documented
- [x] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input
- [ ] (R) Graduation criteria is in place
- [x] (R) Graduation criteria is in place
- [x] (R) Production readiness review completed
- [x] (R) Production readiness review approved
- [ ] "Implementation History" section is up-to-date for milestone
Expand Down Expand Up @@ -256,80 +258,39 @@ the owner reference.

### Test Plan

<!--
**Note:** *Not required until targeted at a release.*
Consider the following in developing a test plan for this enhancement:
- Will there be e2e and integration tests, in addition to unit tests?
- How will it be tested in isolation vs with other components?
No need to outline all of the test cases, just the general strategy. Anything
that would count as tricky in the implementation, and anything particularly
challenging to test, should be called out.
All code is expected to have adequate tests (eventually with coverage
expectations). Please adhere to the [Kubernetes testing guidelines][testing-guidelines]
when drafting this test plan.
[testing-guidelines]: https://git.k8s.io/community/contributors/devel/sig-testing/testing.md
-->
- Unit tests:
- Job sync with feature gate enabled.
- Removal of finalizers when feature gate is disabled.
- Tracking of terminating Pods.
- Integration tests:
- Job tracking with feature enabled.
- Tracking of terminating Pods.
- Transition from feature enabled to disabled and enabled again.
- E2E test:
- Job tracking with feature enabled.

### Graduation Criteria

<!--
**Note:** *Not required until targeted at a release.*
Define graduation milestones.
These may be defined in terms of API maturity, or as something else. The KEP
should keep this high-level with a focus on what signals will be looked at to
determine graduation.
Consider the following in developing the graduation criteria for this enhancement:
- [Maturity levels (`alpha`, `beta`, `stable`)][maturity-levels]
- [Deprecation policy][deprecation-policy]
Clearly define what graduation means by either linking to the [API doc
definition](https://kubernetes.io/docs/concepts/overview/kubernetes-api/#api-versioning)
or by redefining what graduation means.
In general we try to use the same stages (alpha, beta, GA), regardless of how the
functionality is accessed.
#### Alpha

[maturity-levels]: https://git.k8s.io/community/contributors/devel/sig-architecture/api_changes.md#alpha-beta-and-stable-versions
[deprecation-policy]: https://kubernetes.io/docs/reference/using-api/deprecation-policy/
Below are some examples to consider, in addition to the aforementioned [maturity levels][maturity-levels].
- Implementation:
- Job tracking without lingering Pods
- Removal of finalizer when feature gate is disabled.
- Tests: unit, integration, E2E

#### Alpha -> Beta Graduation

- Gather feedback from developers and surveys
- Complete features A, B, C
- Implementation:
- Index tracking for [Indexed Jobs](http://git.k8s.io/enhancements/keps/sig-apps/2214-indexed-job)
- Metrics:
- latency
- errors
- Tests are in Testgrid and linked in KEP

#### Beta -> GA Graduation

- N examples of real-world usage
- N installs
- More rigorous forms of testing—e.g., downgrade tests and scalability tests
- Allowing time for feedback
**Note:** Generally we also wait at least two releases between beta and
GA/stable, because there's no opportunity for user feedback, or even bug reports,
in back-to-back releases.
#### Removing a Deprecated Flag
- Announce deprecation and support policy of the existing flag
- Two versions passed since introducing the functionality that deprecates the flag (to address version skew)
- Address feedback on usage/changed behavior, provided on GitHub issues
- Deprecate the flag
**For non-optional features moving to GA, the graduation criteria must include
[conformance tests].**
[conformance tests]: https://git.k8s.io/community/contributors/devel/sig-architecture/conformance-tests.md
-->
- E2E test graduates to conformance.
- Job tracking scales to 10^5 completions.

### Upgrade / Downgrade Strategy

Expand All @@ -352,52 +313,20 @@ the Job controller syncs a Job:

### Version Skew Strategy

<!--
If applicable, how will the component handle version skew with other
components? What are the guarantees? Make sure this is in the test plan.
Consider the following in developing a version skew strategy for this
enhancement:
- Does this enhancement involve coordinating behavior in the control plane and
in the kubelet? How does an n-2 kubelet without this feature available behave
when this feature is used?
- Will any other components on the node change? For example, changes to CSI,
CRI or CNI may require updating that component before the kubelet.
-->
No implications to node runtime.

## Production Readiness Review Questionnaire

<!--
Production readiness reviews are intended to ensure that features merging into
Kubernetes are observable, scalable and supportable; can be safely operated in
production environments, and can be disabled or rolled back in the event they
cause increased failures in production. See more in the PRR KEP at
https://git.k8s.io/enhancements/keps/sig-architecture/1194-prod-readiness.
The production readiness review questionnaire must be completed and approved
for the KEP to move to `implementable` status and be included in the release.
In some cases, the questions below should also have answers in `kep.yaml`. This
is to enable automation to verify the presence of the review, and to reduce review
burden and latency.
The KEP must have a approver from the
[`prod-readiness-approvers`](http://git.k8s.io/enhancements/OWNERS_ALIASES)
team. Please reach out on the
[#prod-readiness](https://kubernetes.slack.com/archives/CPNHUMN74) channel if
you need any help or guidance.
-->

### Feature Enablement and Rollback

_This section must be completed when targeting alpha to a release._

* **How can this feature be enabled / disabled in a live cluster?**
- [ ] Feature gate (also fill in values in `kep.yaml`)
- Feature gate name:
- [x] Feature gate (also fill in values in `kep.yaml`)
- Feature gate name: JobTrackingWithoutLingeringPods
- Components depending on the feature gate:
- kube-apiserver
- kube-controller-manager
- [ ] Other
- Describe the mechanism:
- Will enabling / disabling the feature require downtime of the control
Expand All @@ -406,22 +335,29 @@ _This section must be completed when targeting alpha to a release._
of a node? (Do not assume `Dynamic Kubelet Config` feature is enabled).

* **Does enabling the feature change any default behavior?**
Any change of default behavior may be surprising to users or break existing
automations, so be extremely careful here.

Yes.

- Removing terminated Pods doesn't affect Job status.

<<[UNRESOLVED: should we count terminating pods?]>>
- Pods removed by the user or other controllers count towards failures or
completions.
<<[/UNRESOLVED]>>

* **Can the feature be disabled once it has been enabled (i.e. can we roll back
the enablement)?**
Also set `disable-supported` to `true` or `false` in `kep.yaml`.
Describe the consequences on existing workloads (e.g., if this is a runtime
feature, can it break the existing applications?).

Yes.
The job controller removes finalizers in this case.

* **What happens if we reenable the feature if it was previously rolled back?**

Existing Jobs are tracked with lingering Pods.

* **Are there any tests for feature enablement/disablement?**
The e2e framework does not currently support enabling or disabling feature
gates. However, unit tests in each component dealing with managing data, created
with and without the feature, are necessary. At the very least, think about
conversion tests if API types are being modified.

Yes, there are integration tests.

### Rollout, Upgrade and Rollback Planning

Expand Down Expand Up @@ -504,45 +440,43 @@ _For GA, this section is required: approvers should be able to confirm the
previous answers based on experience in the field._

* **Will enabling / using this feature result in any new API calls?**
Describe them, providing:
- API call type (e.g. PATCH pods)
- estimated throughput
- originating component(s) (e.g. Kubelet, Feature-X-controller)
focusing mostly on:
- components listing and/or watching resources they didn't before
- API calls that may be triggered by changes of some Kubernetes resources
(e.g. update of object X triggers new updates of object Y)
- periodic API calls to reconcile state (e.g. periodic fetching state,
heartbeats, leader election, etc.)

- PATCH Pods, to remove finalizers.
- estimated throughput: one per created Pod, when Pod finishes or is removed.
- originating component: kube-controller-manager
- PUT Job status, to keep track of uncounted Pods.
- estimated throughput: at least one per Job sync. Extra calls are throttled
at 1 per minute.
- originating component: kube-controller-manager.

* **Will enabling / using this feature result in introducing new API types?**
Describe them, providing:
- API type
- Supported number of objects per cluster
- Supported number of objects per namespace (for namespace-scoped objects)

No.

* **Will enabling / using this feature result in any new calls to the cloud
provider?**

No.

* **Will enabling / using this feature result in increasing size or count of
the existing API objects?**
Describe them, providing:
- API type(s):
- Estimated increase in size: (e.g., new annotation of size 32B)
- Estimated amount of new objects: (e.g., new Object X for every existing Pod)

- Pod
- Estimated increase: new finalizer of 33 bytes.
- Job status
- Estimated increase: new array temporarily containing terminated Pod UIDs.
The job controller caps the size of the array.

* **Will enabling / using this feature result in increasing time taken by any
operations covered by [existing SLIs/SLOs]?**
Think about adding additional work or introducing new steps in between
(e.g. need to do X to start a container), etc. Please describe the details.

No existing SLIs/SLOs for Jobs.

* **Will enabling / using this feature result in non-negligible increase of
resource usage (CPU, RAM, disk, IO, ...) in any components?**
Things to keep in mind include: additional in-memory state, additional
non-trivial computations, excessive access to disks (including increased log
volume), significant amount of data sent and/or received over network, etc.
This through this both in small and large cases, again with respect to the
[supported limits].

Additional memory to hold terminated Pods and the status of removing their
finalizers.

### Troubleshooting

Expand Down Expand Up @@ -586,22 +520,12 @@ Major milestones might include:

## Drawbacks

<!--
Why should this KEP _not_ be implemented?
-->
- Extra API calls and temporarily bigger Job status. However, without them
it's impossible to ever scale the Job controller to deal with greater amount
of Jobs or Jobs with greater amount of Pods.

## Alternatives

<!--
What other approaches did you consider, and why did you rule them out? These do
not need to be as detailed as the proposal, but should include enough
information to express the idea and why it was not acceptable.
-->

## Infrastructure Needed (Optional)

<!--
Use this section if you need things from the project/SIG. Examples include a
new subproject, repos requested, or GitHub details. Listing these here allows a
SIG to get the process for these resources started right away.
-->
- Keep a list of created Pod UIDs, clearing them when they have been accounted.
This has the benefit of requiring less Job status updates. On the other hand,
the size of the updates is unbounded.
5 changes: 4 additions & 1 deletion keps/sig-apps/2307-job-tracking-wihout-lingering-pods/kep.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@ kep-number: 2307
authors:
- "@alculquicondor"
owning-sig: sig-apps
status: provisional
status: implementable
creation-date: 2020-01-21
reviewers:
- "@erictune"
- "@lavalamp"
- "@solthys"
approvers:
- "@janetkuo"
prr-approvers:
- "@wojtek-t"

# The target maturity stage in the current dev cycle for this KEP.
stage: alpha
Expand Down

0 comments on commit 4164b63

Please sign in to comment.