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

Installing multiple third party resources on same resource path prefix has issues #23831

Closed
uruddarraju opened this issue Apr 4, 2016 · 20 comments · Fixed by #24129
Closed
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.

Comments

@uruddarraju
Copy link

I am trying to install the following thirdparty resources:

metadata:
  name: compute-node.tess.io
apiVersion: extensions/v1beta1
kind: ThirdPartyResource
description: "A compute node object is used by the mastercontroller to track computes in cloud"
versions:
 - name: v1

And

metadata:
  name: node-pool.tess.io
apiVersion: extensions/v1beta1
kind: ThirdPartyResource
description: "A node pool object to nodes is analogous to what a replicationcontroller is to pods"
versions:
 - name: v1

The first one gets successfully registered when I try to create it. But the second on never gets registered and kubectl does not throw an error out either. I digged a little deep into the kube-apiserver logs and I see the following error:

Recovered from panic: "http: multiple registrations for /apis/tess.io/v1" (http: multiple registrations for /apis/tess.io/v1)

When I digged deep into the thirdparty controller and master.go, here: https://github.com/kubernetes/kubernetes/blob/master/pkg/master/master.go#L611, I see that we have not covered the case where there can be 2 API objects defined on the same resource path prefix, like /apis/tess.io/v1/nodepools and /apis/tess.io/v1/computenodes

Is there something I am missing, or is this a genuine issue you think ?

@uruddarraju uruddarraju changed the title Installing multiple third party resources on same path has issues Installing multiple third party resources on same resource path prefix has issues Apr 4, 2016
@uruddarraju
Copy link
Author

Also, from here https://github.com/kubernetes/kubernetes/blob/master/pkg/master/master.go#L522

From what I understand, if I try to register a new Third Party resource whose group already exists, it considers that thirdpartyreource to already exist and would not take any action to register the new resource.

@uruddarraju
Copy link
Author

cc @brendandburns @wojtek-t

@wojtek-t
Copy link
Member

wojtek-t commented Apr 5, 2016

@lavalamp @kubernetes/sig-api-machinery

@uruddarraju
Copy link
Author

Also, once I create a thirdpartyresource and delete it, for some reason I have not been able to recreate any other thirdparty resources. I am running master on head using vagrant.

@lavalamp lavalamp added kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Apr 5, 2016
@lavalamp
Copy link
Member

lavalamp commented Apr 5, 2016

Looks like bugs. Thanks for reporting.

@uruddarraju
Copy link
Author

The gist of the problem is that, there is no way we can modify a webservice with more routes and storages once it is installed. So we have to delete and reinstall the webservice with all the right things whenever a new thirdpartyresource is to be installed. But the removal of the webservice from the container is leaving some trails in the ServeMux.m map, which is a map of routes to handler functions.

go-restful does 2 things whenever you want to add a new webservice to the container.

  1. add the webservice to the registeredwebservices list and
  2. register a handle function. .

But, during a remove, it undos the 1 but not 2.

From the go-restful perspective, if we add a webservice and remove it and if I try to add it back, it results in a panic.

@uruddarraju
Copy link
Author

I will try to reproduce it with a simple http application using go-restful and will file an issue if I am able to reproduce the issue.

@lavalamp
Copy link
Member

lavalamp commented Apr 7, 2016

Thanks for all your effort, Uday!

On Wed, Apr 6, 2016 at 4:51 PM, Uday Ruddarraju notifications@github.com
wrote:

I will try to reproduce it with a simple http application using go-restful
and will file an issue if I am able to reproduce the issue.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#23831 (comment)

@uruddarraju
Copy link
Author

Was able to reproduce the issue with just go-restful today, where I was able to successfully add a webservice to a container, remove it and when I add it back, I see the following issue:

panic: http: multiple registrations for /users

I tried the example here: https://github.com/emicklei/go-restful/blob/master/examples/restful-user-resource.go. And changed the lines https://github.com/emicklei/go-restful/blob/master/examples/restful-user-resource.go#L74 to do,

    container.Add(ws)
    container.Remove(ws)
    container.Add(ws)

This created the panic above.

@uruddarraju
Copy link
Author

This explains why a thirdparty registration followed by unregistration, cannot be registered back (Which I think is critical for us.)

@uruddarraju
Copy link
Author

@lavalamp Created an issue with go-restful: emicklei/go-restful#274. Please add any details if I may have missed something.

@uruddarraju
Copy link
Author

There was an other issue with the RemoveRoute call in go-restful for webservices having more than one route. Fixed it in emicklei/go-restful#275

@uruddarraju
Copy link
Author

@lavalamp I still have to submit a fix to fix this. The PR only updates the godep :)

@sdminonne
Copy link
Contributor

/sub

@duglin
Copy link

duglin commented Jul 9, 2016

FYI - I'm getting similar errors even when I use a different API group (domain name on the resources).

@duglin
Copy link

duglin commented Jul 9, 2016

Here's a gist with a simple test showing the issue: https://gist.github.com/duglin/ffe983ba6f15ad8f1bad968ccbcdcba3

k8s-github-robot pushed a commit that referenced this issue Jul 22, 2016
Automatic merge from submit-queue

Allow multiple APIs to register for the same API Group

Fixes #23831

@kubernetes/sig-api-machinery 

[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/.github/PULL_REQUEST_TEMPLATE.md?pixel)]()
@nebril
Copy link
Contributor

nebril commented Aug 2, 2016

I think this one should be reopened, the problem is still there on c7ec11b

@brendandburns
Copy link
Contributor

I believe that #29724 should have the fix, can you test it out?

@caesarxuchao
Copy link
Member

Thanks @brendandburns. The added test-cmd.sh test in #29724 looks proper.

k8s-github-robot pushed a commit that referenced this issue Aug 14, 2016
Automatic merge from submit-queue

Fix third party APIResource reporting

@polvi @caesarxuchao @deads2k 

This "fixes" some additional bugs in third party `APIResourceList` reporting.

This code needs a bunch of cleanup, and more tests, but sending it out for a quick smell check review in case I'm doing something stupid.

Fixes the bug referenced here:  #28414 (comment) and in #23831

Fixes #25570
@brendandburns
Copy link
Contributor

Closing this since #29724 is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants