-
Notifications
You must be signed in to change notification settings - Fork 261
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 dependency on k8s.io/kubernetes #535
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Chris Hoffman <[email protected]>
pkg/utils/kube/scheme/scheme.go
Outdated
var Scheme = runtime.NewScheme() | ||
|
||
func init() { | ||
runtimeutil.Must(admissionv1.AddToScheme(Scheme)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not know it was possible to get defaults from k8s.io/api
package 🤯.
Thanks a lot for PR @cehoffman ! Removing conversion will affect PR but probably we can do something about it. E.g. cache converted version as well. Let me please test and see if significantly affects performance |
Kudos, SonarCloud Quality Gate passed! |
@cehoffman @alexmt is this PR still active? We would benefit from it also - happy to help where I can. |
Same here! |
This fixes #56 and helps on the path to addressing argoproj/argo-cd#14727
The go.mod and go.sum changes are from running
go mod tidy
.I misunderstood the initial output and now see that
gitops-engine
is using the private conversion functions ofk8s.io/kubernetes
. I'm now looking how it is being used since flux does not depend on that.It looks like the conversion is used in one place in order to avoid a round trip to the Kubernetes API server to perform the conversion. Given that immediately above this section the same API server round trip is done to get the resource and it is only avoided if the cluster cache has a copy of the resource already, it seems to me the conversion functionality imposes more burden on the wider ecosystem than is warranted to maintain.
The other usage is in diff for getting the default values for types. As is, defaulting isn't fully covered by these imports as types continue to evolve and gain new fields and types unknown to the system, e.g. CRDs. We use a few CRDs which make heavy use of field defaulting and they for sure are not known to the gitops-engine, i.e those types from GCP Config Connector. In practice we have not encountered anything impacting our usage of ArgoCD with these types.