Skip to content

Conversation

@jaehanbyun
Copy link
Contributor

@jaehanbyun jaehanbyun commented Oct 28, 2025

Summary

This PR fixes 409 Conflict errors in the YAML editor.
The editor now sends only the changed fields instead of the entire resource, significantly reducing conflicts on frequently updated resources like Nodes.

Related Issue

Fixes #4097

Changes

Before

The editor performed a full PUT request with the entire resource object:

async function updateFunc(newItem: KubeObjectInterface) {
  // Simply sent the entire edited resource
  await item.update(newItem);  // Uses PUT request
}

Problem: If the resource's resourceVersion changed between opening the editor and saving (e.g., kubelet updating Node status), Kubernetes rejected the request with 409 Conflict.

After

The editor now captures the original snapshot and sends only the diff:

async function updateFunc(newItem: KubeObjectInterface) {
  // Calculate JSON Patch: diff between original and edited
  const patches = compare(originalResourceRef.current, newItem);
  
  // Send PATCH with json-patch format
  await patch(url, patches, true, {
    cluster: item._clusterName,
    headers: { 'Content-Type': 'application/json-patch+json' }
  });
}

Benefit: Only changed fields are sent, dramatically reducing conflict probability even when the resource is modified by other processes.

Steps to Test

  1. Navigate to a Node page
  2. Click the Edit button to open the YAML editor
  3. Make a small change (e.g., add a label or modify a taint)
  4. Wait a few seconds (kubelet will update the Node status in the background)
  5. Click "Save & Apply"
  6. Verify the change is applied successfully without a 409 Conflict error

Screenshots (if applicable)

Before

image

After

image

Notes for the Reviewer

  • [e.g., This touches the i18n layer, so please check language consistency.]

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 28, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jaehanbyun
Once this PR has been reviewed and has the lgtm label, please assign illume for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

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 k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 28, 2025
…licts

Signed-off-by: jaehanbyun <awbrg789@naver.com>
@jaehanbyun jaehanbyun force-pushed the fix/yaml-editor-conflict branch from 0fb7696 to f6923f8 Compare October 28, 2025 15:53
Signed-off-by: jaehanbyun <awbrg789@naver.com>
@illume illume added kind/feature Categorizes issue or PR as related to a new feature. frontend Issues related to the frontend labels Oct 29, 2025
@illume illume added this to the v0.38.0 milestone Oct 29, 2025
@jaehanbyun jaehanbyun changed the title frontend: EditButton: Use JSON patch to prevent resource version conficts frontend: EditButton: Use JSON patch to prevent resource version conflicts Oct 30, 2025
@illume
Copy link
Contributor

illume commented Oct 30, 2025

This is a really nice fix.

I need some time to test it though. So I marked it for the 0.38.0 release.

My initial question is what happens if someone else updates it in the mean time? Do you handle this case?

@jaehanbyun
Copy link
Contributor Author

jaehanbyun commented Oct 31, 2025

@illume
Thanks for the review!

I tested concurrent edits to answer your question:

  • Different fields (e.g., different label keys): both updates apply successfully.

  • Same field (same label key, different values): the later request wins. The second replace on the same path overwrites the first one (effectively last-write-wins).

  • Delete vs modify on the same key: also last-write-wins.

    • If A removes labels.key1 and B changes labels.key1 from value1 → value2, the one that reaches the API server later determines the outcome:
      • A later → key1 is deleted.
      • B later → key1: value2 is present.

@illume
Copy link
Contributor

illume commented Nov 3, 2025

I wonder if last-write-wins is the way in these cases? I think it could be a bit dangerous.

Currently it doesn't let them apply, so they have to try again. It's not a good experience, but at least they need to try make the change again and take into consideration the changes.

The alternative could be:

    1. let Different fields (e.g., different label keys): show the user there are different fields.
    1. Same field (same label key, different values): show the user the conflicted field and values, with last one shown to apply.
    1. Delete vs modify on the same key: show the user the conflicted field and values, with last one shown to apply.

Basically show them the conflict with a default of the last write... but make them confirm the change... and let them have an opportunity to change it (maybe keeping the other change).

What do you think?

@jaehanbyun
Copy link
Contributor Author

jaehanbyun commented Nov 4, 2025

@illume Yeah, handling the three cases as you described would definitely provide a better UX.
So, before implementing it, I think we should follow an approach(3 ver diff) like this:

  1. Prepare the base (when the editor opens), theirs (latest from server at "Save & Apply"), and ours (user edits).

  2. Compare base→theirs and base→ours to detect only “path-level” conflicts.

  3. If no conflicts, but there are differences between base and theirs, show a modal highlighting those changes.

  4. If conflicts exist, show a modal listing the conflicted fields with ours/theirs previews.
    Default to ours (last edit) but require confirmation.
    Then merge based on selections and apply a JSON Patch against the latest version.

What do you think about proceeding with this approach before we start implementing it?

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

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. frontend Issues related to the frontend kind/feature Categorizes issue or PR as related to a new feature. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

YAML editor produces 409 Conflict errors on frequently updated resources (e.g., Nodes)

3 participants