Skip to content

Commit

Permalink
Merge pull request kubernetes#54586 from DirectXMan12/bug/fix-incorre…
Browse files Browse the repository at this point in the history
…ct-scale-and-hpa-gvks

Automatic merge from submit-queue (batch tested with PRs 53645, 54734, 54586, 55015, 54688). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Fix Incorrect Scale Subresources and HPA e2e ScaleTargetRefs

The HPA e2es failed to actually set `apiVersion` on the created HPAs, which previous was ignored.  Since the polymorphic scale client was merged, this behavior is no longer tolerated (it was never correct to begin with, but it accidentally worked).

Additionally, the `apps` resources have their own version of scale.  Until `apps/v1beta1` and `apps/v1beta2` go away, we need to support those versions in the scale client.

Together, these broke some of the HPA e2es.

Fixes kubernetes#54574

```release-note
NONE
```
  • Loading branch information
Kubernetes Submit Queue authored Nov 6, 2017
2 parents 3625a32 + 2c9fc43 commit 67c9e74
Show file tree
Hide file tree
Showing 24 changed files with 781 additions and 36 deletions.
4 changes: 4 additions & 0 deletions hack/.golint_failures
Original file line number Diff line number Diff line change
Expand Up @@ -672,9 +672,13 @@ staging/src/k8s.io/client-go/rest/fake
staging/src/k8s.io/client-go/scale
staging/src/k8s.io/client-go/scale/fake
staging/src/k8s.io/client-go/scale/scheme
staging/src/k8s.io/client-go/scale/scheme/appsint
staging/src/k8s.io/client-go/scale/scheme/appsv1beta1
staging/src/k8s.io/client-go/scale/scheme/appsv1beta2
staging/src/k8s.io/client-go/scale/scheme/autoscalingv1
staging/src/k8s.io/client-go/scale/scheme/extensionsint
staging/src/k8s.io/client-go/scale/scheme/extensionsv1beta1
staging/src/k8s.io/client-go/scale/scheme/extensionsv1beta1
staging/src/k8s.io/client-go/testing
staging/src/k8s.io/client-go/tools/cache
staging/src/k8s.io/client-go/tools/cache/testing
Expand Down
4 changes: 4 additions & 0 deletions staging/src/k8s.io/client-go/scale/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ go_library(
"//vendor/k8s.io/client-go/dynamic:go_default_library",
"//vendor/k8s.io/client-go/rest:go_default_library",
"//vendor/k8s.io/client-go/scale/scheme:go_default_library",
"//vendor/k8s.io/client-go/scale/scheme/appsint:go_default_library",
"//vendor/k8s.io/client-go/scale/scheme/appsv1beta1:go_default_library",
"//vendor/k8s.io/client-go/scale/scheme/appsv1beta2:go_default_library",
"//vendor/k8s.io/client-go/scale/scheme/autoscalingv1:go_default_library",
"//vendor/k8s.io/client-go/scale/scheme/extensionsint:go_default_library",
"//vendor/k8s.io/client-go/scale/scheme/extensionsv1beta1:go_default_library",
Expand All @@ -36,6 +39,7 @@ go_test(
library = ":go_default_library",
deps = [
"//vendor/github.com/stretchr/testify/assert:go_default_library",
"//vendor/k8s.io/api/apps/v1beta1:go_default_library",
"//vendor/k8s.io/api/apps/v1beta2:go_default_library",
"//vendor/k8s.io/api/autoscaling/v1:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
Expand Down
41 changes: 39 additions & 2 deletions staging/src/k8s.io/client-go/scale/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
fakerest "k8s.io/client-go/rest/fake"

"github.com/stretchr/testify/assert"
appsv1beta1 "k8s.io/api/apps/v1beta1"
appsv1beta2 "k8s.io/api/apps/v1beta2"
autoscalingv1 "k8s.io/api/autoscaling/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -75,7 +76,14 @@ func fakeScaleClient(t *testing.T) (ScalesGetter, []schema.GroupResource) {
GroupVersion: appsv1beta2.SchemeGroupVersion.String(),
APIResources: []metav1.APIResource{
{Name: "deployments", Namespaced: true, Kind: "Deployment"},
{Name: "deployments/scale", Namespaced: true, Kind: "Scale", Group: "autoscaling", Version: "v1"},
{Name: "deployments/scale", Namespaced: true, Kind: "Scale", Group: "apps", Version: "v1beta2"},
},
},
{
GroupVersion: appsv1beta1.SchemeGroupVersion.String(),
APIResources: []metav1.APIResource{
{Name: "statefulsets", Namespaced: true, Kind: "StatefulSet"},
{Name: "statefulsets/scale", Namespaced: true, Kind: "Scale", Group: "apps", Version: "v1beta1"},
},
},
// test a resource that doesn't exist anywere to make sure we're not accidentally depending
Expand Down Expand Up @@ -123,11 +131,40 @@ func fakeScaleClient(t *testing.T) (ScalesGetter, []schema.GroupResource) {
TargetSelector: "foo=bar",
},
}
appsV1beta2Scale := &appsv1beta2.Scale{
TypeMeta: metav1.TypeMeta{
Kind: "Scale",
APIVersion: appsv1beta2.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
Spec: appsv1beta2.ScaleSpec{Replicas: 10},
Status: appsv1beta2.ScaleStatus{
Replicas: 10,
TargetSelector: "foo=bar",
},
}
appsV1beta1Scale := &appsv1beta1.Scale{
TypeMeta: metav1.TypeMeta{
Kind: "Scale",
APIVersion: appsv1beta1.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
Spec: appsv1beta1.ScaleSpec{Replicas: 10},
Status: appsv1beta1.ScaleStatus{
Replicas: 10,
TargetSelector: "foo=bar",
},
}

resourcePaths := map[string]runtime.Object{
"/api/v1/namespaces/default/replicationcontrollers/foo/scale": autoscalingScale,
"/apis/extensions/v1beta1/namespaces/default/replicasets/foo/scale": extScale,
"/apis/apps/v1beta2/namespaces/default/deployments/foo/scale": autoscalingScale,
"/apis/apps/v1beta1/namespaces/default/statefulsets/foo/scale": appsV1beta1Scale,
"/apis/apps/v1beta2/namespaces/default/deployments/foo/scale": appsV1beta2Scale,
"/apis/cheese.testing.k8s.io/v27alpha15/namespaces/default/cheddars/foo/scale": extScale,
}

Expand Down
3 changes: 3 additions & 0 deletions staging/src/k8s.io/client-go/scale/scheme/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ filegroup(
name = "all-srcs",
srcs = [
":package-srcs",
"//staging/src/k8s.io/client-go/scale/scheme/appsint:all-srcs",
"//staging/src/k8s.io/client-go/scale/scheme/appsv1beta1:all-srcs",
"//staging/src/k8s.io/client-go/scale/scheme/appsv1beta2:all-srcs",
"//staging/src/k8s.io/client-go/scale/scheme/autoscalingv1:all-srcs",
"//staging/src/k8s.io/client-go/scale/scheme/extensionsint:all-srcs",
"//staging/src/k8s.io/client-go/scale/scheme/extensionsv1beta1:all-srcs",
Expand Down
31 changes: 31 additions & 0 deletions staging/src/k8s.io/client-go/scale/scheme/appsint/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "go_default_library",
srcs = [
"doc.go",
"register.go",
],
importpath = "k8s.io/client-go/scale/scheme/appsint",
visibility = ["//visibility:public"],
deps = [
"//vendor/k8s.io/api/apps/v1beta2:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/k8s.io/client-go/scale/scheme:go_default_library",
],
)

filegroup(
name = "package-srcs",
srcs = glob(["**"]),
tags = ["automanaged"],
visibility = ["//visibility:private"],
)

filegroup(
name = "all-srcs",
srcs = [":package-srcs"],
tags = ["automanaged"],
visibility = ["//visibility:public"],
)
22 changes: 22 additions & 0 deletions staging/src/k8s.io/client-go/scale/scheme/appsint/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
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 appsint contains the necessary scaffolding of the
// internal version of extensions as required by conversion logic.
// It doesn't have any of its own types -- it's just necessary to
// get the expected behavoir out of runtime.Scheme.ConvertToVersion
// and associated methods.
package appsint
53 changes: 53 additions & 0 deletions staging/src/k8s.io/client-go/scale/scheme/appsint/register.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
Copyright 2016 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 appsint

import (
appsv1beta2 "k8s.io/api/apps/v1beta2"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
scalescheme "k8s.io/client-go/scale/scheme"
)

// GroupName is the group name use in this package
const GroupName = appsv1beta2.GroupName

// SchemeGroupVersion is group version used to register these objects
var SchemeGroupVersion = schema.GroupVersion{Group: GroupName, Version: runtime.APIVersionInternal}

// Kind takes an unqualified kind and returns a Group qualified GroupKind
func Kind(kind string) schema.GroupKind {
return SchemeGroupVersion.WithKind(kind).GroupKind()
}

// Resource takes an unqualified resource and returns a Group qualified GroupResource
func Resource(resource string) schema.GroupResource {
return SchemeGroupVersion.WithResource(resource).GroupResource()
}

var (
SchemeBuilder = runtime.NewSchemeBuilder(addKnownTypes)
AddToScheme = SchemeBuilder.AddToScheme
)

// Adds the list of known types to api.Scheme.
func addKnownTypes(scheme *runtime.Scheme) error {
scheme.AddKnownTypes(SchemeGroupVersion,
&scalescheme.Scale{},
)
return nil
}
35 changes: 35 additions & 0 deletions staging/src/k8s.io/client-go/scale/scheme/appsv1beta1/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "go_default_library",
srcs = [
"conversion.go",
"doc.go",
"register.go",
"zz_generated.conversion.go",
],
importpath = "k8s.io/client-go/scale/scheme/appsv1beta1",
visibility = ["//visibility:public"],
deps = [
"//vendor/k8s.io/api/apps/v1beta1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/conversion:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/k8s.io/client-go/scale/scheme:go_default_library",
],
)

filegroup(
name = "package-srcs",
srcs = glob(["**"]),
tags = ["automanaged"],
visibility = ["//visibility:private"],
)

filegroup(
name = "all-srcs",
srcs = [":package-srcs"],
tags = ["automanaged"],
visibility = ["//visibility:public"],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
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 appsv1beta1

import (
"fmt"

v1beta1 "k8s.io/api/apps/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/conversion"
"k8s.io/apimachinery/pkg/runtime"
scheme "k8s.io/client-go/scale/scheme"
)

// addConversions registers conversions between the internal version
// of Scale and supported external versions of Scale.
func addConversionFuncs(scheme *runtime.Scheme) error {
err := scheme.AddConversionFuncs(
Convert_scheme_ScaleStatus_To_v1beta1_ScaleStatus,
Convert_v1beta1_ScaleStatus_To_scheme_ScaleStatus,
)
if err != nil {
return err
}

return nil
}
func Convert_scheme_ScaleStatus_To_v1beta1_ScaleStatus(in *scheme.ScaleStatus, out *v1beta1.ScaleStatus, s conversion.Scope) error {
out.Replicas = in.Replicas
out.Selector = nil
out.TargetSelector = ""
if in.Selector != nil {
if in.Selector.MatchExpressions == nil || len(in.Selector.MatchExpressions) == 0 {
out.Selector = in.Selector.MatchLabels
}

selector, err := metav1.LabelSelectorAsSelector(in.Selector)
if err != nil {
return fmt.Errorf("invalid label selector: %v", err)
}
out.TargetSelector = selector.String()
}

return nil
}

func Convert_v1beta1_ScaleStatus_To_scheme_ScaleStatus(in *v1beta1.ScaleStatus, out *scheme.ScaleStatus, s conversion.Scope) error {
out.Replicas = in.Replicas

// Normally when 2 fields map to the same internal value we favor the old field, since
// old clients can't be expected to know about new fields but clients that know about the
// new field can be expected to know about the old field (though that's not quite true, due
// to kubectl apply). However, these fields are readonly, so any non-nil value should work.
if in.TargetSelector != "" {
labelSelector, err := metav1.ParseToLabelSelector(in.TargetSelector)
if err != nil {
out.Selector = nil
return fmt.Errorf("failed to parse target selector: %v", err)
}
out.Selector = labelSelector
} else if in.Selector != nil {
out.Selector = new(metav1.LabelSelector)
selector := make(map[string]string)
for key, val := range in.Selector {
selector[key] = val
}
out.Selector.MatchLabels = selector
} else {
out.Selector = nil
}

return nil
}
20 changes: 20 additions & 0 deletions staging/src/k8s.io/client-go/scale/scheme/appsv1beta1/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
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.
*/

// +k8s:conversion-gen=k8s.io/kubernetes/vendor/k8s.io/client-go/scale/scheme
// +k8s:conversion-gen-external-types=../../../../../k8s.io/api/apps/v1beta1

package appsv1beta1 // import "k8s.io/client-go/scale/scheme/appsv1beta1"
Loading

0 comments on commit 67c9e74

Please sign in to comment.