Commit 8d0016b5 authored by Brendan Burns's avatar Brendan Burns

Address comments.

parent 0a3aa224
...@@ -783,8 +783,8 @@ func (m *Master) InstallThirdPartyAPI(rsrc *experimental.ThirdPartyResource) err ...@@ -783,8 +783,8 @@ func (m *Master) InstallThirdPartyAPI(rsrc *experimental.ThirdPartyResource) err
if err := thirdparty.InstallREST(m.handlerContainer); err != nil { if err := thirdparty.InstallREST(m.handlerContainer); err != nil {
glog.Fatalf("Unable to setup thirdparty api: %v", err) glog.Fatalf("Unable to setup thirdparty api: %v", err)
} }
thirdPartyPrefix := "/thirdparty/" + group + "/" thirdPartyAPIPrefix := makeThirdPartyPath(group) + "/"
apiserver.AddApiWebService(m.handlerContainer, thirdPartyPrefix, []string{rsrc.Versions[0].Name}) apiserver.AddApiWebService(m.handlerContainer, thirdPartyAPIPrefix, []string{rsrc.Versions[0].Name})
thirdPartyRequestInfoResolver := &apiserver.APIRequestInfoResolver{APIPrefixes: sets.NewString(strings.TrimPrefix(group, "/")), RestMapper: thirdparty.Mapper} thirdPartyRequestInfoResolver := &apiserver.APIRequestInfoResolver{APIPrefixes: sets.NewString(strings.TrimPrefix(group, "/")), RestMapper: thirdparty.Mapper}
apiserver.InstallServiceErrorHandler(m.handlerContainer, thirdPartyRequestInfoResolver, []string{thirdparty.Version}) apiserver.InstallServiceErrorHandler(m.handlerContainer, thirdPartyRequestInfoResolver, []string{thirdparty.Version})
return nil return nil
...@@ -793,7 +793,7 @@ func (m *Master) InstallThirdPartyAPI(rsrc *experimental.ThirdPartyResource) err ...@@ -793,7 +793,7 @@ func (m *Master) InstallThirdPartyAPI(rsrc *experimental.ThirdPartyResource) err
func (m *Master) thirdpartyapi(group, kind, version string) *apiserver.APIGroupVersion { func (m *Master) thirdpartyapi(group, kind, version string) *apiserver.APIGroupVersion {
resourceStorage := thirdpartyresourcedataetcd.NewREST(m.thirdPartyStorage, group, kind) resourceStorage := thirdpartyresourcedataetcd.NewREST(m.thirdPartyStorage, group, kind)
apiRoot := "/thirdparty/" + group + "/" apiRoot := makeThirdPartyPath(group) + "/"
storage := map[string]rest.Storage{ storage := map[string]rest.Storage{
strings.ToLower(kind) + "s": resourceStorage, strings.ToLower(kind) + "s": resourceStorage,
......
...@@ -479,7 +479,7 @@ func testInstallThirdPartyAPIListVersion(t *testing.T, version string) { ...@@ -479,7 +479,7 @@ func testInstallThirdPartyAPIListVersion(t *testing.T, version string) {
fakeClient.ExpectNotFoundGet(etcdtest.PathPrefix() + "/ThirdPartyResourceData/company.com/foos/default") fakeClient.ExpectNotFoundGet(etcdtest.PathPrefix() + "/ThirdPartyResourceData/company.com/foos/default")
resp, err := http.Get(server.URL + "/thirdparty/company.com/" + version + "/namespaces/default/foos") resp, err := http.Get(server.URL + "/apis/company.com/" + version + "/namespaces/default/foos")
if !assert.NoError(err) { if !assert.NoError(err) {
return return
} }
...@@ -558,7 +558,7 @@ func testInstallThirdPartyAPIGetVersion(t *testing.T, version string) { ...@@ -558,7 +558,7 @@ func testInstallThirdPartyAPIGetVersion(t *testing.T, version string) {
return return
} }
resp, err := http.Get(server.URL + "/thirdparty/company.com/" + version + "/namespaces/default/foos/test") resp, err := http.Get(server.URL + "/apis/company.com/" + version + "/namespaces/default/foos/test")
if !assert.NoError(err) { if !assert.NoError(err) {
return return
} }
...@@ -603,8 +603,9 @@ func testInstallThirdPartyAPIPostForVersion(t *testing.T, version string) { ...@@ -603,8 +603,9 @@ func testInstallThirdPartyAPIPostForVersion(t *testing.T, version string) {
return return
} }
resp, err := http.Post(server.URL+"/thirdparty/company.com/"+version+"/namespaces/default/foos", "application/json", bytes.NewBuffer(data)) resp, err := http.Post(server.URL+"/apis/company.com/"+version+"/namespaces/default/foos", "application/json", bytes.NewBuffer(data))
if !assert.NoError(err) { if !assert.NoError(err) {
t.Errorf("unexpected error: %v", err)
return return
} }
...@@ -670,7 +671,7 @@ func testInstallThirdPartyAPIDeleteVersion(t *testing.T, version string) { ...@@ -670,7 +671,7 @@ func testInstallThirdPartyAPIDeleteVersion(t *testing.T, version string) {
return return
} }
resp, err := http.Get(server.URL + "/thirdparty/company.com/" + version + "/namespaces/default/foos/test") resp, err := http.Get(server.URL + "/apis/company.com/" + version + "/namespaces/default/foos/test")
if !assert.NoError(err) { if !assert.NoError(err) {
return return
} }
...@@ -687,14 +688,14 @@ func testInstallThirdPartyAPIDeleteVersion(t *testing.T, version string) { ...@@ -687,14 +688,14 @@ func testInstallThirdPartyAPIDeleteVersion(t *testing.T, version string) {
t.Errorf("expected:\n%v\nsaw:\n%v\n", expectedObj, item) t.Errorf("expected:\n%v\nsaw:\n%v\n", expectedObj, item)
} }
resp, err = httpDelete(server.URL + "/thirdparty/company.com/" + version + "/namespaces/default/foos/test") resp, err = httpDelete(server.URL + "/apis/company.com/" + version + "/namespaces/default/foos/test")
if !assert.NoError(err) { if !assert.NoError(err) {
return return
} }
assert.Equal(http.StatusOK, resp.StatusCode) assert.Equal(http.StatusOK, resp.StatusCode)
resp, err = http.Get(server.URL + "/thirdparty/company.com/" + version + "/namespaces/default/foos/test") resp, err = http.Get(server.URL + "/apis/company.com/" + version + "/namespaces/default/foos/test")
if !assert.NoError(err) { if !assert.NoError(err) {
return return
} }
......
...@@ -21,7 +21,7 @@ import ( ...@@ -21,7 +21,7 @@ import (
"strings" "strings"
"k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/expapi" expapi "k8s.io/kubernetes/pkg/apis/experimental"
"k8s.io/kubernetes/pkg/fields" "k8s.io/kubernetes/pkg/fields"
"k8s.io/kubernetes/pkg/labels" "k8s.io/kubernetes/pkg/labels"
thirdpartyresourceetcd "k8s.io/kubernetes/pkg/registry/thirdpartyresource/etcd" thirdpartyresourceetcd "k8s.io/kubernetes/pkg/registry/thirdpartyresource/etcd"
...@@ -30,13 +30,13 @@ import ( ...@@ -30,13 +30,13 @@ import (
"k8s.io/kubernetes/pkg/util/sets" "k8s.io/kubernetes/pkg/util/sets"
) )
const thirdpartyprefix = "/thirdparty/" const thirdpartyprefix = "/apis/"
func makeThirdPartyPath(group string) string { func makeThirdPartyPath(group string) string {
return thirdpartyprefix + group return thirdpartyprefix + group
} }
// resourceInterface is the interface for the parts of the master than know how to add/remove // resourceInterface is the interface for the parts of the master that know how to add/remove
// third party resources. Extracted into an interface for injection for testing. // third party resources. Extracted into an interface for injection for testing.
type resourceInterface interface { type resourceInterface interface {
// Remove a third party resource based on the RESTful path for that resource // Remove a third party resource based on the RESTful path for that resource
...@@ -58,6 +58,9 @@ type ThirdPartyController struct { ...@@ -58,6 +58,9 @@ type ThirdPartyController struct {
// Synchronize a single resource with RESTful resources on the master // Synchronize a single resource with RESTful resources on the master
func (t *ThirdPartyController) SyncOneResource(rsrc *expapi.ThirdPartyResource) error { func (t *ThirdPartyController) SyncOneResource(rsrc *expapi.ThirdPartyResource) error {
// TODO: we also need to test if the existing installed resource matches the resource we are sync-ing.
// Currently, if there is an older, incompatible resource installed, we won't remove it. We should detect
// older, incompatible resources and remove them before testing if the resource exists.
hasResource, err := t.master.HasThirdPartyResource(rsrc) hasResource, err := t.master.HasThirdPartyResource(rsrc)
if err != nil { if err != nil {
return err return err
...@@ -105,7 +108,7 @@ func (t *ThirdPartyController) syncResourceList(list runtime.Object) error { ...@@ -105,7 +108,7 @@ func (t *ThirdPartyController) syncResourceList(list runtime.Object) error {
found := false found := false
// search across the expected restful resources to see if this resource belongs to one of the expected ones // search across the expected restful resources to see if this resource belongs to one of the expected ones
for _, apiPath := range existing.List() { for _, apiPath := range existing.List() {
if strings.HasPrefix(installedAPI, apiPath) { if installedAPI == apiPath || strings.HasPrefix(installedAPI, apiPath+"/") {
found = true found = true
break break
} }
......
...@@ -20,7 +20,7 @@ import ( ...@@ -20,7 +20,7 @@ import (
"testing" "testing"
"k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/expapi" expapi "k8s.io/kubernetes/pkg/apis/experimental"
"k8s.io/kubernetes/pkg/registry/thirdpartyresourcedata" "k8s.io/kubernetes/pkg/registry/thirdpartyresourcedata"
"k8s.io/kubernetes/pkg/util/sets" "k8s.io/kubernetes/pkg/util/sets"
) )
...@@ -94,8 +94,8 @@ func TestSyncAPIs(t *testing.T) { ...@@ -94,8 +94,8 @@ func TestSyncAPIs(t *testing.T) {
}, },
}, },
apis: []string{ apis: []string{
"/thirdparty/example.com", "/apis/example.com",
"/thirdparty/example.com/v1", "/apis/example.com/v1",
}, },
name: "does nothing", name: "does nothing",
}, },
...@@ -107,6 +107,28 @@ func TestSyncAPIs(t *testing.T) { ...@@ -107,6 +107,28 @@ func TestSyncAPIs(t *testing.T) {
Name: "foo.example.com", Name: "foo.example.com",
}, },
}, },
},
},
apis: []string{
"/apis/example.com",
"/apis/example.com/v1",
"/apis/example.co",
"/apis/example.co/v1",
},
name: "deletes substring API",
expectedRemoved: []string{
"/apis/example.co",
"/apis/example.co/v1",
},
},
{
list: &expapi.ThirdPartyResourceList{
Items: []expapi.ThirdPartyResource{
{
ObjectMeta: api.ObjectMeta{
Name: "foo.example.com",
},
},
{ {
ObjectMeta: api.ObjectMeta{ ObjectMeta: api.ObjectMeta{
Name: "foo.company.com", Name: "foo.company.com",
...@@ -115,8 +137,8 @@ func TestSyncAPIs(t *testing.T) { ...@@ -115,8 +137,8 @@ func TestSyncAPIs(t *testing.T) {
}, },
}, },
apis: []string{ apis: []string{
"/thirdparty/company.com", "/apis/company.com",
"/thirdparty/company.com/v1", "/apis/company.com/v1",
}, },
expectedInstalled: []string{"foo.example.com"}, expectedInstalled: []string{"foo.example.com"},
name: "adds with existing", name: "adds with existing",
...@@ -132,11 +154,11 @@ func TestSyncAPIs(t *testing.T) { ...@@ -132,11 +154,11 @@ func TestSyncAPIs(t *testing.T) {
}, },
}, },
apis: []string{ apis: []string{
"/thirdparty/company.com", "/apis/company.com",
"/thirdparty/company.com/v1", "/apis/company.com/v1",
}, },
expectedInstalled: []string{"foo.example.com"}, expectedInstalled: []string{"foo.example.com"},
expectedRemoved: []string{"/thirdparty/company.com", "/thirdparty/company.com/v1"}, expectedRemoved: []string{"/apis/company.com", "/apis/company.com/v1"},
name: "removes with existing", name: "removes with existing",
}, },
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment