gitops-engine icon indicating copy to clipboard operation
gitops-engine copied to clipboard

Drop k8s.io/kubernetes dependency

Open ash2k opened this issue 5 years ago • 21 comments

Please consider dropping dependency on k8s.io/kubernetes module as it makes the engine really unwieldy to use.

Imported:

  • https://github.com/argoproj/gitops-engine/blob/1723191ddef4cde1f62a7cb5e2d7eb743eb79cf5/pkg/diff/diff.go#L27 fixed in #57.
  • https://github.com/argoproj/gitops-engine/blob/df17961bbd04e2d98b52bc175cd0f1da96eb9d20/pkg/health/health.go#L17 copy it?
  • https://github.com/argoproj/gitops-engine/blob/8430dc06d70093793a05279ce002336822594394/pkg/utils/kube/convert.go#L7 a custom scheme can be constructed to avoid the dependency
  • https://github.com/argoproj/gitops-engine/blob/4bd4f29670eee1ad3889430a7d4245693e424547/pkg/utils/kube/ctl.go#L29 copy it?
  • https://github.com/argoproj/gitops-engine/blob/8430dc06d70093793a05279ce002336822594394/pkg/utils/kube/import_known_versions.go#L4 a custom scheme can be constructed to avoid the dependency

ash2k avatar Jun 10 '20 05:06 ash2k

One option would be to copy the bits that are not available in other packages. I'm willing to do the work if maintainers are ok with that. Please let me know.

ash2k avatar Jun 10 '20 05:06 ash2k

Hm, I have just discovered that I cannot use the engine because of this dependency. I'm getting the following error when building my project with bazel:

ERROR: /private/var/tmp/_bazel_mikhail/6a0c1de97b81a8c5de5b6b9aec394679/external/com_github_argoproj_gitops_engine/pkg/utils/kube/BUILD.bazel:3:11: in go_library rule @com_github_argoproj_gitops_engine//pkg/utils/kube:go_default_library: target '@io_k8s_kubernetes//pkg/kubectl/cmd/auth:go_default_library' is not visible from target '@com_github_argoproj_gitops_engine//pkg/utils/kube:go_default_library'. Check the visibility declaration of the former target if you think the dependency is legitimate

This is because https://github.com/kubernetes/kubernetes/blob/07d88617e55b64c243956c7b2032430beb03b159/pkg/kubectl/cmd/auth/BUILD#L16 prohibits to import this package. This is not a problem if Go modules are used, but the package cannot be used via bazel+gazelle (my case). Clearly package authors do not want anyone to depend on it.

ash2k avatar Jun 10 '20 05:06 ash2k

I've tried copying the code but ran into these issues:

  • k8s.io/kubernetes/pkg/kubectl/cmd/auth is a lot of logic, not just trivial helpers.
  • looks like creating a custom scheme is not an option because conversion functions are used but they are not exported from k/k.

So I guess I have to look for a workaround...

p.s. I think conversion functions might not be safe to use on the client side. An example: gitops-engine imports kubernetes version X and conversion functions for this version. The cluster may be of a different version Y and an object of the same group/version/kind might have a new field (I think adding fields within the same version is ok as long as default value matches absent field semantics). This field is unknown to conversion functions from version X and hence will be dropped. Because of that it is not safe to use conversion functions from any other version but Y.

ash2k avatar Jun 10 '20 06:06 ash2k

https://github.com/kubernetes/enhancements/issues/1020

ash2k avatar Jun 15 '20 10:06 ash2k

Same requirement will be for https://github.com/argoproj/argo-cd, but this needs to be fixed first (as gitops-engine is a dependency of argo-cd)

mdvorak avatar Jul 09 '20 09:07 mdvorak

Please consider dropping dependency on k8s.io/kubernetes module as it makes the engine really unwieldy to use.

I'm just curious what makes the dependency on kubernetes unwieldy exactly?

ghostsquad avatar Jul 27 '20 06:07 ghostsquad

Please consider dropping dependency on k8s.io/kubernetes module as it makes the engine really unwieldy to use.

I'm just curious what makes the dependency on kubernetes unwieldy exactly?

go module system - replace directive, used in argocd go.mod, is not used by anything that uses argo as own dependency (they are not transitive). Which means, that anyone depending on argo needs whole replace block for each k8s dependency, and maintain it.

Plus, using k8s.io/kubernetes is explicitly deprecated, as it intentionally depends on non-existent v0.0.0 versions of all other libraries. See also https://github.com/kubernetes/kubernetes/issues/81878

mdvorak avatar Jul 27 '20 08:07 mdvorak

ok, ya, I just ran into this:

go get: github.com/argoproj/[email protected] requires
	k8s.io/[email protected] requires
	k8s.io/[email protected]: reading k8s.io/api/go.mod at revision v0.0.0: unknown revision v0.0.0

ghostsquad avatar Jul 27 '20 20:07 ghostsquad

this one is hairy: https://github.com/argoproj/gitops-engine/blob/4bd4f29670eee1ad3889430a7d4245693e424547/pkg/utils/kube/ctl.go#L29

lots to copy

as this pulls in

"k8s.io/kubernetes/pkg/registry/rbac/reconciliation"

which pulls in:

"k8s.io/kubernetes/pkg/apis/core/helper"

ghostsquad avatar Jul 28 '20 00:07 ghostsquad

Some good progress https://github.com/kubernetes/kubernetes/pull/96145

ash2k avatar Nov 03 '20 16:11 ash2k

Some more good news: https://docs.google.com/document/d/121FWis7sIzdG83H9T8G2qvSB6inxJoUub5KsVwFDa60/edit#heading=h.mipaetrdowr and https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/2144-clientgo-apply#apply-functions

So after https://github.com/argoproj/gitops-engine/pull/205 merges our only blocker for dropping k/k dependency is the defaulting that we are using from scheme - see https://github.com/argoproj/gitops-engine/pull/206. Once client-go receives Server Side Apply support, my understanding is that it'll be possible to use this new API to issue a dry-run apply command to receive the new object. Then it should be possible to perform a diff between current in-cluster object and the returned one to see if something would change if we apply it for real. Obviously, I haven't tried that so I may be missing something here.

ash2k avatar Feb 05 '21 23:02 ash2k

Thank you for update and reminder about #205 and #206 ! I've started looking at PRs, then got distracted and totally forgot. Will try to review and test both next week.

alexmt avatar Feb 05 '21 23:02 alexmt

We should pick this up soon :)

sbose78 avatar Mar 19 '21 04:03 sbose78

Hey! I may be missing something but isn't the package https://pkg.go.dev/k8s.io/client-go/kubernetes/scheme able to replace the current usage of k8s.io/kubernetes.

celrenheit avatar May 26 '21 10:05 celrenheit

@celrenheit This is the client-side scheme, it does not contain conversion/defaulting functions the server-side one contains.

ash2k avatar May 26 '21 10:05 ash2k

Oh I see, nvm the silly question

celrenheit avatar May 26 '21 13:05 celrenheit

This is the client-side scheme, it does not contain conversion/defaulting functions the server-side one contains.

How do you handle custom resources for which conversions/defaulting functions aren't available?

abursavich avatar Jan 05 '23 23:01 abursavich

The last update nearly two years ago was that this would be picked up soon -- is there a way to move the project forward?

I spent a day on removing the dependency a year ago but took an approach that ultimately did not work and would not solve the issue.

twmb avatar Jan 10 '23 19:01 twmb

The problem with letting this continue to linger is that this really isn't a very reusable library in it's current state. It's quite painful to have to manage the mod file manually.

ghostsquad avatar Jan 24 '23 04:01 ghostsquad

Bump.

Importing the Argo CD API (e.g. https://pkg.go.dev/github.com/argoproj/argo-cd/[email protected]/pkg/apis/application/v1alpha1) brings in gitops-engine which brings in all of kubernetes and is a maintenance nightmare.

abursavich avatar May 23 '23 18:05 abursavich