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 diff fails to detect differences in Service objects #1601

Open
jrcast opened this issue May 24, 2024 · 11 comments
Open

kubectl diff fails to detect differences in Service objects #1601

jrcast opened this issue May 24, 2024 · 11 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. priority/backlog Higher priority than priority/awaiting-more-evidence. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@jrcast
Copy link

jrcast commented May 24, 2024

What happened:
kubectl diff is not detecting diffs between a service's manifest file and the modified service object in Kubernetes. I tried both client-side or server-side mode without success.

What you expected to happen:
kubectl diff identifies the diff

How to reproduce:

  1. Create a K8s service using a manifest kubectl create -f my-svc.yaml. Where my-svc.yaml is:
    apiVersion: v1
    kind: Service
    metadata:
      name: example
      namespace: default
    spec:
      ports:
        - name: service-port
          port: 80
          protocol: TCP
          targetPort: service-port
      selector:
        app.kubernetes.io/instance: example
      type: ClusterIP
  2. Using kubectl edit, modify the K8s service. i.e. Add an extra port.
    apiVersion: v1
    kind: Service
    metadata:
      name: example
      namespace: default
    spec:
      ports:
        - name: service-port
          port: 80
          protocol: TCP
          targetPort: service-port
        - name: shouldntbehere    # <<<< THIS 
          port: 8080
          protocol: TCP
          targetPort: service-port
      selector:
        app.kubernetes.io/instance: example
      type: ClusterIP
  3. Run:
    kubectl diff -f my-svc.yaml
    # or
    kubectl diff -f my-svc.yaml --server-side --force-conflicts
  4. No diff shows up.

Anything else we need to know?:

Environment:

  • Kubernetes client and server versions (use kubectl version): 1.27 on both server/client. Also tried 1.30 with same results.
  • Cloud provider or hardware configuration: AWS EKS
  • OS (e.g: cat /etc/os-release): Ubuntu 22.04.4 LTS
@jrcast jrcast added the kind/bug Categorizes issue or PR as related to a bug. label May 24, 2024
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label May 24, 2024
@ardaguclu
Copy link
Member

Have you verified that the kubectl edit actually updated your service?

@kundan2707
Copy link

@jrcast have you checked if kubectl command executed successfully without error ?

@kundan2707
Copy link

/remove-kind bug

@k8s-ci-robot k8s-ci-robot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. and removed kind/bug Categorizes issue or PR as related to a bug. labels May 27, 2024
@kundan2707
Copy link

/kind support

@k8s-ci-robot k8s-ci-robot added the kind/support Categorizes issue or PR as a support question. label May 27, 2024
@brianpursley
Copy link
Member

Can confirm this happens when you make changes using kubectl edit, and it does seem to have something to do with server side apply.

If you use the --save-config flag with edit, then kubectl diff will work:

kubectl edit service example --save-config
$ kubectl diff -f my-svc.yaml 
diff -u -N /tmp/LIVE-3530718821/v1.Service.default.example /tmp/MERGED-1768176721/v1.Service.default.example
--- /tmp/LIVE-3530718821/v1.Service.default.example	2024-05-28 11:44:49.052930430 -0400
+++ /tmp/MERGED-1768176721/v1.Service.default.example	2024-05-28 11:44:49.056930322 -0400
@@ -22,10 +22,6 @@
     port: 80
     protocol: TCP
     targetPort: service-port
-  - name: shouldntbehere
-    port: 8080
-    protocol: TCP
-    targetPort: service-port
   selector:
     app.kubernetes.io/instance: example
   sessionAffinity: None

But if you use that flag, it's going to add the kubectl.kubernetes.io/last-applied-configuration annotation, which shouldn't be a requirement for diff.

/remove-kind support
/kind bug

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. and removed kind/support Categorizes issue or PR as a support question. labels May 28, 2024
@ardaguclu
Copy link
Member

/triage accepted
/priority backlog

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/backlog Higher priority than priority/awaiting-more-evidence. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 5, 2024
@vikasrajputin
Copy link

Hello @ardaguclu - Can i pick this up?

@mabulgu
Copy link

mabulgu commented Oct 18, 2024

Hi @vikasrajputin, are you still working on this issue? If not I would love to work on this. cc @ardaguclu

@mabulgu
Copy link

mabulgu commented Oct 21, 2024

Since no answers and upon discussion with @ardaguclu taking this on me

/assign

@mabulgu
Copy link

mabulgu commented Nov 3, 2024

This not only happens with an edit but also:

1- Have two different service YAMLs. One is for the initial state (service.yaml), the other one (service-with-additional-port.yaml) is for the updated state (with the additional port etc.)
2- Run kubectl create -f service.yaml and create the service.
3- Replace the service with the service-with-additional-port.yaml by using the replace command.
4- Run the diff against the service.yaml and the resource on K8s (with and without the flags --server-side --force-conflicts) and see no diff.

Adding an integration test for this scenario first & investigating in parallel. JFYI

@mabulgu
Copy link

mabulgu commented Nov 4, 2024

Let me explain how the diff works roughly and the problem behind the above issue is.

When you do kubectl diff -f myresource.yaml, it basically does the comparison with the LIVE resource, which is the resource on the serverside, and the MERGED resource, which is the merged version of the LIVE and myresource.yaml.

For the LIVE resource, since it is the bare resource, it is OK. However, the issue gets complicated when we generate a MERGED resource with the implementation here.

As you can see in the Merged() method, wheter you use the --server-side flag or not, there happens a patch. In the --server-side case, it is simpler than the non --server-side one because all we do is a apply patch there..

The modified object, which is the resource in the YAML file, is returned as a byte array here and used in the patching action here

The main problem is, when you do a patch in both ways, because the YAML file has less resources than our LIVE resource, the patching (which is actually a merging) ends up with a merged resource, which is exactly the same with the LIVE resource. So in the end, no diff shows up.

So if we want to get a diff between a resource YAML that has less items (e.i removed items from an array or just a field etc.), we should either get rid of the MERGE mentality and use the obj.LocalObj ** but the side affects should be discussed. I wonder why we started with a diff with a merged resource from the beginning TBH.

@ardaguclu any ideas here? or if you can tag someone else (like the original author) who might some insights, that will be appreciated.

If I am not clear enough I can rephrase, re-explain happily either here or in one of the comm. meetings if needed.

Update:

[**] When we use the local resource directly instead of the merged one we can get a diff as follows:

(.venv) ➜  testdata git:(fix/kubectl-diff) ✗ kubectl diff -f service.yaml
diff -u -N /var/folders/0n/m2mcfrmj6h132v21j0jy26jm0000gn/T/LIVE-1825494912/v1.Service.default.example-service /var/folders/0n/m2mcfrmj6h132v21j0jy26jm0000gn/T/MERGED-1861289674/v1.Service.default.example-service
--- /var/folders/0n/m2mcfrmj6h132v21j0jy26jm0000gn/T/LIVE-1825494912/v1.Service.default.example-service 2024-11-04 01:11:36
+++ /var/folders/0n/m2mcfrmj6h132v21j0jy26jm0000gn/T/MERGED-1861289674/v1.Service.default.example-service       2024-11-04 01:11:36
@@ -1,31 +1,14 @@
 apiVersion: v1
 kind: Service
 metadata:
-  creationTimestamp: "2024-11-03T19:08:41Z"
   name: example-service
   namespace: default
-  resourceVersion: "118332"
-  uid: 40d19a01-025a-479c-815a-88bf0ba5b904
 spec:
-  clusterIP: 10.96.2.90
-  clusterIPs:
-  - 10.96.2.90
-  internalTrafficPolicy: Cluster
-  ipFamilies:
-  - IPv4
-  ipFamilyPolicy: SingleStack
   ports:
   - name: service-port
     port: 80
     protocol: TCP
     targetPort: service-port
-  - name: additional-service-port
-    port: 8080
-    protocol: TCP
-    targetPort: service-port
   selector:
     app.kubernetes.io/instance: example
-  sessionAffinity: None
   type: ClusterIP
-status:
-  loadBalancer: {}

In this case we get K8s generated resources as well not only the port diff. So main idea behind the merge maybe simply avoiding this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. priority/backlog Higher priority than priority/awaiting-more-evidence. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

7 participants