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

1.18 Server-side apply #19286

Merged

k8s-ci-robot merged 3 commits into kubernetes:dev-1.18 from apelisse:ssa-1.18

Mar 18, 2020

Member

apelisse commented Feb 24, 2020 •

edited

This is not empty now.

k8s-ci-robot added the cncf-cla: yes label

Collaborator

k8sio-netlify-preview-bot commented Feb 24, 2020 •

edited

Deploy preview for kubernetes-io-vnext-staging processing.

Building with commit 0efc46e

https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/5e6beef2a4626b000a57c0d8

k8s-ci-robot added size/XS approved labels

apelisse mentioned this pull request

Server-side Apply kubernetes/enhancements#555

Closed

Contributor

VineethReddy02 commented Mar 7, 2020

👋 Please rebase this PR on dev-1.18 in order to avoid merge conflicts.

Feel free to /hold cancel after you've rebased.

/hold

k8s-ci-robot added the do-not-merge/hold label

Contributor

VineethReddy02 commented Mar 9, 2020

Hi, @apelisse today is the deadline for docs placeholder PR to be ready for review state. I see the status of the doc as still TODO Can I know by when can I expect all the docs to be ready for review state?

Member Author

apelisse commented Mar 9, 2020

cc @kwiesmueller

apelisse closed this

apelisse force-pushed the ssa-1.18 branch from 47532ab to 899ab7b Compare

March 9, 2020 16:05

apelisse reopened this

k8s-ci-robot added size/S and removed size/XS labels

apelisse changed the title ~~Placehold for 1.18 Server-side apply~~ 1.18 Server-side apply

Member Author

apelisse commented Mar 11, 2020

cc @VineethReddy02

k8s-ci-robot added language/en sig/docs and removed approved labels

Member

kwiesmueller commented Mar 13, 2020

/lgtm
/approve

k8s-ci-robot assigned kwiesmueller

k8s-ci-robot added the lgtm label

Contributor

VineethReddy02 commented Mar 13, 2020

@sftim do you have any comments on these docs? I see a technical lgtm
If you are okay, I can go ahead with my approval.

VineethReddy02 requested review from jimangel and sftim

March 13, 2020 16:06

sftim reviewed

View reviewed changes

Contributor

sftim left a comment

Some feedback

content/en/docs/reference/using-api/api-concepts.md Outdated Show resolved Hide resolved

content/en/docs/reference/using-api/api-concepts.md Outdated

               which modify the object). Both operations update the `managedFields`, but behave
               a little differently.
+              {{< note >}}The apply content type `application/apply-patch+yaml` accepts both yaml **and** json,

Contributor

sftim Mar 13, 2020

nit:

Suggested change

      
            {{< note >}}The apply content type `application/apply-patch+yaml` accepts both yaml **and** json,
          
            {{< note >}}The apply content type `application/apply-patch+yaml` accepts both YAML **and** JSON,

or, better, reword:

{{< note >}}
Whether you are submitting JSON data or YAML data, use `application/apply-patch+yaml` as the
Content-Type header value.

All JSON documents are valid YAML.
{{< /note >}}

content/en/docs/reference/using-api/api-concepts.md Outdated Show resolved Hide resolved

content/en/docs/reference/using-api/api-concepts.md Outdated

Comment on lines 669 to 680

+              ### Known Issues
+              - There is a known issue with using Server Side Apply on sub-resource endpoints
+                where field ownership is not being updated [#88981](https://github.com/kubernetes/kubernetes/issues/88981).

Contributor

sftim Mar 13, 2020

Suggested change

      
            ### Known Issues
          
            - There is a known issue with using Server Side Apply on sub-resource endpoints
          
              where field ownership is not being updated [#88981](https://github.com/kubernetes/kubernetes/issues/88981). 
          
            {{< caution >}}
          
            Server Side Apply does not correctly track ownership on sub-resources. If you are using
          
            Server Side Apply with a subresource, …
          
            {{< /caution >}}

what should the … say?

Contributor

sftim Mar 13, 2020

(written to avoid statements about the future)

content/en/docs/reference/using-api/api-concepts.md Outdated

    
              Apply can send partially specified objects to this endpoint. An applied config

              should always include every field that the applier has an opinion about.

              Apply can send partially specified objects as yaml or json to this endpoint.

              An applied config should always include every field that the applier has an opinion about.

Contributor

sftim Mar 13, 2020

Can you reword? A reader might think that the applier is the API server; as a piece of code, API servers don't have opinions.

k8s-ci-robot removed the lgtm label

Kevin Wiesmueller and others added 3 commits

March 13, 2020 13:35


          Clarify content-type and tracking of all objects

bc47f0b


          Update content/en/docs/reference/using-api/api-concepts.md

Co-Authored-By: Tim Bannister <tim@scalefactory.com>


          SSA: Improve a couple of sentences

0efc46e

apelisse force-pushed the ssa-1.18 branch from 528b6da to 0efc46e Compare

March 13, 2020 20:37

Member Author

apelisse commented Mar 13, 2020

@sftim Tentatively improved based on your feedback, possibly made it worse, let me know ;-) Thanks!

Contributor

VineethReddy02 commented Mar 17, 2020

@sftim Can you have a final look here?

Contributor

sftim commented Mar 17, 2020

If you rebased @apelisse then you should /hold cancel
See #19286 (comment)

Member Author

apelisse commented Mar 17, 2020

/hold cancel

k8s-ci-robot removed the do-not-merge/hold label

sftim reviewed

View reviewed changes

Contributor

sftim left a comment

These changes
/lgtm

(and they seem simple enough not to need detailed tech review etc)

k8s-ci-robot assigned sftim

k8s-ci-robot added the lgtm label

Member Author

apelisse commented Mar 17, 2020

Who's supposed to give the approval?

Contributor

sftim commented Mar 17, 2020

SIG Docs release team can approve. @VineethReddy02 are you able to take a look?

Contributor

VineethReddy02 commented Mar 18, 2020

/lgtm
/approve

k8s-ci-robot assigned VineethReddy02

Contributor

k8s-ci-robot commented Mar 18, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kwiesmueller, VineethReddy02

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:

~~content/en/OWNERS~~ [VineethReddy02]

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 the approved label

k8s-ci-robot merged commit ed434a8 into kubernetes:dev-1.18

kwiesmueller mentioned this pull request

Server Side Apply patch type only mentions yaml kubernetes/kubernetes#88975

Closed

2 tasks

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

Labels

approved cncf-cla: yes language/en lgtm sig/docs size/S