From 0a3aa2242b63ad2c996ce4266e70d66f8a3e3393 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Wed, 9 Sep 2015 14:36:02 -0700 Subject: [PATCH 1/2] Add a third party controller that creates/deletes third party apis --- pkg/master/thirdparty_controller.go | 122 +++++++++++++++ pkg/master/thirdparty_controller_test.go | 182 +++++++++++++++++++++++ 2 files changed, 304 insertions(+) create mode 100644 pkg/master/thirdparty_controller.go create mode 100644 pkg/master/thirdparty_controller_test.go diff --git a/pkg/master/thirdparty_controller.go b/pkg/master/thirdparty_controller.go new file mode 100644 index 00000000000..6101b7f7c12 --- /dev/null +++ b/pkg/master/thirdparty_controller.go @@ -0,0 +1,122 @@ +/* +Copyright 2014 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package master + +import ( + "fmt" + "strings" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/expapi" + "k8s.io/kubernetes/pkg/fields" + "k8s.io/kubernetes/pkg/labels" + thirdpartyresourceetcd "k8s.io/kubernetes/pkg/registry/thirdpartyresource/etcd" + "k8s.io/kubernetes/pkg/registry/thirdpartyresourcedata" + "k8s.io/kubernetes/pkg/runtime" + "k8s.io/kubernetes/pkg/util/sets" +) + +const thirdpartyprefix = "/thirdparty/" + +func makeThirdPartyPath(group string) string { + return thirdpartyprefix + group +} + +// resourceInterface is the interface for the parts of the master than know how to add/remove +// third party resources. Extracted into an interface for injection for testing. +type resourceInterface interface { + // Remove a third party resource based on the RESTful path for that resource + RemoveThirdPartyResource(path string) error + // Install a third party resource described by 'rsrc' + InstallThirdPartyResource(rsrc *expapi.ThirdPartyResource) error + // Is a particular third party resource currently installed? + HasThirdPartyResource(rsrc *expapi.ThirdPartyResource) (bool, error) + // List all currently installed third party resources + ListThirdPartyResources() []string +} + +// ThirdPartyController is a control loop that knows how to synchronize ThirdPartyResource objects with +// RESTful resources which are present in the API server. +type ThirdPartyController struct { + master resourceInterface + thirdPartyResourceRegistry *thirdpartyresourceetcd.REST +} + +// Synchronize a single resource with RESTful resources on the master +func (t *ThirdPartyController) SyncOneResource(rsrc *expapi.ThirdPartyResource) error { + hasResource, err := t.master.HasThirdPartyResource(rsrc) + if err != nil { + return err + } + if !hasResource { + return t.master.InstallThirdPartyResource(rsrc) + } + return nil +} + +// Synchronize all resources with RESTful resources on the master +func (t *ThirdPartyController) SyncResources() error { + list, err := t.thirdPartyResourceRegistry.List(api.NewDefaultContext(), labels.Everything(), fields.Everything()) + if err != nil { + return err + } + return t.syncResourceList(list) +} + +func (t *ThirdPartyController) syncResourceList(list runtime.Object) error { + existing := sets.String{} + switch list := list.(type) { + case *expapi.ThirdPartyResourceList: + // Loop across all schema objects for third party resources + for ix := range list.Items { + item := &list.Items[ix] + // extract the api group and resource kind from the schema + _, group, err := thirdpartyresourcedata.ExtractApiGroupAndKind(item) + if err != nil { + return err + } + // place it in the set of resources that we expect, so that we don't delete it in the delete pass + existing.Insert(makeThirdPartyPath(group)) + // ensure a RESTful resource for this schema exists on the master + if err := t.SyncOneResource(item); err != nil { + return err + } + } + default: + return fmt.Errorf("expected a *ThirdPartyResourceList, got %#v", list) + } + // deletion phase, get all installed RESTful resources + installed := t.master.ListThirdPartyResources() + for _, installedAPI := range installed { + found := false + // search across the expected restful resources to see if this resource belongs to one of the expected ones + for _, apiPath := range existing.List() { + if strings.HasPrefix(installedAPI, apiPath) { + found = true + break + } + } + // not expected, delete the resource + if !found { + if err := t.master.RemoveThirdPartyResource(installedAPI); err != nil { + return err + } + } + } + + return nil +} diff --git a/pkg/master/thirdparty_controller_test.go b/pkg/master/thirdparty_controller_test.go new file mode 100644 index 00000000000..920124f7f44 --- /dev/null +++ b/pkg/master/thirdparty_controller_test.go @@ -0,0 +1,182 @@ +/* +Copyright 2014 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package master + +import ( + "testing" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/expapi" + "k8s.io/kubernetes/pkg/registry/thirdpartyresourcedata" + "k8s.io/kubernetes/pkg/util/sets" +) + +type FakeAPIInterface struct { + removed []string + installed []*expapi.ThirdPartyResource + apis []string + t *testing.T +} + +func (f *FakeAPIInterface) RemoveThirdPartyResource(path string) error { + f.removed = append(f.removed, path) + return nil +} + +func (f *FakeAPIInterface) InstallThirdPartyResource(rsrc *expapi.ThirdPartyResource) error { + f.installed = append(f.installed, rsrc) + _, group, _ := thirdpartyresourcedata.ExtractApiGroupAndKind(rsrc) + f.apis = append(f.apis, makeThirdPartyPath(group)) + return nil +} + +func (f *FakeAPIInterface) HasThirdPartyResource(rsrc *expapi.ThirdPartyResource) (bool, error) { + if f.apis == nil { + return false, nil + } + _, group, _ := thirdpartyresourcedata.ExtractApiGroupAndKind(rsrc) + path := makeThirdPartyPath(group) + for _, api := range f.apis { + if api == path { + return true, nil + } + } + return false, nil +} + +func (f *FakeAPIInterface) ListThirdPartyResources() []string { + return f.apis +} + +func TestSyncAPIs(t *testing.T) { + tests := []struct { + list *expapi.ThirdPartyResourceList + apis []string + expectedInstalled []string + expectedRemoved []string + name string + }{ + { + list: &expapi.ThirdPartyResourceList{ + Items: []expapi.ThirdPartyResource{ + { + ObjectMeta: api.ObjectMeta{ + Name: "foo.example.com", + }, + }, + }, + }, + expectedInstalled: []string{"foo.example.com"}, + name: "simple add", + }, + { + list: &expapi.ThirdPartyResourceList{ + Items: []expapi.ThirdPartyResource{ + { + ObjectMeta: api.ObjectMeta{ + Name: "foo.example.com", + }, + }, + }, + }, + apis: []string{ + "/thirdparty/example.com", + "/thirdparty/example.com/v1", + }, + name: "does nothing", + }, + { + list: &expapi.ThirdPartyResourceList{ + Items: []expapi.ThirdPartyResource{ + { + ObjectMeta: api.ObjectMeta{ + Name: "foo.example.com", + }, + }, + { + ObjectMeta: api.ObjectMeta{ + Name: "foo.company.com", + }, + }, + }, + }, + apis: []string{ + "/thirdparty/company.com", + "/thirdparty/company.com/v1", + }, + expectedInstalled: []string{"foo.example.com"}, + name: "adds with existing", + }, + { + list: &expapi.ThirdPartyResourceList{ + Items: []expapi.ThirdPartyResource{ + { + ObjectMeta: api.ObjectMeta{ + Name: "foo.example.com", + }, + }, + }, + }, + apis: []string{ + "/thirdparty/company.com", + "/thirdparty/company.com/v1", + }, + expectedInstalled: []string{"foo.example.com"}, + expectedRemoved: []string{"/thirdparty/company.com", "/thirdparty/company.com/v1"}, + name: "removes with existing", + }, + } + + for _, test := range tests { + fake := FakeAPIInterface{ + apis: test.apis, + t: t, + } + + cntrl := ThirdPartyController{master: &fake} + + if err := cntrl.syncResourceList(test.list); err != nil { + t.Errorf("[%s] unexpected error: %v", test.name) + } + if len(test.expectedInstalled) != len(fake.installed) { + t.Errorf("[%s] unexpected installed APIs: %d, expected %d (%#v)", test.name, len(fake.installed), len(test.expectedInstalled), fake.installed[0]) + continue + } else { + names := sets.String{} + for ix := range fake.installed { + names.Insert(fake.installed[ix].Name) + } + for _, name := range test.expectedInstalled { + if !names.Has(name) { + t.Errorf("[%s] missing installed API: %s", test.name, name) + } + } + } + if len(test.expectedRemoved) != len(fake.removed) { + t.Errorf("[%s] unexpected installed APIs: %d, expected %d", test.name, len(fake.removed), len(test.expectedRemoved)) + continue + } else { + names := sets.String{} + names.Insert(fake.removed...) + for _, name := range test.expectedRemoved { + if !names.Has(name) { + t.Errorf("[%s] missing removed API: %s (%s)", test.name, name, names) + } + } + } + } +} From 8d0016b54216f526bba7daadb743a1e7d5401906 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Mon, 14 Sep 2015 13:37:40 -0700 Subject: [PATCH 2/2] Address comments. --- pkg/master/master.go | 6 ++-- pkg/master/master_test.go | 13 ++++---- pkg/master/thirdparty_controller.go | 11 ++++--- pkg/master/thirdparty_controller_test.go | 38 +++++++++++++++++++----- 4 files changed, 47 insertions(+), 21 deletions(-) diff --git a/pkg/master/master.go b/pkg/master/master.go index 6e702fc8f38..76dd3dc658a 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -783,8 +783,8 @@ func (m *Master) InstallThirdPartyAPI(rsrc *experimental.ThirdPartyResource) err if err := thirdparty.InstallREST(m.handlerContainer); err != nil { glog.Fatalf("Unable to setup thirdparty api: %v", err) } - thirdPartyPrefix := "/thirdparty/" + group + "/" - apiserver.AddApiWebService(m.handlerContainer, thirdPartyPrefix, []string{rsrc.Versions[0].Name}) + thirdPartyAPIPrefix := makeThirdPartyPath(group) + "/" + apiserver.AddApiWebService(m.handlerContainer, thirdPartyAPIPrefix, []string{rsrc.Versions[0].Name}) thirdPartyRequestInfoResolver := &apiserver.APIRequestInfoResolver{APIPrefixes: sets.NewString(strings.TrimPrefix(group, "/")), RestMapper: thirdparty.Mapper} apiserver.InstallServiceErrorHandler(m.handlerContainer, thirdPartyRequestInfoResolver, []string{thirdparty.Version}) return nil @@ -793,7 +793,7 @@ func (m *Master) InstallThirdPartyAPI(rsrc *experimental.ThirdPartyResource) err func (m *Master) thirdpartyapi(group, kind, version string) *apiserver.APIGroupVersion { resourceStorage := thirdpartyresourcedataetcd.NewREST(m.thirdPartyStorage, group, kind) - apiRoot := "/thirdparty/" + group + "/" + apiRoot := makeThirdPartyPath(group) + "/" storage := map[string]rest.Storage{ strings.ToLower(kind) + "s": resourceStorage, diff --git a/pkg/master/master_test.go b/pkg/master/master_test.go index 2e2a1c340aa..8671a4c927a 100644 --- a/pkg/master/master_test.go +++ b/pkg/master/master_test.go @@ -479,7 +479,7 @@ func testInstallThirdPartyAPIListVersion(t *testing.T, version string) { 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) { return } @@ -558,7 +558,7 @@ func testInstallThirdPartyAPIGetVersion(t *testing.T, version string) { 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) { return } @@ -603,8 +603,9 @@ func testInstallThirdPartyAPIPostForVersion(t *testing.T, version string) { 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) { + t.Errorf("unexpected error: %v", err) return } @@ -670,7 +671,7 @@ func testInstallThirdPartyAPIDeleteVersion(t *testing.T, version string) { 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) { return } @@ -687,14 +688,14 @@ func testInstallThirdPartyAPIDeleteVersion(t *testing.T, version string) { 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) { return } 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) { return } diff --git a/pkg/master/thirdparty_controller.go b/pkg/master/thirdparty_controller.go index 6101b7f7c12..e1fe03100a6 100644 --- a/pkg/master/thirdparty_controller.go +++ b/pkg/master/thirdparty_controller.go @@ -21,7 +21,7 @@ import ( "strings" "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/labels" thirdpartyresourceetcd "k8s.io/kubernetes/pkg/registry/thirdpartyresource/etcd" @@ -30,13 +30,13 @@ import ( "k8s.io/kubernetes/pkg/util/sets" ) -const thirdpartyprefix = "/thirdparty/" +const thirdpartyprefix = "/apis/" func makeThirdPartyPath(group string) string { 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. type resourceInterface interface { // Remove a third party resource based on the RESTful path for that resource @@ -58,6 +58,9 @@ type ThirdPartyController struct { // Synchronize a single resource with RESTful resources on the master 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) if err != nil { return err @@ -105,7 +108,7 @@ func (t *ThirdPartyController) syncResourceList(list runtime.Object) error { found := false // search across the expected restful resources to see if this resource belongs to one of the expected ones for _, apiPath := range existing.List() { - if strings.HasPrefix(installedAPI, apiPath) { + if installedAPI == apiPath || strings.HasPrefix(installedAPI, apiPath+"/") { found = true break } diff --git a/pkg/master/thirdparty_controller_test.go b/pkg/master/thirdparty_controller_test.go index 920124f7f44..4fe37584065 100644 --- a/pkg/master/thirdparty_controller_test.go +++ b/pkg/master/thirdparty_controller_test.go @@ -20,7 +20,7 @@ import ( "testing" "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/util/sets" ) @@ -94,11 +94,33 @@ func TestSyncAPIs(t *testing.T) { }, }, apis: []string{ - "/thirdparty/example.com", - "/thirdparty/example.com/v1", + "/apis/example.com", + "/apis/example.com/v1", }, name: "does nothing", }, + { + list: &expapi.ThirdPartyResourceList{ + Items: []expapi.ThirdPartyResource{ + { + ObjectMeta: api.ObjectMeta{ + 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{ @@ -115,8 +137,8 @@ func TestSyncAPIs(t *testing.T) { }, }, apis: []string{ - "/thirdparty/company.com", - "/thirdparty/company.com/v1", + "/apis/company.com", + "/apis/company.com/v1", }, expectedInstalled: []string{"foo.example.com"}, name: "adds with existing", @@ -132,11 +154,11 @@ func TestSyncAPIs(t *testing.T) { }, }, apis: []string{ - "/thirdparty/company.com", - "/thirdparty/company.com/v1", + "/apis/company.com", + "/apis/company.com/v1", }, 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", }, }