From b864a5a808da63e0163b04bef20a5c0e959c5f8e Mon Sep 17 00:00:00 2001 From: Jonathan Basseri Date: Mon, 3 Dec 2018 16:17:06 -0800 Subject: [PATCH] Remove scheduler "TestGroup" utility. This util was used to fake certain aspects of apiserver behavior, such as resource paths and JSON encoding. Our unit tests have been refactored so they don't rely on the REST or JSON aspects of apiserver. This util is no longer needed. --- pkg/scheduler/factory/BUILD | 1 - pkg/scheduler/factory/factory_test.go | 14 +- pkg/scheduler/testing/BUILD | 26 +--- pkg/scheduler/testing/util.go | 173 --------------------- pkg/scheduler/testing/util_test.go | 214 -------------------------- 5 files changed, 7 insertions(+), 421 deletions(-) delete mode 100644 pkg/scheduler/testing/util.go delete mode 100644 pkg/scheduler/testing/util_test.go diff --git a/pkg/scheduler/factory/BUILD b/pkg/scheduler/factory/BUILD index 9a2e20ce296..c503aa315b5 100644 --- a/pkg/scheduler/factory/BUILD +++ b/pkg/scheduler/factory/BUILD @@ -72,7 +72,6 @@ go_test( "//pkg/scheduler/cache:go_default_library", "//pkg/scheduler/internal/cache/fake:go_default_library", "//pkg/scheduler/internal/queue:go_default_library", - "//pkg/scheduler/testing:go_default_library", "//pkg/scheduler/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/scheduler/factory/factory_test.go b/pkg/scheduler/factory/factory_test.go index aa68c716fc0..3936ef361d9 100644 --- a/pkg/scheduler/factory/factory_test.go +++ b/pkg/scheduler/factory/factory_test.go @@ -40,7 +40,6 @@ import ( schedulercache "k8s.io/kubernetes/pkg/scheduler/cache" fakecache "k8s.io/kubernetes/pkg/scheduler/internal/cache/fake" internalqueue "k8s.io/kubernetes/pkg/scheduler/internal/queue" - schedulertesting "k8s.io/kubernetes/pkg/scheduler/testing" "k8s.io/kubernetes/pkg/scheduler/util" ) @@ -366,18 +365,15 @@ func testBind(binding *v1.Binding, t *testing.T) { pod := client.CoreV1().Pods(metav1.NamespaceDefault).(*fakeV1.FakePods) - bind, err := pod.GetBinding(binding.GetName()) + actualBinding, err := pod.GetBinding(binding.GetName()) if err != nil { t.Fatalf("Unexpected error: %v", err) return } - - expectedBody := runtime.EncodeOrDie(schedulertesting.Test.Codec(), binding) - bind.APIVersion = "" - bind.Kind = "" - body := runtime.EncodeOrDie(schedulertesting.Test.Codec(), bind) - if expectedBody != body { - t.Errorf("Expected body %s, Got %s", expectedBody, body) + if !reflect.DeepEqual(binding, actualBinding) { + t.Errorf("Binding did not match expectation") + t.Logf("Expected: %v", binding) + t.Logf("Actual: %v", actualBinding) } } diff --git a/pkg/scheduler/testing/BUILD b/pkg/scheduler/testing/BUILD index 91acca08c54..b64d47c514c 100644 --- a/pkg/scheduler/testing/BUILD +++ b/pkg/scheduler/testing/BUILD @@ -1,22 +1,12 @@ package(default_visibility = ["//visibility:public"]) -load( - "@io_bazel_rules_go//go:def.bzl", - "go_library", - "go_test", -) +load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "go_default_library", - srcs = [ - "fake_lister.go", - "util.go", - ], + srcs = ["fake_lister.go"], importpath = "k8s.io/kubernetes/pkg/scheduler/testing", deps = [ - "//pkg/api/legacyscheme:go_default_library", - "//pkg/apis/core:go_default_library", - "//pkg/apis/core/install:go_default_library", "//pkg/scheduler/algorithm:go_default_library", "//pkg/scheduler/internal/cache:go_default_library", "//staging/src/k8s.io/api/apps/v1:go_default_library", @@ -24,8 +14,6 @@ go_library( "//staging/src/k8s.io/api/policy/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", ], ) @@ -42,13 +30,3 @@ filegroup( srcs = [":package-srcs"], tags = ["automanaged"], ) - -go_test( - name = "go_default_test", - srcs = ["util_test.go"], - embed = [":go_default_library"], - deps = [ - "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", - ], -) diff --git a/pkg/scheduler/testing/util.go b/pkg/scheduler/testing/util.go deleted file mode 100644 index 25921a2e058..00000000000 --- a/pkg/scheduler/testing/util.go +++ /dev/null @@ -1,173 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -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 testing - -import ( - "fmt" - "mime" - "os" - "reflect" - "strings" - - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/kubernetes/pkg/api/legacyscheme" - api "k8s.io/kubernetes/pkg/apis/core" - - // Init the core api installation - _ "k8s.io/kubernetes/pkg/apis/core/install" -) - -// TestGroup defines a api group for testing. -type TestGroup struct { - externalGroupVersion schema.GroupVersion - internalGroupVersion schema.GroupVersion - internalTypes map[string]reflect.Type - externalTypes map[string]reflect.Type -} - -var ( - // Groups defines a TestGroup map. - Groups = make(map[string]TestGroup) - // Test defines a TestGroup object. - Test TestGroup - - serializer runtime.SerializerInfo -) - -func init() { - if apiMediaType := os.Getenv("KUBE_TEST_API_TYPE"); len(apiMediaType) > 0 { - var ok bool - mediaType, _, err := mime.ParseMediaType(apiMediaType) - if err != nil { - panic(err) - } - serializer, ok = runtime.SerializerInfoForMediaType(legacyscheme.Codecs.SupportedMediaTypes(), mediaType) - if !ok { - panic(fmt.Sprintf("no serializer for %s", apiMediaType)) - } - } - - kubeTestAPI := os.Getenv("KUBE_TEST_API") - if len(kubeTestAPI) != 0 { - // priority is "first in list preferred", so this has to run in reverse order - testGroupVersions := strings.Split(kubeTestAPI, ",") - for i := len(testGroupVersions) - 1; i >= 0; i-- { - gvString := testGroupVersions[i] - groupVersion, err := schema.ParseGroupVersion(gvString) - if err != nil { - panic(fmt.Sprintf("Error parsing groupversion %v: %v", gvString, err)) - } - - internalGroupVersion := schema.GroupVersion{Group: groupVersion.Group, Version: runtime.APIVersionInternal} - Groups[groupVersion.Group] = TestGroup{ - externalGroupVersion: groupVersion, - internalGroupVersion: internalGroupVersion, - internalTypes: legacyscheme.Scheme.KnownTypes(internalGroupVersion), - externalTypes: legacyscheme.Scheme.KnownTypes(groupVersion), - } - } - } - - if _, ok := Groups[api.GroupName]; !ok { - externalGroupVersion := schema.GroupVersion{Group: api.GroupName, Version: "v1"} - Groups[api.GroupName] = TestGroup{ - externalGroupVersion: externalGroupVersion, - internalGroupVersion: api.SchemeGroupVersion, - internalTypes: legacyscheme.Scheme.KnownTypes(api.SchemeGroupVersion), - externalTypes: legacyscheme.Scheme.KnownTypes(externalGroupVersion), - } - } - - Test = Groups[api.GroupName] -} - -// Codec returns the codec for the API version to test against, as set by the -// KUBE_TEST_API_TYPE env var. -func (g TestGroup) Codec() runtime.Codec { - if serializer.Serializer == nil { - return legacyscheme.Codecs.LegacyCodec(g.externalGroupVersion) - } - return legacyscheme.Codecs.CodecForVersions(serializer.Serializer, legacyscheme.Codecs.UniversalDeserializer(), schema.GroupVersions{g.externalGroupVersion}, nil) -} - -// SelfLink returns a self link that will appear to be for the version Version(). -// 'resource' should be the resource path, e.g. "pods" for the Pod type. 'name' should be -// empty for lists. -func (g TestGroup) SelfLink(resource, name string) string { - if g.externalGroupVersion.Group == api.GroupName { - if name == "" { - return fmt.Sprintf("/api/%s/%s", g.externalGroupVersion.Version, resource) - } - return fmt.Sprintf("/api/%s/%s/%s", g.externalGroupVersion.Version, resource, name) - } - - // TODO: will need a /apis prefix once we have proper multi-group - // support - if name == "" { - return fmt.Sprintf("/apis/%s/%s/%s", g.externalGroupVersion.Group, g.externalGroupVersion.Version, resource) - } - return fmt.Sprintf("/apis/%s/%s/%s/%s", g.externalGroupVersion.Group, g.externalGroupVersion.Version, resource, name) -} - -// ResourcePathWithPrefix returns the appropriate path for the given prefix (watch, proxy, redirect, etc), resource, namespace and name. -// For ex, this is of the form: -// /api/v1/watch/namespaces/foo/pods/pod0 for v1. -func (g TestGroup) ResourcePathWithPrefix(prefix, resource, namespace, name string) string { - var path string - if g.externalGroupVersion.Group == api.GroupName { - path = "/api/" + g.externalGroupVersion.Version - } else { - // TODO: switch back once we have proper multiple group support - // path = "/apis/" + g.Group + "/" + Version(group...) - path = "/apis/" + g.externalGroupVersion.Group + "/" + g.externalGroupVersion.Version - } - - if prefix != "" { - path = path + "/" + prefix - } - if namespace != "" { - path = path + "/namespaces/" + namespace - } - // Resource names are lower case. - resource = strings.ToLower(resource) - if resource != "" { - path = path + "/" + resource - } - if name != "" { - path = path + "/" + name - } - return path -} - -// ResourcePath returns the appropriate path for the given resource, namespace and name. -// For example, this is of the form: -// /api/v1/namespaces/foo/pods/pod0 for v1. -func (g TestGroup) ResourcePath(resource, namespace, name string) string { - return g.ResourcePathWithPrefix("", resource, namespace, name) -} - -// SubResourcePath returns the appropriate path for the given resource, namespace, -// name and subresource. -func (g TestGroup) SubResourcePath(resource, namespace, name, sub string) string { - path := g.ResourcePathWithPrefix("", resource, namespace, name) - if sub != "" { - path = path + "/" + sub - } - - return path -} diff --git a/pkg/scheduler/testing/util_test.go b/pkg/scheduler/testing/util_test.go deleted file mode 100644 index e6545b00559..00000000000 --- a/pkg/scheduler/testing/util_test.go +++ /dev/null @@ -1,214 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -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 testing - -import ( - "encoding/json" - "reflect" - "testing" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" -) - -func TestResourcePathWithPrefix(t *testing.T) { - testCases := []struct { - prefix string - resource string - namespace string - name string - expected string - }{ - {"prefix", "resource", "mynamespace", "myresource", "/api/" + Test.externalGroupVersion.Version + "/prefix/namespaces/mynamespace/resource/myresource"}, - {"prefix", "resource", "", "myresource", "/api/" + Test.externalGroupVersion.Version + "/prefix/resource/myresource"}, - {"prefix", "resource", "mynamespace", "", "/api/" + Test.externalGroupVersion.Version + "/prefix/namespaces/mynamespace/resource"}, - {"prefix", "resource", "", "", "/api/" + Test.externalGroupVersion.Version + "/prefix/resource"}, - {"", "resource", "mynamespace", "myresource", "/api/" + Test.externalGroupVersion.Version + "/namespaces/mynamespace/resource/myresource"}, - } - for _, item := range testCases { - if actual := Test.ResourcePathWithPrefix(item.prefix, item.resource, item.namespace, item.name); actual != item.expected { - t.Errorf("Expected: %s, got: %s for prefix: %s, resource: %s, namespace: %s and name: %s", item.expected, actual, item.prefix, item.resource, item.namespace, item.name) - } - } - - TestGroup := Test - TestGroup.externalGroupVersion.Group = "TestGroup" - - testGroupCases := []struct { - prefix string - resource string - namespace string - name string - expected string - }{ - {"prefix", "resource", "mynamespace", "myresource", "/apis/" + TestGroup.externalGroupVersion.Group + "/" + TestGroup.externalGroupVersion.Version + "/prefix/namespaces/mynamespace/resource/myresource"}, - {"prefix", "resource", "", "myresource", "/apis/" + TestGroup.externalGroupVersion.Group + "/" + TestGroup.externalGroupVersion.Version + "/prefix/resource/myresource"}, - {"prefix", "resource", "mynamespace", "", "/apis/" + TestGroup.externalGroupVersion.Group + "/" + TestGroup.externalGroupVersion.Version + "/prefix/namespaces/mynamespace/resource"}, - {"prefix", "resource", "", "", "/apis/" + TestGroup.externalGroupVersion.Group + "/" + TestGroup.externalGroupVersion.Version + "/prefix/resource"}, - {"", "resource", "mynamespace", "myresource", "/apis/" + TestGroup.externalGroupVersion.Group + "/" + TestGroup.externalGroupVersion.Version + "/namespaces/mynamespace/resource/myresource"}, - } - for _, item := range testGroupCases { - if actual := TestGroup.ResourcePathWithPrefix(item.prefix, item.resource, item.namespace, item.name); actual != item.expected { - t.Errorf("Expected: %s, got: %s for prefix: %s, resource: %s, namespace: %s and name: %s", item.expected, actual, item.prefix, item.resource, item.namespace, item.name) - } - } - -} - -func TestResourcePath(t *testing.T) { - testCases := []struct { - resource string - namespace string - name string - expected string - }{ - {"resource", "mynamespace", "myresource", "/api/" + Test.externalGroupVersion.Version + "/namespaces/mynamespace/resource/myresource"}, - {"resource", "", "myresource", "/api/" + Test.externalGroupVersion.Version + "/resource/myresource"}, - {"resource", "mynamespace", "", "/api/" + Test.externalGroupVersion.Version + "/namespaces/mynamespace/resource"}, - {"resource", "", "", "/api/" + Test.externalGroupVersion.Version + "/resource"}, - } - for _, item := range testCases { - if actual := Test.ResourcePath(item.resource, item.namespace, item.name); actual != item.expected { - t.Errorf("Expected: %s, got: %s for resource: %s, namespace: %s and name: %s", item.expected, actual, item.resource, item.namespace, item.name) - } - } - - TestGroup := Test - TestGroup.externalGroupVersion.Group = "TestGroup" - - testGroupCases := []struct { - resource string - namespace string - name string - expected string - }{ - {"resource", "mynamespace", "myresource", "/apis/" + TestGroup.externalGroupVersion.Group + "/" + TestGroup.externalGroupVersion.Version + "/namespaces/mynamespace/resource/myresource"}, - {"resource", "", "myresource", "/apis/" + TestGroup.externalGroupVersion.Group + "/" + TestGroup.externalGroupVersion.Version + "/resource/myresource"}, - {"resource", "mynamespace", "", "/apis/" + TestGroup.externalGroupVersion.Group + "/" + TestGroup.externalGroupVersion.Version + "/namespaces/mynamespace/resource"}, - {"resource", "", "", "/apis/" + TestGroup.externalGroupVersion.Group + "/" + TestGroup.externalGroupVersion.Version + "/resource"}, - } - for _, item := range testGroupCases { - if actual := TestGroup.ResourcePath(item.resource, item.namespace, item.name); actual != item.expected { - t.Errorf("Expected: %s, got: %s for resource: %s, namespace: %s and name: %s", item.expected, actual, item.resource, item.namespace, item.name) - } - } - -} - -func TestSubResourcePath(t *testing.T) { - testCases := []struct { - resource string - namespace string - name string - sub string - expected string - }{ - {"resource", "mynamespace", "myresource", "subresource", "/api/" + Test.externalGroupVersion.Version + "/namespaces/mynamespace/resource/myresource/subresource"}, - {"resource", "mynamespace", "myresource", "", "/api/" + Test.externalGroupVersion.Version + "/namespaces/mynamespace/resource/myresource"}, - } - for _, item := range testCases { - if actual := Test.SubResourcePath(item.resource, item.namespace, item.name, item.sub); actual != item.expected { - t.Errorf("Expected: %s, got: %s for resource: %s, namespace: %s, name: %s and sub: %s", item.expected, actual, item.resource, item.namespace, item.name, item.sub) - } - } - - TestGroup := Test - TestGroup.externalGroupVersion.Group = "TestGroup" - - testGroupCases := []struct { - resource string - namespace string - name string - sub string - expected string - }{ - {"resource", "mynamespace", "myresource", "subresource", "/apis/" + TestGroup.externalGroupVersion.Group + "/" + TestGroup.externalGroupVersion.Version + "/namespaces/mynamespace/resource/myresource/subresource"}, - {"resource", "mynamespace", "myresource", "", "/apis/" + TestGroup.externalGroupVersion.Group + "/" + TestGroup.externalGroupVersion.Version + "/namespaces/mynamespace/resource/myresource"}, - } - for _, item := range testGroupCases { - if actual := TestGroup.SubResourcePath(item.resource, item.namespace, item.name, item.sub); actual != item.expected { - t.Errorf("Expected: %s, got: %s for resource: %s, namespace: %s, name: %s and sub: %s", item.expected, actual, item.resource, item.namespace, item.name, item.sub) - } - } - -} - -func TestSelfLink(t *testing.T) { - testCases := []struct { - resource string - name string - expected string - }{ - {"resource", "name", "/api/" + Test.externalGroupVersion.Version + "/resource/name"}, - {"resource", "", "/api/" + Test.externalGroupVersion.Version + "/resource"}, - } - for _, item := range testCases { - if actual := Test.SelfLink(item.resource, item.name); actual != item.expected { - t.Errorf("Expected: %s, got: %s for resource: %s and name: %s", item.expected, actual, item.resource, item.name) - } - } - - TestGroup := Test - TestGroup.externalGroupVersion.Group = "TestGroup" - - testGroupCases := []struct { - resource string - name string - expected string - }{ - {"resource", "name", "/apis/" + TestGroup.externalGroupVersion.Group + "/" + TestGroup.externalGroupVersion.Version + "/resource/name"}, - {"resource", "", "/apis/" + TestGroup.externalGroupVersion.Group + "/" + TestGroup.externalGroupVersion.Version + "/resource"}, - } - for _, item := range testGroupCases { - if actual := TestGroup.SelfLink(item.resource, item.name); actual != item.expected { - t.Errorf("Expected: %s, got: %s for resource: %s and name: %s", item.expected, actual, item.resource, item.name) - } - } -} - -var status = &metav1.Status{ - Status: metav1.StatusFailure, - Code: 200, - Reason: metav1.StatusReasonUnknown, - Message: "", -} - -func TestV1EncodeDecodeStatus(t *testing.T) { - v1Codec := Test.Codec() - - encoded, err := runtime.Encode(v1Codec, status) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - typeMeta := metav1.TypeMeta{} - if err := json.Unmarshal(encoded, &typeMeta); err != nil { - t.Errorf("unexpected error: %v", err) - } - if typeMeta.Kind != "Status" { - t.Errorf("Kind is not set to \"Status\". Got %v", string(encoded)) - } - if typeMeta.APIVersion != "v1" { - t.Errorf("APIVersion is not set to \"v1\". Got %v", string(encoded)) - } - decoded, err := runtime.Decode(v1Codec, encoded) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if !reflect.DeepEqual(status, decoded) { - t.Errorf("expected: %#v, got: %#v", status, decoded) - } -}