From 4787c66214dde2d7da1fe7cc71a408cdf79ebf9e Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Wed, 29 Apr 2015 11:17:55 -0700 Subject: [PATCH] Register the API versions to the DefaultRESTMapper in the order of preferred versions first. This makes kubectl sends requests to URLs in the format defined by the latest API version. --- hack/e2e-internal/e2e-cluster-size.sh | 2 +- hack/test-cmd.sh | 6 +++--- pkg/api/latest/latest.go | 3 ++- pkg/api/latest/latest_test.go | 5 ++--- pkg/api/meta/restmapper.go | 6 ++++-- pkg/kubectl/cmd/delete_test.go | 4 ++-- pkg/kubectl/cmd/get_test.go | 2 +- 7 files changed, 15 insertions(+), 13 deletions(-) diff --git a/hack/e2e-internal/e2e-cluster-size.sh b/hack/e2e-internal/e2e-cluster-size.sh index 84886e5f97d..e53f09202e6 100755 --- a/hack/e2e-internal/e2e-cluster-size.sh +++ b/hack/e2e-internal/e2e-cluster-size.sh @@ -31,4 +31,4 @@ source "${KUBE_VERSION_ROOT}/cluster/${KUBERNETES_PROVIDER}/util.sh" prepare-e2e -${KUBECTL} get minions --no-headers | wc -l +${KUBECTL} get nodes --no-headers | wc -l diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh index 2d425e044f1..77e7f7567f2 100755 --- a/hack/test-cmd.sh +++ b/hack/test-cmd.sh @@ -126,14 +126,14 @@ for version in "${kube_api_versions[@]}"; do -s "http://127.0.0.1:${API_PORT}" --match-server-version ) - [ "$(kubectl get minions -t $'{{ .apiVersion }}' "${kube_flags[@]}")" == "v1beta3" ] + [ "$(kubectl get minions -t '{{ .apiVersion }}' "${kube_flags[@]}")" == "v1beta3" ] else kube_flags=( -s "http://127.0.0.1:${API_PORT}" --match-server-version --api-version="${version}" ) - [ "$(kubectl get minions -t $'{{ .apiVersion }}' "${kube_flags[@]}")" == "${version}" ] + [ "$(kubectl get minions -t '{{ .apiVersion }}' "${kube_flags[@]}")" == "${version}" ] fi id_field=".metadata.name" labels_field=".metadata.labels" @@ -625,7 +625,7 @@ __EOF__ # Minions # ########### - if [[ "${version}" != "v1beta3" ]]; then + if [[ "${version}" = "v1beta1" ]] || [[ "${version}" = "v1beta2" ]]; then kube::log::status "Testing kubectl(${version}:minions)" kube::test::get_object_assert minions "{{range.items}}{{$id_field}}:{{end}}" '127.0.0.1:' diff --git a/pkg/api/latest/latest.go b/pkg/api/latest/latest.go index 799e072c755..61df0e24a9f 100644 --- a/pkg/api/latest/latest.go +++ b/pkg/api/latest/latest.go @@ -110,7 +110,8 @@ func init() { }, ) // list of versions we support on the server - versions := []string{"v1beta1", "v1beta2", "v1beta3", "v1"} + // versions should be listed in the order of perferred versions first + versions := []string{"v1beta3", "v1beta2", "v1beta1", "v1"} // versions that used mixed case URL formats versionMixedCase := map[string]bool{ diff --git a/pkg/api/latest/latest_test.go b/pkg/api/latest/latest_test.go index 6b1591b5117..a80d15e2eb0 100644 --- a/pkg/api/latest/latest_test.go +++ b/pkg/api/latest/latest_test.go @@ -72,11 +72,10 @@ func TestInterfacesFor(t *testing.T) { } func TestRESTMapper(t *testing.T) { - // TODO: This test does not seem right. The version returned here depends on the order in which API versions were registered. This will just return the API version that was registered first. Fix this. - if v, k, err := RESTMapper.VersionAndKindForResource("replicationControllers"); err != nil || v != "v1beta1" || k != "ReplicationController" { + if v, k, err := RESTMapper.VersionAndKindForResource("replicationControllers"); err != nil || v != "v1beta3" || k != "ReplicationController" { t.Errorf("unexpected version mapping: %s %s %v", v, k, err) } - if v, k, err := RESTMapper.VersionAndKindForResource("replicationcontrollers"); err != nil || v != "v1beta1" || k != "ReplicationController" { + if v, k, err := RESTMapper.VersionAndKindForResource("replicationcontrollers"); err != nil || v != "v1beta3" || k != "ReplicationController" { t.Errorf("unexpected version mapping: %s %s %v", v, k, err) } diff --git a/pkg/api/meta/restmapper.go b/pkg/api/meta/restmapper.go index 117a942c328..0421c0e2cbc 100644 --- a/pkg/api/meta/restmapper.go +++ b/pkg/api/meta/restmapper.go @@ -115,7 +115,9 @@ func NewDefaultRESTMapper(versions []string, f VersionInterfacesFunc) *DefaultRE func (m *DefaultRESTMapper) Add(scope RESTScope, kind string, version string, mixedCase bool) { plural, singular := kindToResource(kind, mixedCase) meta := typeMeta{APIVersion: version, Kind: kind} - if _, ok := m.mapping[plural]; !ok { + _, ok1 := m.mapping[plural] + _, ok2 := m.mapping[strings.ToLower(plural)] + if !ok1 && !ok2 { m.mapping[plural] = meta m.mapping[singular] = meta if strings.ToLower(plural) != plural { @@ -155,7 +157,7 @@ func kindToResource(kind string, mixedCase bool) (plural, singular string) { // VersionAndKindForResource implements RESTMapper func (m *DefaultRESTMapper) VersionAndKindForResource(resource string) (defaultVersion, kind string, err error) { - meta, ok := m.mapping[resource] + meta, ok := m.mapping[strings.ToLower(resource)] if !ok { return "", "", fmt.Errorf("no resource %q has been defined", resource) } diff --git a/pkg/kubectl/cmd/delete_test.go b/pkg/kubectl/cmd/delete_test.go index 45dade1ce7b..e9eac612047 100644 --- a/pkg/kubectl/cmd/delete_test.go +++ b/pkg/kubectl/cmd/delete_test.go @@ -53,7 +53,7 @@ func TestDeleteObjectByTuple(t *testing.T) { cmd.Flags().Set("cascade", "false") cmd.Run(cmd, []string{"replicationcontrollers/redis-master-controller"}) - if buf.String() != "replicationControllers/redis-master-controller\n" { + if buf.String() != "replicationcontrollers/redis-master-controller\n" { t.Errorf("unexpected output: %s", buf.String()) } } @@ -84,7 +84,7 @@ func TestDeleteNamedObject(t *testing.T) { cmd.Flags().Set("cascade", "false") cmd.Run(cmd, []string{"replicationcontrollers", "redis-master-controller"}) - if buf.String() != "replicationControllers/redis-master-controller\n" { + if buf.String() != "replicationcontrollers/redis-master-controller\n" { t.Errorf("unexpected output: %s", buf.String()) } } diff --git a/pkg/kubectl/cmd/get_test.go b/pkg/kubectl/cmd/get_test.go index 452ffd9ff8d..ab0c04098d0 100644 --- a/pkg/kubectl/cmd/get_test.go +++ b/pkg/kubectl/cmd/get_test.go @@ -163,7 +163,7 @@ func TestGetUnknownSchemaObjectListGeneric(t *testing.T) { output: "unlikelyversion", list: "v1beta3", obj1: "unlikelyversion", // doesn't have v1beta3 - obj2: "v1beta1", // version of the API response + obj2: "v1beta3", // version of the API response }, "handles common version": { output: "v1beta1",