Skip to content
This repository has been archived by the owner on Nov 17, 2021. It is now read-only.

diff does not report when keys have been removed from the spec #228

Open
dmlemos opened this issue Oct 5, 2018 · 6 comments
Open

diff does not report when keys have been removed from the spec #228

dmlemos opened this issue Oct 5, 2018 · 6 comments
Milestone

Comments

@dmlemos
Copy link

dmlemos commented Oct 5, 2018

When removing a key from the spec, the diff doesn't report that its changed.

Lets assume I have this configuration applied to a cluster in a file called resourcequota.json:

{
   "apiVersion": "v1",
   "kind": "ResourceQuota",
   "metadata": {
      "name": "test",
   },
   "spec": {
      "hard": {
         "limits.cpu": "8"
      }
   }
}

Adding a new key with the following spec:

{
...
   "spec": {
      "hard": {
         "limits.cpu": "8",
         "requests.cpu": "100m"
      }
   }
}
$ kubecfg diff --diff-strategy subset resourcequota.json
---
- live resourcequotas test
+ config resourcequotas test
 {
...
   "spec": {
     "hard": {
       "limits.cpu": "8"
+      "requests.cpu": "100m"
     }
   }
 }
$ kubecfg udpate resourcequota.json

Now after removing the last key, the diff thinks there is no difference:

$ kubecfg diff --diff-strategy subset resourcequota.json
---
- live resourcequotas test
+ config resourcequotas test
resourcequotas test unchanged

The expected behaviour would be to show that the key has been removed.

@gotwarlost
Copy link

I believe this is because of diff-strategy=subset which ignores extra fields in the original YAML. While this is useful for not showing spurious diffs such as server generated metadata, it does mean that valid removals will not be shown.

@mkmik
Copy link
Contributor

mkmik commented Aug 2, 2019

FWIW, update now does strategic merge patch (using an annotation, like kubectl apply does).
I guess we should make kubecfg diff should do the same trick.

@mkmik mkmik added this to the v0.13.0 milestone Aug 2, 2019
@shric
Copy link
Contributor

shric commented Feb 14, 2020

FWIW, update now does strategic merge patch (using an annotation, like kubectl apply does).
I guess we should make kubecfg diff should do the same trick.

I think there should still be two diff modes

  • the existing mode which ignores the last-applied-configuration annotation so that it's really comparing with the running config.
  • the new mode behaving like update, which uses the annotation.

I am not sure how this would work in the UI -- --diff-strategy=update?

@mkmik I'd like to attempt a PR on this if you're not currently working on it or plan to do so soon. I also think it would be great if this issue is fixed before #282 (comment) is implemented.

@mkmik
Copy link
Contributor

mkmik commented Feb 17, 2020

@shric yes, that would be great, thanks!

@gaurav517
Copy link

PR for the discussed approach: #303 Thanks.

Sorry @shric, we wanted it sooner.

@shric
Copy link
Contributor

shric commented May 8, 2021

No problem, thanks @gaurav517 ! Apologies.for not providing this as stated above, I changed employers in July 2020 and sadly I no longer get to use kubecfg :(

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants