Merge pull request #17517 from liggitt/dc-scale

Automatic merge from submit-queue.

Fix deployment config scale subresource

3.6 and 3.7 shipped with the /apis/apps.openshift.io/deploymentconfigs/scale subresource returning apps.openshift.io/v1 Scale objects

This is a non-standard Scale object that the HPA will never be able to make use of.

The intent was to continue send the same thing as /oapi/v1/deploymentconfigs/scale: extensions/v1beta1 Scale objects

This PR fixes the groupified API to send/receive the correct type.

It also updates the UpdateScale() client method to send extensions/v1beta1 Scale objects

api/swagger-spec/openshift-openapi-spec.json

@@ -19461,8 +19461,8 @@
      },
      "x-kubernetes-action": "get",
      "x-kubernetes-group-version-kind": {
-      "group": "apps.openshift.io",
-      "version": "v1",
+      "group": "extensions",
+      "version": "v1beta1",
       "kind": "Scale"
      }
     },
@@ -19506,8 +19506,8 @@
      },
      "x-kubernetes-action": "put",
      "x-kubernetes-group-version-kind": {
-      "group": "apps.openshift.io",
-      "version": "v1",
+      "group": "extensions",
+      "version": "v1beta1",
       "kind": "Scale"
      }
     },
@@ -19553,8 +19553,8 @@
      },
      "x-kubernetes-action": "patch",
      "x-kubernetes-group-version-kind": {
-      "group": "apps.openshift.io",
-      "version": "v1",
+      "group": "extensions",
+      "version": "v1beta1",
       "kind": "Scale"
      }
     },

pkg/apps/apis/apps/types.go

@@ -138,7 +138,6 @@ const (
 // +genclient:method=Instantiate,verb=create,subresource=instantiate,input=DeploymentRequest
 // +genclient:method=Rollback,verb=create,subresource=rollback,input=DeploymentConfigRollback
 // +genclient:method=GetScale,verb=get,subresource=scale,result=k8s.io/kubernetes/pkg/apis/extensions/v1beta1.Scale
-// +genclient:method=UpdateScale,verb=update,subresource=scale,input=k8s.io/kubernetes/pkg/apis/extensions/v1beta1.Scale,result=k8s.io/kubernetes/pkg/apis/extensions/v1beta1.Scale
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
 // DeploymentConfig represents a configuration for a single deployment (represented as a

pkg/apps/apiserver/apiserver.go

@@ -12,6 +12,7 @@ import (
 	genericapiserver "k8s.io/apiserver/pkg/server"
 	restclient "k8s.io/client-go/rest"
 	kapi "k8s.io/kubernetes/pkg/api"
+	v1beta1extensions "k8s.io/kubernetes/pkg/apis/extensions/v1beta1"
 	kclientsetexternal "k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
 	kclientsetinternal "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	kubeletclient "k8s.io/kubernetes/pkg/kubelet/client"
@@ -84,6 +85,12 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget)
 	apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(appsapiv1.GroupName, c.Registry, c.Scheme, parameterCodec, c.Codecs)
 	apiGroupInfo.GroupMeta.GroupVersion = appsapiv1.SchemeGroupVersion
 	apiGroupInfo.VersionedResourcesStorageMap[appsapiv1.SchemeGroupVersion.Version] = v1Storage
+
+	if apiGroupInfo.SubresourceGroupVersionKind == nil {
+		apiGroupInfo.SubresourceGroupVersionKind = map[string]schema.GroupVersionKind{}
+	}
+	apiGroupInfo.SubresourceGroupVersionKind["deploymentconfigs/scale"] = v1beta1extensions.SchemeGroupVersion.WithKind("Scale")
+
 	if err := s.GenericAPIServer.InstallAPIGroup(&apiGroupInfo); err != nil {
 		return nil, err
 	}

pkg/apps/generated/internalclientset/scheme/register_expansion.go

@@ -0,0 +1,10 @@
+package scheme
+
+import (
+	extensionsv1beta1 "k8s.io/kubernetes/pkg/apis/extensions/v1beta1"
+)
+
+func init() {
+	// Needed for GetScale/UpdateScale
+	extensionsv1beta1.AddToScheme(Scheme)
+}

pkg/apps/generated/internalclientset/typed/apps/internalversion/deploymentconfig.go

@@ -30,7 +30,6 @@ type DeploymentConfigInterface interface {
 	Instantiate(deploymentConfigName string, deploymentRequest *apps.DeploymentRequest) (*apps.DeploymentConfig, error)
 	Rollback(deploymentConfigName string, deploymentConfigRollback *apps.DeploymentConfigRollback) (*apps.DeploymentConfig, error)
 	GetScale(deploymentConfigName string, options v1.GetOptions) (*v1beta1.Scale, error)
-	UpdateScale(deploymentConfigName string, scale *v1beta1.Scale) (*v1beta1.Scale, error)
 
 	DeploymentConfigExpansion
 }
@@ -202,17 +201,3 @@ func (c *deploymentConfigs) GetScale(deploymentConfigName string, options v1.Get
 		Into(result)
 	return
 }
-
-// UpdateScale takes the top resource name and the representation of a scale and updates it. Returns the server's representation of the scale, and an error, if there is any.
-func (c *deploymentConfigs) UpdateScale(deploymentConfigName string, scale *v1beta1.Scale) (result *v1beta1.Scale, err error) {
-	result = &v1beta1.Scale{}
-	err = c.client.Put().
-		Namespace(c.ns).
-		Resource("deploymentconfigs").
-		Name(deploymentConfigName).
-		SubResource("scale").
-		Body(scale).
-		Do().
-		Into(result)
-	return
-}

pkg/apps/generated/internalclientset/typed/apps/internalversion/deploymentconfig_expansion.go

@@ -0,0 +1,33 @@
+package internalversion
+
+import (
+	"k8s.io/apimachinery/pkg/runtime"
+	kapi "k8s.io/kubernetes/pkg/api"
+	v1beta1 "k8s.io/kubernetes/pkg/apis/extensions/v1beta1"
+)
+
+type DeploymentConfigExpansion interface {
+	UpdateScale(deploymentConfigName string, scale *v1beta1.Scale) (result *v1beta1.Scale, err error)
+}
+
+var scaleCodec = kapi.Codecs.LegacyCodec(v1beta1.SchemeGroupVersion)
+
+// UpdateScale takes the top resource name and the representation of a scale and updates it. Returns the server's representation of the scale, and an error, if there is any.
+func (c *deploymentConfigs) UpdateScale(deploymentConfigName string, scale *v1beta1.Scale) (result *v1beta1.Scale, err error) {
+	// FIXME: make non-homogenous subresource GV client generation work
+	data, err := runtime.Encode(scaleCodec, scale)
+	if err != nil {
+		return nil, err
+	}
+
+	result = &v1beta1.Scale{}
+	err = c.client.Put().
+		Namespace(c.ns).
+		Resource("deploymentconfigs").
+		Name(deploymentConfigName).
+		SubResource("scale").
+		Body(data).
+		Do().
+		Into(result)
+	return
+}

pkg/apps/generated/internalclientset/typed/apps/internalversion/fake/fake_deploymentconfig.go

@@ -154,14 +154,3 @@ func (c *FakeDeploymentConfigs) GetScale(deploymentConfigName string, options v1
 	}
 	return obj.(*v1beta1.Scale), err
 }
-
-// UpdateScale takes the representation of a scale and updates it. Returns the server's representation of the scale, and an error, if there is any.
-func (c *FakeDeploymentConfigs) UpdateScale(deploymentConfigName string, scale *v1beta1.Scale) (result *v1beta1.Scale, err error) {
-	obj, err := c.Fake.
-		Invokes(testing.NewUpdateSubresourceAction(deploymentconfigsResource, "scale", c.ns, scale), &v1beta1.Scale{})
-
-	if obj == nil {
-		return nil, err
-	}
-	return obj.(*v1beta1.Scale), err
-}

pkg/apps/generated/internalclientset/typed/apps/internalversion/fake/fake_deploymentconfig_expansion.go

@@ -0,0 +1,17 @@
+package fake
+
+import (
+	testing "k8s.io/client-go/testing"
+	v1beta1 "k8s.io/kubernetes/pkg/apis/extensions/v1beta1"
+)
+
+// UpdateScale takes the representation of a scale and updates it. Returns the server's representation of the scale, and an error, if t
+func (c *FakeDeploymentConfigs) UpdateScale(deploymentConfigName string, scale *v1beta1.Scale) (result *v1beta1.Scale, err error) {
+	obj, err := c.Fake.
+		Invokes(testing.NewUpdateSubresourceAction(deploymentconfigsResource, "scale", c.ns, scale), &v1beta1.Scale{})
+
+	if obj == nil {
+		return nil, err
+	}
+	return obj.(*v1beta1.Scale), err
+}

pkg/apps/generated/internalclientset/typed/apps/internalversion/generated_expansion.go

@@ -1,3 +1 @@
 package internalversion
-
-type DeploymentConfigExpansion interface{}

test/integration/deploy_scale_test.go

@@ -1,10 +1,12 @@
 package integration
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/util/wait"
 	extensionsv1beta1 "k8s.io/kubernetes/pkg/apis/extensions/v1beta1"
 
@@ -49,6 +51,40 @@ func TestDeployScale(t *testing.T) {
 	}
 	generation := dc.Generation
 
+	{
+		// Get scale subresource
+		legacyPath := fmt.Sprintf("/oapi/v1/namespaces/%s/deploymentconfigs/%s/scale", dc.Namespace, dc.Name)
+		legacyScale := &unstructured.Unstructured{}
+		if err := adminAppsClient.AppsClient.RESTClient().Get().AbsPath(legacyPath).Do().Into(legacyScale); err != nil {
+			t.Fatal(err)
+		}
+		// Ensure correct type
+		if legacyScale.GetAPIVersion() != "extensions/v1beta1" {
+			t.Fatalf("Expected extensions/v1beta1, got %v", legacyScale.GetAPIVersion())
+		}
+		// Ensure we can submit the same type back
+		if err := adminAppsClient.AppsClient.RESTClient().Put().AbsPath(legacyPath).Body(legacyScale).Do().Error(); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	{
+		// Get scale subresource
+		scalePath := fmt.Sprintf("/apis/apps.openshift.io/v1/namespaces/%s/deploymentconfigs/%s/scale", dc.Namespace, dc.Name)
+		scale := &unstructured.Unstructured{}
+		if err := adminAppsClient.AppsClient.RESTClient().Get().AbsPath(scalePath).Do().Into(scale); err != nil {
+			t.Fatal(err)
+		}
+		// Ensure correct type
+		if scale.GetAPIVersion() != "extensions/v1beta1" {
+			t.Fatalf("Expected extensions/v1beta1, got %v", scale.GetAPIVersion())
+		}
+		// Ensure we can submit the same type back
+		if err := adminAppsClient.AppsClient.RESTClient().Put().AbsPath(scalePath).Body(scale).Do().Error(); err != nil {
+			t.Fatal(err)
+		}
+	}
+
 	condition := func() (bool, error) {
 		config, err := adminAppsClient.DeploymentConfigs(namespace).Get(dc.Name, metav1.GetOptions{})
 		if err != nil {