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

WIP: Use KUBECTL_EXTERNAL_DIFF for static diffs #558

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

curusarn
Copy link
Contributor

@curusarn curusarn commented Apr 28, 2021

Kubectl diff uses KUBECTL_EXTERNAL_COMMAND for diffing when it is set. Tanka uses kubectl diff when diffing existing k8s resources. But it uses standard diff for newly created or destroyed resources.

For consistency, we want Tanka to respect the KUBECTL_EXTERNAL_COMMAND env variable when doing static diffs.

Thread in Grafana Labs Slack: https://grafana.slack.com/archives/CPPEN0KMH/p1619600657038100?thread_ts=1618844404.031100&cid=CPPEN0KMH


Handling of KUBECTL_EXTERNAL_DIFF was picked up from kubectl itself to achieve the exactly same behavior.
Both Tanka and Kubernetes is released as Apache 2.0 so this should be okay. But IANAL.


I wanted to use kubectl-neat-diff to test this but it can't accept files as arguments - it expects directories.
I'm not sure if it's a good idea to work around kubectl-neat-diff by preparing directories for it in Tanka.

@stale
Copy link

stale bot commented Jun 2, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 2, 2021
@curusarn
Copy link
Contributor Author

curusarn commented Jun 2, 2021

I will get to this eventually. Or more precisely, this won't be WIP anymore once I (or someone else) fixes kubectl-neat-diff.

@stale stale bot removed the stale label Jun 2, 2021
@stale
Copy link

stale bot commented Jul 8, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 8, 2021
@curusarn
Copy link
Contributor Author

curusarn commented Jul 8, 2021

I will get to this eventually.

@stale stale bot removed the stale label Jul 8, 2021
@dohnto
Copy link

dohnto commented Nov 12, 2021

@curusarn The Slack thread is lost, what happened in that thread? Can you please provide short summary?

@curusarn
Copy link
Contributor Author

curusarn commented Nov 12, 2021

@dohnto I'm a little foggy on it.

IIRC it was a discussion between me and @sh0rez. I primarily linked it because we were discussing whether this is a good idea and @sh0rez agreed that this is a feature/change that makes sense in Tanka. I didn't even bother making an issue for this because the thread felt like a sufficient ack from maintainers side.

There was likely more context but I don't recall the details. I hope this is helpful.

@curusarn
Copy link
Contributor Author

The thread also showed the motivation for this feature. Essentially:

  • Managed fields were added to Kubernetes which makes kubectl diff output incredibly noisy.
  • Kubernetes will solve this but the fix is not going to be back ported afaik.
  • kubectl-neat-diff solves this by filtering the resources before diffing them and it can be plugged into Tanka using env variable KUBECTL_EXTERNAL_DIFF.
  • However, when deleting some resources using tanka you still see managed fields because tanka doesn't use KUBECTL_EXTERNAL_DIFF for deleted and created stuff.
  • This PR changes tanka behavior to also use the KUBECTL_EXTERNAL_DIFF for creations and deletions.

@CLAassistant
Copy link

CLAassistant commented Jun 15, 2022

CLA assistant check
All committers have signed the CLA.

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

Successfully merging this pull request may close these issues.

None yet

4 participants