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

Allow different resources to use different apiVersion #208

Open
garo opened this issue Nov 3, 2016 · 12 comments
Open

Allow different resources to use different apiVersion #208

garo opened this issue Nov 3, 2016 · 12 comments

Comments

@garo
Copy link

garo commented Nov 3, 2016

Currently the apiVersion is set on the Kubeclient::Client.new and it's enforced in https://github.com/abonas/kubeclient/blob/master/lib/kubeclient/common.rb#L320

We should instead allow that the resource contains apiVersion by itself and Kubeclient should honour that. This would allow easily to read a standard Kubernetes manifest .yaml file and apply that into the apiserver, so that the .yaml specifies the api version it requires.

@jeremywadsack
Copy link
Contributor

jeremywadsack commented Nov 11, 2016

Is that needed anymore? It was commented as a patch for kubernetes/kubernetes#6439, which according to comments on that issue has a commit that addresses it.

Also, +1 for not having to tie the client to a specific API version and endpoint, but allowing a single instance of the client to create resources for different manifest types.

@salilgupta1
Copy link
Contributor

Is this anywhere on the roadmap? It would be great to decouple the client from a specific API version

@simon3z
Copy link
Collaborator

simon3z commented Mar 1, 2017

cc @moolitayer

@moolitayer
Copy link
Collaborator

Not currently on my road map.

Designing the API (as well as backward compatibility) for this might prove somewhat complex.
for example this proposal describes put requests, but what is the plan for get, will we still enforce passing a version in initialization and use that?

I remember last time we discussed that, we had an idea of supporting some set_version which seems easier but might not support what is described here

I don't think I'll be investing time in this anytime soon since it is not needed for my usecases
(@zakiva my say otherwise, aka template initialization of different versions)

if @simon3z will be willing to accept a pr for this I will be willing to review.

@simon3z
Copy link
Collaborator

simon3z commented Mar 2, 2017

if @simon3z will be willing to accept a pr for this I will be willing to review.

I'd like to see how the design/api would look like before anyone investing into it.

@zakiva
Copy link
Contributor

zakiva commented Mar 2, 2017

(@zakiva my say otherwise, aka template initialization of different versions)

For the use case of template instantiation, we just create different clients according to the api and version we need. It could have been a little more elegant to use the same client with different versions, but I wouldn't consider it as a high priority item for us in this context.

@jeremywadsack
Copy link
Contributor

jeremywadsack commented Mar 4, 2017

It seems to me that on the all requests (including GET requests) you still have methods for each resource so maybe you can look up the default ApiVersion for a resource?

It would be nice to not have to have multiple clients. This pattern feels cumbersome:

      def jobs_client
        @jobs_client ||= client("/apis/batch")
      end

      def pods_client
        @pods_client ||= client("")
      end

      def clean_up
        jobs = jobs_client.get_jobs(label_selector: "resque-kubernetes=job")
        finished_jobs = jobs.select { |job| job.spec.completions == job.status.succeeded }

        finished_jobs.each do |job|
          jobs_client.delete_job(job.metadata.name, job.metadata.namespace)
        end

        pods = pods_client.get_pods(label_selector: "resque-kubernetes=pod")
        finished_pods = pods.select { |pod| pod.status.phase == "Succeeded" }

        finished_pods.each do |pod|
          pods_client.delete_pod(pod.metadata.name, pod.metadata.namespace)
        end
      end

@garo
Copy link
Author

garo commented Mar 4, 2017

I had to do a similar system where I create multiple clients for different api/extension versions and then try to find out the suitable client which supports a particular entity type.

It would be really great if kubeclient would just transparently support all different api versions with a single client instance.

@simon3z
Copy link
Collaborator

simon3z commented Mar 7, 2017

It would be really great if kubeclient would just transparently support all different api versions with a single client instance.

@garo OK for creation and update of resources (above request doesn't apply to getting resources).
It shouldn't too hard to submit a PR for that (creation/update).

Although be advised that if the different api versions are changing the URL paths, that case we may not be able to handle so easily.

@danajp
Copy link

danajp commented Mar 28, 2017

Hey all. I've opened a PR to make multiple apis with a single client instance work as well as detecting kind/apiVersion from a KubeClient::Resource. This is definitely WIP.

I'm looking for feedback on the approach at this point. If my general approach looks good, I'll continue with tests/cleanup.

@simon3z
Copy link
Collaborator

simon3z commented Mar 28, 2017

Thanks @danajp! I asked @moolitayer and @cben to review your WIP PR.

@cben
Copy link
Collaborator

cben commented Aug 14, 2018

Overview of directions

Opened separate issues for several proposed directions, for easier discussion. (cc @stiller-leser @kuahyeow)

What do you think?
IMHO #348 can be better than #241 & #346.
But #332 needs to happen anyway, is simple, and provides good foundation so that's IMHO best place to start.

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

Successfully merging a pull request may close this issue.

8 participants