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

Drop k8s.io/kubernetes dependency #56

Open
ash2k opened this issue Jun 10, 2020 · 21 comments · May be fixed by #535
Open

Drop k8s.io/kubernetes dependency #56

ash2k opened this issue Jun 10, 2020 · 21 comments · May be fixed by #535

Comments

@ash2k
Copy link
Member

ash2k commented Jun 10, 2020

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

Imported:

@ash2k
Copy link
Member Author

ash2k commented Jun 10, 2020

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
Copy link
Member Author

ash2k commented Jun 10, 2020

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
Copy link
Member Author

ash2k commented Jun 10, 2020

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
Copy link
Member Author

ash2k commented Jun 15, 2020

@mdvorak
Copy link

mdvorak commented Jul 9, 2020

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)

@ghostsquad
Copy link

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?

@mdvorak
Copy link

mdvorak commented Jul 27, 2020

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 kubernetes/kubernetes#81878

@ghostsquad
Copy link

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
Copy link

ghostsquad commented Jul 28, 2020

this one is hairy:

"k8s.io/kubernetes/pkg/kubectl/cmd/auth"

lots to copy

as this pulls in

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

which pulls in:

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

@ash2k
Copy link
Member Author

ash2k commented Nov 3, 2020

Some good progress kubernetes/kubernetes#96145

@ash2k
Copy link
Member Author

ash2k commented Feb 5, 2021

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 #205 merges our only blocker for dropping k/k dependency is the defaulting that we are using from scheme - see #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.

@alexmt
Copy link
Contributor

alexmt commented Feb 5, 2021

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.

@sbose78
Copy link
Contributor

sbose78 commented Mar 19, 2021

We should pick this up soon :)

@celrenheit
Copy link

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.

@ash2k
Copy link
Member Author

ash2k commented May 26, 2021

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

@celrenheit
Copy link

Oh I see, nvm the silly question

manno pushed a commit to manno/fleet that referenced this issue Jul 5, 2022
Kubernetes hasn't been updated in argoproj.
The files have been copied and import paths have been adapted.

Since argo-cd and gitops-engine use the server scheme from
k8s.io/kubernetes we need to work around 'unknown revision 0.0.0'
errors, by including replace statements in the go.mod file.

kubernetes/issues/79384

Vendoring the argoproj files helps with updating k8s, but is not enough
to get rid of the replace statements.

argoproj/gitops-engine/issues/56
manno pushed a commit to manno/fleet-staging that referenced this issue Jul 5, 2022
Kubernetes hasn't been updated in argoproj.
The files have been copied and import paths have been adapted.

Since argo-cd and gitops-engine use the server scheme from
k8s.io/kubernetes we need to work around 'unknown revision 0.0.0'
errors, by including replace statements in the go.mod file.

kubernetes/issues/79384

Vendoring the argoproj files helps with updating k8s, but is not enough
to get rid of the replace statements.

argoproj/gitops-engine/issues/56
manno pushed a commit to manno/fleet-staging that referenced this issue Jul 5, 2022
Kubernetes hasn't been updated in argoproj.
The files have been copied and import paths have been adapted.

Since argo-cd and gitops-engine use the server scheme from
k8s.io/kubernetes we need to work around 'unknown revision 0.0.0'
errors, by including replace statements in the go.mod file.

kubernetes/issues/79384

Vendoring the argoproj files helps with updating k8s, but is not enough
to get rid of the replace statements.

argoproj/gitops-engine/issues/56
mattfarina pushed a commit to mattfarina/fleet that referenced this issue Jul 12, 2022
Kubernetes hasn't been updated in argoproj.
The files have been copied and import paths have been adapted.

Since argo-cd and gitops-engine use the server scheme from
k8s.io/kubernetes we need to work around 'unknown revision 0.0.0'
errors, by including replace statements in the go.mod file.

kubernetes/issues/79384

Vendoring the argoproj files helps with updating k8s, but is not enough
to get rid of the replace statements.

argoproj/gitops-engine/issues/56

(cherry picked from commit 701a2f6)
@abursavich
Copy link

abursavich commented Jan 5, 2023

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?

@twmb
Copy link

twmb commented Jan 10, 2023

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.

@ghostsquad
Copy link

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.

@abursavich
Copy link

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.

@jonathan-lo
Copy link

Are there any updates on this?

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