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.

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

kubectl docs: explanation json patch, strategic merge $patch directives #47624

Closed
wants to merge 1 commit into from

Conversation

ringerc
Copy link
Contributor

@ringerc ringerc commented Aug 22, 2024

Description

When learning k8s I found myself confused by the difference between json and merge patch types, usage of $patch strategic merge directives.

It's very confusing for new (and not so new) users that merge is the name for the RFC7386 JSON merge patch ("dumb" structural merge), json is the name for the RFC6902 json patch patch (array of operations), and strategic is the name for the strategic merge patch. And all of them can be in yaml...

Then there's server-side apply, which may act like a patch rather than replace in the presence of multiple resource managers.

Draft to expand the kubectl patch (and soon, apply) guidance a bit to:

  • Link to the specification of strategic merge patches (obsoleted by server-side apply?)
  • mention $patch directives for overriding the merge strategy in strategic merge patches
  • More clearly explain the difference between a json (json patch) and merge (json merge) patch
  • add an example of a json patch, including selection of a container in an array and a test for the container name
  • add an example of using a json patch to append to volumes and volumemounts

This is draft, because I'm still not 100% sure myself of the rules surrounding server-side vs client-side patching and strategic merge patches so I have some more tests to write before proposing for merge. The current documentation is very confusing - server-side-apply is actually used for both patch and apply --server-side operations, but patch can accept json / merge / strategic formats, etc. I will seek to further refine my changes until they clearly explain how exactly k8s applies changes in different modes, and update further.

TODO

Issue

Copy link

linux-foundation-easycla bot commented Aug 22, 2024

CLA Not Signed

@ringerc ringerc marked this pull request as draft August 22, 2024 01:27
Copy link

netlify bot commented Aug 22, 2024

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit f4670a5
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/66c6983a41c7240008b1e822
😎 Deploy Preview https://deploy-preview-47624--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

* Link to the specification of strategic merge patches
* More clealy explain the difference between a json (json patch) and merge (json merge) patch
* add an example of a json patch, including selection of a container in an array and a test for the container name
* mention using json patch to append to args, volumes, volumeMounts etc
* mention $patch strategic merge directives
@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 22, 2024
@k8s-triage-robot
Copy link

Unknown CLA label state. Rechecking for CLA labels.

Send feedback to sig-contributor-experience at kubernetes/community.

/check-cla
/easycla

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Aug 22, 2024
@sftim
Copy link
Contributor

sftim commented Aug 22, 2024

You'll need to sign the CLA before we can review your changes, but you might like to check out https://kubernetes.io/docs/reference/using-api/api-concepts/#patch-and-apply and see if you want to also update that page, or maybe to direct readers to look there.

@ringerc
Copy link
Contributor Author

ringerc commented Sep 5, 2024

@sftim Thanks very much. The more I got into this the worse it got - I don't think I can do justice to untangling all the confusion and outdated or incomplete info in the relevant docs regarding the patch types, server-side apply, etc.

I can't clearly figure out for example if strategic merge patch directives are still supported with --server-side apply, and I don't expect to have time to do sufficiently in-depth study and test writing for it. I'm happy to do some further docs work if I can get some guidance understanding the problem space better, but I realised when I started looking into server-side apply that it's way more different than I thought.

I'll look at the ref you provided, and see if I can tidy this docs change up to be a minimal update that doesn't go too deep into the weeds.

I'm currently stuck waiting for my org to eventually appoint a CLA manager so I can sign the CLA, which is taking forever.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all 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:

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

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

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 4, 2024
@ringerc ringerc closed this Dec 16, 2024
@ringerc
Copy link
Contributor Author

ringerc commented Dec 16, 2024

abandoning

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants