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

Including CRD client into client-go repo #247

Closed
hongchaodeng opened this issue Jul 17, 2017 · 19 comments
Closed

Including CRD client into client-go repo #247

hongchaodeng opened this issue Jul 17, 2017 · 19 comments
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@hongchaodeng
Copy link
Contributor

Currently CRD client and api all sits in https://github.com/kubernetes/apiextensions-apiserver.

Eventually, CRD client would be better to live in client-go as well. It would be easier for users if client-go can manage dependencies, manage backward compatibility, and testing all in one place.

@ericchiang
Copy link
Contributor

Some background here: kubernetes/kubernetes#46702

@sttts still plan to create a "create an authoritative client in client-go"? Something we can help out with?

@lavalamp
Copy link
Member

lavalamp commented Jul 17, 2017 via email

@sttts
Copy link
Contributor

sttts commented Jul 18, 2017

if client-go can manage dependencies, manage backward compatibility, and testing all in one place.

Right now both apiextensions-apiserver and client-go are maintained, tested and exported from the same repo (k8s.io/kube). So for the moment the integration argument does't exist.

Now with the upcoming 1.8 we could move the types into k8s.io/api and the client into client-go, if we want to centralize everything there. /cc @deads2k @caesarxuchao

@lavalamp the document is outdated because it doesn't mention the role of k8s.io/api, isn't it?

@deads2k
Copy link
Contributor

deads2k commented Jul 18, 2017

Now with the upcoming 1.8 we could move the types into k8s.io/api and the client into client-go, if we want to centralize everything there. /cc @deads2k @caesarxuchao

I don't think we want to do that. The plan has been to have a core library that all clients share and then various clients can use it to have a common set of machinery for interoperability while maintaining logical separation. The technique appears to be working (the clients play together in e2e tests). I think we should maintain the logical separation.

@hongchaodeng
Copy link
Contributor Author

hongchaodeng commented Jul 19, 2017

@deads2k

The plan has been to have a core library

Where is the plan? Any docs?

@hongchaodeng
Copy link
Contributor Author

@lavalamp

Thanks for sharing.
From https://github.com/kubernetes/community/blob/master/contributors/design-proposals/csi-client-structure-proposal.md, is CRD client gonna sit in github.com/kubernetes-client/core-go ?

@caesarxuchao
Copy link
Member

caesarxuchao commented Jul 19, 2017

People usually use CRD together with other core API. I think it makes sense to let people access both API from one clientset.

We still plan to split client-go to core-go and base-go, but CRD doesn't have to live outside of core-go.

@nstott
Copy link

nstott commented Sep 29, 2017

People usually use CRD together with other core API. I think it makes sense to let people access both API from one clientset

the hoop jumping that's involved in managing tpr clients is awkward, any simplification here is great

@sigxcpu76
Copy link

I was bit by this today. I think the README needs to be adjusted because Exactly the same features / API objects in both client-go and the Kubernetes version. is misleading, isn't it?

@sigxcpu76
Copy link

client-go 5.0.0 does not include them either. Maybe it can be fixed before the release, as I've read that the current is the release candidate.

@caesarxuchao
Copy link
Member

@deads2k do you still feel strongly that the CRD type definition should remain in k8s.io/apiextension-apiserver, rather than in k8s.io/api? If so, we need to spend some time making the example clearer, or even adapting the clientset interface for easier integration.

@sttts
Copy link
Contributor

sttts commented Oct 10, 2017

or even adapting the clientset interface

I don't think this is the right solution. But I agree that we have to improve the experience.

We cannot import k8s.io/apiextension-apiserver here because it creates an import cycle. We could place a very simple _crd example here though. Underscore packages are allowed. And we have to improve the README.md, being explicit that k8s.io/apiextension-apiserver must be vendored.

@timothysc
Copy link
Member

timothysc commented Oct 11, 2017

It's a really bad UX, whose design is befuddling to most outside observers. I honestly don't know if someone new to the community would make any sense of what is there and why.

+1 for move into client-go

/cc @jbeda

@deads2k
Copy link
Contributor

deads2k commented Oct 11, 2017

It's a really bad UX, whose design is befuddling to most outside observers. I honestly don't know if someone new to the community would make any sense of what is there and why.

As new API servers (like service catalog) are added, this will be normal operating procedure. You need the shared client library and the different clients you want to use layered on top. We should devote our effort to making that as easy an obvious as possible, because client-go will not grow to contain everything.

As @sttts noted, we can help with clear examples and documentation, but patching in a fix in here doesn't serve the community in the mid to long term. client-go was designed for this composition and it is the only viable long term option. Let's try to make it hum.

@krmayankk
Copy link

@deads2k are you against moving CRD creation client in client-go ? Why or am i misunderstanding this ?

@deads2k
Copy link
Contributor

deads2k commented Nov 29, 2017

@deads2k are you against moving CRD creation client in client-go ? Why or am i misunderstanding this ?

I am against it. As our type ecosystem grows, we'll have more and more clients that must build on top of client-go. As it stands now, we have a clean logical separation of this type and its related server. Rather than take the "easy" path that combines unrelated API groups together (this isn't related to storage.k8s.io at all) and is only available to kubernetes members, we should make the intended client-go extension mechanisms easier to manage.

For instance, consider service-catalog or openshift/client-go. Their types will never be included in k8s.io/api, but many people would like to use both clients. The broader community needs to have an easy way to use multiple discrete client libraries and we need to be eating our own dog food or the story may never improve.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 27, 2018
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 29, 2018
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests