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.

Sign up for GitHub

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

Jump to bottom

Umbrella Issue for Server Side Apply #73723

Closed
jennybuckley opened this issue Feb 4, 2019 · 27 comments
Closed

Umbrella Issue for Server Side Apply #73723

jennybuckley opened this issue Feb 4, 2019 · 27 comments
Assignees
@jennybuckley
Labels
kind/design Categorizes issue or PR as related to design. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Milestone
v1.18

Comments

@jennybuckley
Copy link

jennybuckley commented Feb 4, 2019 •
edited

UPDATE: This is now being tracked in https://github.com/orgs/kubernetes/projects/38

This is to track the changes we still want to make to the [`ServerSideApply`](https://github.com/kubernetes/enhancements/issues/555) alpha feature, now that it's feature branch has merged into master. These were previously tracked in a Working Group Apply sprint planning spreadsheet, but are being moved here for more transparency.

Dry-run and `kubectl diff` features are tracked separately:
- Dry-run: #68514
- `kubectl diff`: #68526

## Beta 2
Topics are mostly sorted by priorities.

### Scalability
- [ ] Reduce further the number of allocations performed when tracking field ownership: @apelisse
  - [x] Improve benchmarks to be closer to real-life and cover more scenarios
  - [x] https://github.com/kubernetes-sigs/structured-merge-diff/pull/129
- [ ] Reduce the serialized size of the managedFields @jennybuckley 
  - [x] Write document/KEP? to describe new format/approach
  - [x] Generate a map of field names to numbers **help wanted**
  - [ ] Implement the new serialization/deserialization format
  - [ ] Replace field names with integers to further reduce size
  - [ ] Test implementation
  - [ ] Document new format
- [ ] Test scalability with 50% of requests tracking managedFIelds
- [ ] Remove managedFIelds from very large objects (close to etcd object size limit)

### Declarative Manager
- [ ] Allow setting "fieldManager" declaratively (add it as a field): KEP (https://github.com/kubernetes/enhancements/pull/923), #75917 @kwiesmueller 

### Topology
- [ ] Understand, test and document how topology changes affect types compatibility (e.g. setting a list from atomic to associative-list, and many other combinations)
- [ ] Document CRD can use list-type and list-map-keys https://github.com/kubernetes-sigs/structured-merge-diff/issues/115 @apelisse
- [ ] API user targeted documentation (what is an asso. list and why) https://github.com/kubernetes/website/issues/16725 @apelisse
- [ ] Update KEP to explain `x-kubernetes-list-type` and other markers @apelisse

### Documentation
- [ ] Write documentation re: PATCH content type changes (document wire format)

### Miscellaneous
- [ ] Return code if Apply didn't change the object: https://github.com/kubernetes/kubernetes/issues/85750
- [ ] Find out what to do for objects that go over the size limit BECAUSE of SSA
- [ ] Add listType validation (set, maps) to CRDs even if SSA is disabled #84724
- [ ] Add `Apply()` to client-go (and potentially other clients) @joshmsamuels 
- [ ] Add apply functions to kubebuilder https://github.com/kubernetes-sigs/controller-runtime/issues/347 @mariantalla 
- [ ] Scale sub-resource doesn't update managedFields for replicas: #82046 @julianvmodesto  @mariantalla 
- [ ] server-side apply takes ownership of fields that don't get set due to status wiping #75564 https://github.com/kubernetes/enhancements/pull/1123 @kwiesmueller @joshmsamuels https://github.com/kubernetes-sigs/structured-merge-diff/pull/133
- [ ] Skip `proto.Schema` and go from v3 to smd schema directly
   - [ ] Surface schema parsing errors from CRD handler to users who don't understand why their CRD isn't being handled by apply like they expect (Related to #82438)
- [ ] Defaulted associative keys are missing @jennybuckley https://github.com/kubernetes-sigs/structured-merge-diff/pull/83
- [ ] Integrate with controller-runtime: https://github.com/kubernetes-sigs/controller-runtime/issues/347
- [ ] Merge annotations available in kube-builder https://github.com/kubernetes-sigs/controller-runtime/issues/638 @apelisse 
- [ ] Decide what to do with mutating admission controllers @kwiesmueller #78469
- [ ] Validation ratcheting: https://github.com/kubernetes/kubernetes/issues/64841#issuecomment-413263824
- [ ] Think about removing feature branch altogether, or archive it somehow
- [ ] Make a set of test cases in an easy-to-consume place (separate package?) so it's easy to test in both integration and e2e. (from [this comment](https://github.com/kubernetes/kubernetes/blob/master/test/integration/apiserver/apply/apply_test.go#L57))

### Unions and Immutability
- [ ] Design and implement API Union types: https://github.com/kubernetes/enhancements/pull/926/, https://github.com/kubernetes-sigs/structured-merge-diff/pull/72 #77370 #77770 
- [ ] Immutable Fields API: @sttts  @jpbetz  https://github.com/kubernetes/enhancements/pull/1099
- [ ] In `kube-openapi/schemaconv`, we could do a better job of detecting bad unions **help wanted**

### Declarative Field Removal
- [ ] Design a way to remove specific fields or values

### Enabling SSA by default on Kubectl
- [ ] KEP Review: https://github.com/kubernetes/enhancements/pull/1382
- [ ] Bug: getting conflict when creating with apply an atomic list @jennybuckley 
- [ ] Handle default values that are optional but  part of a key  (`protocol` in `containerPorts`) 
- [ ] Age-out pathway: how to prevent cruft: design doc
- [ ] Support upgrade from lastApplied annotation to managedFields (outlined in [this doc](https://goo.gl/fg7tUW#heading=h.gf5xewvom24o))
- [ ] How do people start using the feature: coming from client-side apply

## Completed/Canceled
- ~[ ] Consider removing union logic in update pathways (manager identification would need to always be unique)~ 
- [x] Serialization of fieldsets in managedFields should always be the same #82042
- [x] Move FieldManager test where it'll be less flaky: https://github.com/kubernetes/kubernetes/pull/78738/files#r317326938 @tedyu #82135
- [x] Fix int or float bug when deserializing to Object @kwiesmueller
- [x] Update API docs because they mention "Alpha" everywhere @apelisse #82548
- [x] Fail if apply specifies `ManagedFields` and/or document if it does, and write a test @joshmsamuels #81453
- [x] Improve field manager semantics documentation: what does owning mean, for lists, across versions, etc... https://github.com/kubernetes/website/pull/15948
- [x] Use the schema provided by a CRD if it exists (from [this comment](https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/typeconverter.go#L46)) @jennybuckley #77354
  - [x] Add a test for applying CRDs
- [x] Make sure we don't write to etcd when applying has no changes: verified locally, but needs test #81673
- [x] Verify and maybe document? how to wipe out managedFields: should we have a sub-resources to change managedFields? @kwiesmueller https://github.com/kubernetes/website/pull/15700/
- [x] Scalability issue with server-side apply enabled: #76219 https://github.com/kubernetes/enhancements/pull/1110 @jennybuckley @apelisse @lavalamp  
  - [ ] ~Hide `ManagedFields` by default: @apelisse https://github.com/kubernetes/enhancements/pull/1081/~ @apelisse We've decided not to do that.
  - [x] Write benchmarks for each step and for the full-flow
  - [x] Write issue with scalability findings and possible solutions
  - [x] Make API change
  - [x] Would it make sense to use "RawMessage", does it have a protobuf equivalent?
  - [x] Consider etcd storage limits (objects close to maximum)
- [ ] ~RBAC for dry-run (ability to dry-run without write permission): https://github.com/kubernetes/enhancements/pull/893~ @apelisse 
- [x] Kubectl server-side apply shouldn't fallback on client-side apply if not available. TODO: check if this is done. @joshmsamuels 
- [x] Not fail on update failures (and monitor the errors) @apelisse #77987
- [x] Handle conversions for managedFields for versions that are no longer served (https://github.com/kubernetes/kubernetes/issues/77519)
- [x] Add cross group/version test: https://github.com/kubernetes/kubernetes/pull/75302#issuecomment-472227136: @julianvmodesto https://github.com/kubernetes/kubernetes/pull/76468
- [x] Define openapi future strategy (https://github.com/kubernetes/kube-openapi/pull/147#issuecomment-466549008): @jennybuckley 
- [x] Handle RawExtension / CRDs more consistently, as non-atomic Untyped (https://github.com/kubernetes-sigs/structured-merge-diff/pull/71)
- [x] Merge feature-serverside-apply branch into master (#72947)
- [x] Test-grid stayed green after the PR was merged.
- [x] Make sure no tests are broken when the feature is enabled (#73763)
- [x] Fix apply for RawExtensions by updating kube-openapi vendor (https://github.com/kubernetes/kubernetes/pull/71223)
- [x] Handle removed API type versions correctly in managedFields (https://github.com/kubernetes-sigs/structured-merge-diff/pull/63)
- [x] Strip selected fields from managedFields (#73681)
- [x] Revert #73763 when https://github.com/kubernetes/kube-openapi/pull/133 is vendored in (#73974)
- [x] Kubectl doesn't print nice conflict errors because of json manager name (#73976)
- [x] Applying with server-side apply fails for Service (#73759, #73831)
- [x] Make sure forceAllowCreate works for all the custom storage implementations (#65925, might be the cause of #73759)
- [X] Update reviewers/approvers for the feature (#73865)
- [x] Changing container port doesn't remove dangling item (https://github.com/kubernetes-sigs/structured-merge-diff/pull/65)
- [x] Review existing test suite and improve it, add more tests! (https://github.com/kubernetes-sigs/structured-merge-diff/pull/64, https://github.com/kubernetes-sigs/structured-merge-diff/pull/69, https://github.com/kubernetes-sigs/structured-merge-diff/pull/68)
- [x] Strip additional fields from `FieldManager` (#74206)
- [x] Update vendor to pull in changes to smd (#74981)
- [x] Sort `ManagedFields` by `(operation_type, timestamp, manager)` (#74721, #75075)
- [x] Add monitoring for various endpoints/verbs/content-types (#74997)
- [x] Figure out where workflow IDs will come from (outlined in [this doc](https://docs.google.com/document/d/1hNYwQAKDig-ljS8jvV8WtVBssWfG_4WHrsLJgZg_wgQ/edit)) (~#74171~, #74760)
- [x] Fix versionconverter bug (#75151)
- [x] Explicitly check that patchObj is in the proper version (from [this comment](https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go#L155)) (#75135, #75157)
- [x] Preserve int/float distinction by not going through unstructured on patch bytes (#75277)
- [x] Release note and Documentation for new feature (https://github.com/kubernetes/website/pull/13077)

cc @apelisse @lavalamp @kwiesmueller 
/sig api-machinery
@jennybuckley jennybuckley added the kind/bug Categorizes issue or PR as related to a bug. label Feb 4, 2019
@k8s-ci-robot k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed kind/bug Categorizes issue or PR as related to a bug. labels Feb 4, 2019
@k8s-ci-robot
Copy link
Contributor

@jennybuckley: Those labels are not set on the issue: kind/bug

In response to this:

/remove-kind bug

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.

@spiffxp
Copy link
Member

spiffxp commented Feb 5, 2019

/milestone v1.14
since there is work happening to land this for v1.14 (though not necessarily close out entirely)

@k8s-ci-robot k8s-ci-robot added this to the v1.14 milestone Feb 5, 2019
@jennybuckley jennybuckley mentioned this issue Feb 5, 2019
Closed
@jennybuckley jennybuckley mentioned this issue Feb 12, 2019
Merged
@jennybuckley
Copy link
Author

/wg apply

@nikopen
Copy link
Contributor

nikopen commented Feb 27, 2019

/kind design

@k8s-ci-robot k8s-ci-robot added the kind/design Categorizes issue or PR as related to design. label Feb 27, 2019
This was referenced Mar 5, 2019
Merged
Merged
@spiffxp
Copy link
Member

spiffxp commented Mar 9, 2019

/assign @jennybuckley
v1.14 release lead here... a number of the "alpha-blocking" checkboxes remain unchecked, it looks like we're missing some docs and tests, can we have an update on this?

@kwiesmueller
Copy link
Member

I'll look into the docs as much as I can @jennybuckley but haven't worked on website yet so might need help/guidance

@apelisse
Copy link
Member

apelisse commented Mar 9, 2019

@spiffxp We need to work on the documentation, but at this point we're quite happy with what we have. We could rename "Alpha blocking" into "Alpha nice-to-have" :-)

@spiffxp
Copy link
Member

spiffxp commented Mar 18, 2019

/priority important-longterm
remaining work not destined to land in this release

@k8s-ci-robot k8s-ci-robot added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Mar 18, 2019
@nikopen
Copy link
Contributor

nikopen commented Mar 20, 2019

Alpha / 1.14 seems to be complete^^

moving on....

/milestone v1.15

@k8s-ci-robot k8s-ci-robot modified the milestones: v1.14, v1.15 Mar 20, 2019
@DirectXMan12 DirectXMan12 mentioned this issue Mar 21, 2019
Closed
@julianvmodesto julianvmodesto mentioned this issue Apr 11, 2019
Merged
@apelisse
Copy link
Member

apelisse commented Jun 7, 2019

Yeah, thanks, clearly not meant for 1.15 at this point.

@joshmsamuels joshmsamuels mentioned this issue Jun 11, 2019
Merged
@kwiesmueller kwiesmueller mentioned this issue Jun 11, 2019
Closed
@josiahbjorgaard
Copy link
Contributor

Hi all, code freeze is coming up in just a few days (Thursday, end of day, PST), so we need to make sure that this issue will be solved for v1.16 or moved to v1.17. Can you comment on it's status?

@apelisse
Copy link
Member

We've been more busy working on the few PRs that we have pending than updating this issue, so I wouldn't really rely on that. Sorry about that. I'll try to update it ASAP.

I've left a status there this morning: kubernetes/enhancements#555

@yliaog yliaog mentioned this issue Sep 3, 2019
Closed
@josiahbjorgaard
Copy link
Contributor

@apelisse Can this issue be moved out of the current milestone?

@apelisse
Copy link
Member

apelisse commented Sep 5, 2019

The feature is going beta in 1.16, but I agree that we're not going to close it anytime soon.

@liggitt
Copy link
Member

liggitt commented Sep 12, 2019

/milestone v1.17

1.16-targeted PRs and picks are merged

@k8s-ci-robot k8s-ci-robot modified the milestones: v1.16, v1.17 Sep 12, 2019
@alenkacz
Copy link
Contributor

@apelisse I've spent a lot of time reading the server-side apply design and even some parts of the implementation, when I came to this issue. I am just wondering, are there some parts of the tasks still unresolved here good for contributions? If so, would you have any recommendation?

@enxebre enxebre mentioned this issue Oct 19, 2019
Merged
@julianvmodesto julianvmodesto mentioned this issue Oct 20, 2019
Closed
@apelisse
Copy link
Member

@alenkacz A good place to start, which is not super technical but still a good place to start is: kubernetes-sigs/structured-merge-diff#115

@josiahbjorgaard
Copy link
Contributor

Hi, bug triage for 1.17. code freeze is Nov. 14th this time around, is this issue intended to close by then?

@apelisse
Copy link
Member

Nope, this is on-going work, we don't intend to close this any time soon :-)

@josiahbjorgaard
Copy link
Contributor

@apelisse Can we move this to the v1.18 milestone as code freeze is in a couple of days?

@apelisse
Copy link
Member

Of course

@josiahbjorgaard
Copy link
Contributor

/milestone v1.18

@k8s-ci-robot k8s-ci-robot modified the milestones: v1.17, v1.18 Nov 13, 2019
@julianvmodesto julianvmodesto mentioned this issue Nov 26, 2019
Merged
@kwiesmueller kwiesmueller mentioned this issue Dec 11, 2019
Closed
@kwiesmueller
Copy link
Member

The work continues: https://github.com/orgs/kubernetes/projects/38
If it has to stay open for the milestone, we should indicate that work moved to the project in the description. Can't do that.

Otherwise, i would close it.

@gianarb
Copy link

gianarb commented Feb 12, 2020

Hello @apelisse !
Bug Triage team here for the 1.18 release. This is a friendly reminder that code freeze is scheduled for 5 March. Is this issue still intended for milestone 1.18, or should it extend to the next one? Thanks in advance!

@julianvmodesto julianvmodesto mentioned this issue Feb 12, 2020
Closed
@gianarb
Copy link

gianarb commented Feb 19, 2020

Hello @apelisse @jennybuckley !
Bug Triage team here for the 1.18 release. This is a friendly reminder that code freeze is scheduled for 5 March. Is this issue still intended for milestone 1.18, or should it extend to the next one? Thanks in advance!

@jennybuckley
Copy link
Author

Closing, not because this is done, but because this is now being tracked in https://github.com/orgs/kubernetes/projects/38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jennybuckley jennybuckley

Labels
kind/design Categorizes issue or PR as related to design. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Milestone
v1.18
Development

No branches or pull requests