GoogleCloudPlatform / gke-managed-certs

Managed Certificates for Kubernetes clusters using GCLB
Apache License 2.0
246 stars 32 forks source link

Error with dependency #36

Closed nicolasgere closed 4 years ago

nicolasgere commented 5 years ago

Hi, When I try to use the client package with go, there is an error with apimachinery

vendor/github.com/GoogleCloudPlatform/gke-managed-certs/pkg/clientgen/clientset/versioned/typed/networking.gke.io/v1beta1/networking.gke.io_client.go:74:32: undefined: serializer.DirectCodecFactory I use the master branch of apimachinery Which version of apimachinery do you use?

krzykwas commented 4 years ago

There have been a lot of changes since this comment was posted. Right now it is name = "k8s.io/apimachinery" branch = "release-1.15"

axisofentropy commented 4 years ago

Will this project soon be updated to work with newer k8s API versions? I started using gke-managed-certs because GKE offered v1beta2 on versions 1.16.5-gke.1 and higher, so I was surprised that version wasn't supported here.

krzykwas commented 4 years ago

Sorry, I don't understand your comment. The code in this repository is deployed in GKE as well, so the GKE version depends on the same version of apimachinery.

axisofentropy commented 4 years ago

We're using this code in our API client which is outside of GKE but talking to GKE.

krzykwas commented 4 years ago

So you've basically forked the code and you are deploying it yourself, right? I understand you'd like to have the dependencies upgraded to let's say 1.16. That makes sense, if you are deploying a cluster which is 1.16+. On the other hand the code right now depends on k8s API 1.15, but it works with k8s 1.14 as well as 1.16 (not sure about 1.17, though).

From the perspective of GKE, 1.16 is delivered via rapid channel and is not as stable as for instance 1.15. This code is deployed in various GKE releases and 1.15 API is good enough for that. If I upgrade to let's say 1.16, it might work with 1.14, but it's 2 versions apart, so I'm not really sure about that.

Alternatively I could maintain parallel branches targeted for various k8s/GKE versions, but it means a lot of additional effort and I won't do that.

At some point I'll upgrade the dependencies, but it is not planned for now. You can do that in your fork, though.

I'm still now sure if I understood the problem well. Could you elaborate more on your use case?

axisofentropy commented 4 years ago

We're doing a lot of automation. Haven't forked this project, just imported by adding to go.mod. We create a Clientset by calling NewForConfig() within github.com/GoogleCloudPlatform/gke-managed-certs/pkg/clientgen/clientset/versioned. Then we use it much like the k8s.io/client-go/ Clientset just for the ManagedCertificates.

We're also using Config Connector, but we don't like writing our own clients. That's why we used this project. I'm still very new to the Golang ecosystem so tell me if I'm missing something obvious.

Thanks for taking the time to wonder about our use case; rare on GitHub! We can wait a while for the upgrade; are you tracking with GKE's Regular channel? We just moved Clusters into Rapid for the v1beta2 API.

krzykwas commented 4 years ago

Ok, thanks, now I understand the problem.

For this project I used godep and now I switched to dep, but not go modules yet, that may be the next step. So you depend on this project not being go-modularized yet. Maybe you could depend on just the subset, I mean the generated API, not the whole project? This way you wouldn't pull in all the dependencies, also the ones you don't really need? Is it possible/would that work?

I'm not sure Config Connector would be the tool to use here, because its purpose is to expose Cloud APIs in k8s/GKE - here the API is already k8s/GKE-like, so it's not really needed (but maybe it works for your purpose anyway).

This code is released in all the GKE versions. The v1beta2 version was just released in the rapid channel, but if required/needed, the next release might as well go to the next 1.14 release. In principle, I think you should not have to depend on the implementation details of this project, i. e. for instance the version of apimachinery it depends on. Someone else might have to stay on 1.14 and the only way to support both of you would be to maintain different branches, that's not possible.

axisofentropy commented 4 years ago

Thanks for suggesting to use just the subset, maybe we can figure that out.

krzykwas commented 4 years ago

The project has been migrated to go modules.