be tolerant on deployment decode and strict on encode

pkg/apps/apiserver/apiserver.go

@@ -4,13 +4,11 @@ import (
 	"sync"
 
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/apiserver/pkg/registry/rest"
 	genericapiserver "k8s.io/apiserver/pkg/server"
 	"k8s.io/client-go/kubernetes"
 	restclient "k8s.io/client-go/rest"
-	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	kclientsetinternal "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 
 	appsapiv1 "github.com/openshift/api/apps/v1"
@@ -99,9 +97,6 @@ func (c *completedConfig) V1RESTStorage() (map[string]rest.Storage, error) {
 }
 
 func (c *completedConfig) newV1RESTStorage() (map[string]rest.Storage, error) {
-	// TODO sort out who is using this and why.  it was hardcoded before the migration and I suspect that it is being used
-	// to serialize out objects into annotations.
-	externalVersionCodec := legacyscheme.Codecs.LegacyCodec(schema.GroupVersion{Group: "", Version: "v1"})
 	openshiftInternalAppsClient, err := appsclientinternal.NewForConfig(c.GenericConfig.LoopbackClientConfig)
 	if err != nil {
 		return nil, err
@@ -128,10 +123,9 @@ func (c *completedConfig) newV1RESTStorage() (map[string]rest.Storage, error) {
 		*deployConfigStorage.Store,
 		openshiftInternalImageClient,
 		kubeInternalClient,
-		externalVersionCodec,
 		c.GenericConfig.AdmissionControl,
 	)
-	deployConfigRollbackStorage := deployrollback.NewREST(openshiftInternalAppsClient, kubeInternalClient, externalVersionCodec)
+	deployConfigRollbackStorage := deployrollback.NewREST(openshiftInternalAppsClient, kubeInternalClient)
 
 	v1Storage := map[string]rest.Storage{}
 	v1Storage["deploymentConfigs"] = deployConfigStorage

pkg/apps/controller/deployer/deployer_controller.go

@@ -91,9 +91,6 @@ type DeploymentController struct {
 	// environment is a set of environment variables which should be injected into all
 	// deployer pod containers.
 	environment []v1.EnvVar
-	// codec is used for deserializing deploymentconfigs from replication controller
-	// annotations.
-	codec runtime.Codec
 	// recorder is used to record events.
 	recorder record.EventRecorder
 }
@@ -136,7 +133,7 @@ func (c *DeploymentController) handle(deployment *v1.ReplicationController, will
 		// deployer pod (quota, etc..) we should respect the timeoutSeconds in the
 		// config strategy and transition the rollout to failed instead of waiting for
 		// the deployment pod forever.
-		config, err := appsutil.DecodeDeploymentConfig(deployment, c.codec)
+		config, err := appsutil.DecodeDeploymentConfig(deployment)
 		if err != nil {
 			return err
 		}
@@ -281,7 +278,7 @@ func (c *DeploymentController) handle(deployment *v1.ReplicationController, will
 		// If we are going to transition to failed or complete and scale is non-zero, we'll check one more
 		// time to see if we are a test deployment to guarantee that we maintain the test invariant.
 		if *deploymentCopy.Spec.Replicas != 0 && appsutil.IsTerminatedDeployment(deploymentCopy) {
-			if config, err := appsutil.DecodeDeploymentConfig(deploymentCopy, c.codec); err == nil && config.Spec.Test {
+			if config, err := appsutil.DecodeDeploymentConfig(deploymentCopy); err == nil && config.Spec.Test {
 				zero := int32(0)
 				deploymentCopy.Spec.Replicas = &zero
 			}
@@ -357,7 +354,7 @@ func nextStatusComp(fromDeployer, fromPath appsapi.DeploymentStatus) appsapi.Dep
 // makeDeployerPod creates a pod which implements deployment behavior. The pod is correlated to
 // the deployment with an annotation.
 func (c *DeploymentController) makeDeployerPod(deployment *v1.ReplicationController) (*v1.Pod, error) {
-	deploymentConfig, err := appsutil.DecodeDeploymentConfig(deployment, c.codec)
+	deploymentConfig, err := appsutil.DecodeDeploymentConfig(deployment)
 	if err != nil {
 		return nil, err
 	}
@@ -557,7 +554,7 @@ func (c *DeploymentController) cleanupDeployerPods(deployment *v1.ReplicationCon
 }
 
 func (c *DeploymentController) emitDeploymentEvent(deployment *v1.ReplicationController, eventType, title, message string) {
-	if config, _ := appsutil.DecodeDeploymentConfig(deployment, c.codec); config != nil {
+	if config, _ := appsutil.DecodeDeploymentConfig(deployment); config != nil {
 		c.recorder.Eventf(config, eventType, title, message)
 	} else {
 		c.recorder.Eventf(deployment, eventType, title, message)

pkg/apps/controller/deployer/deployer_controller_test.go

@@ -27,16 +27,14 @@ import (
 	kapi "k8s.io/kubernetes/pkg/apis/core"
 	kapihelper "k8s.io/kubernetes/pkg/apis/core/helper"
 
-	appsapiv1 "github.com/openshift/api/apps/v1"
 	appsapi "github.com/openshift/origin/pkg/apps/apis/apps"
 	_ "github.com/openshift/origin/pkg/apps/apis/apps/install"
 	appstest "github.com/openshift/origin/pkg/apps/apis/apps/test"
 	appsutil "github.com/openshift/origin/pkg/apps/util"
 )
 
 var (
-	env   = []kapi.EnvVar{{Name: "ENV1", Value: "VAL1"}}
-	codec = legacyscheme.Codecs.LegacyCodec(appsapiv1.SchemeGroupVersion)
+	env = []kapi.EnvVar{{Name: "ENV1", Value: "VAL1"}}
 )
 
 func alwaysReady() bool { return true }
@@ -51,7 +49,7 @@ func okDeploymentController(client kclientset.Interface, deployment *v1.Replicat
 	rcInformer := informerFactory.Core().V1().ReplicationControllers()
 	podInformer := informerFactory.Core().V1().Pods()
 
-	c := NewDeployerController(rcInformer, podInformer, client, "sa:test", "openshift/origin-deployer", env, codec)
+	c := NewDeployerController(rcInformer, podInformer, client, "sa:test", "openshift/origin-deployer", env)
 	c.podListerSynced = alwaysReady
 	c.rcListerSynced = alwaysReady
 
@@ -185,7 +183,7 @@ func TestHandle_createPodOk(t *testing.T) {
 	// Verify new -> pending
 	config := appstest.OkDeploymentConfig(1)
 	config.Spec.Strategy = appstest.OkCustomStrategy()
-	deployment, _ := appsutil.MakeDeploymentV1(config, codec)
+	deployment, _ := appsutil.MakeDeploymentV1(config)
 	deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusNew)
 	deployment.Spec.Template.Spec.NodeSelector = map[string]string{"labelKey1": "labelValue1", "labelKey2": "labelValue2"}
 	deployment.CreationTimestamp = metav1.Now()
@@ -276,7 +274,7 @@ func TestHandle_createPodFail(t *testing.T) {
 	})
 
 	config := appstest.OkDeploymentConfig(1)
-	deployment, _ := appsutil.MakeDeploymentV1(config, codec)
+	deployment, _ := appsutil.MakeDeploymentV1(config)
 	deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusNew)
 	deployment.CreationTimestamp = metav1.Now()
 
@@ -332,7 +330,7 @@ func TestHandle_deployerPodAlreadyExists(t *testing.T) {
 		var updatedDeployment *v1.ReplicationController
 
 		config := appstest.OkDeploymentConfig(1)
-		deployment, _ := appsutil.MakeDeploymentV1(config, codec)
+		deployment, _ := appsutil.MakeDeploymentV1(config)
 		deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusNew)
 		deployment.CreationTimestamp = metav1.Now()
 		deployerPodName := appsutil.DeployerPodNameForDeployment(deployment.Name)
@@ -373,7 +371,7 @@ func TestHandle_unrelatedPodAlreadyExists(t *testing.T) {
 	var updatedDeployment *v1.ReplicationController
 
 	config := appstest.OkDeploymentConfig(1)
-	deployment, _ := appsutil.MakeDeploymentV1(config, codec)
+	deployment, _ := appsutil.MakeDeploymentV1(config)
 	deployment.CreationTimestamp = metav1.Now()
 	deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusNew)
 
@@ -414,7 +412,7 @@ func TestHandle_unrelatedPodAlreadyExistsTestScaled(t *testing.T) {
 	var updatedDeployment *v1.ReplicationController
 
 	config := appstest.TestDeploymentConfig(appstest.OkDeploymentConfig(1))
-	deployment, _ := appsutil.MakeDeploymentV1(config, codec)
+	deployment, _ := appsutil.MakeDeploymentV1(config)
 	deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusNew)
 	deployment.CreationTimestamp = metav1.Now()
 	one := int32(1)
@@ -486,7 +484,7 @@ func TestHandle_noop(t *testing.T) {
 	for _, test := range tests {
 		client := fake.NewSimpleClientset()
 
-		deployment, _ := appsutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1), codec)
+		deployment, _ := appsutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1))
 		deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(test.deploymentPhase)
 		deployment.CreationTimestamp = metav1.Now()
 
@@ -531,7 +529,7 @@ func TestHandle_failedTest(t *testing.T) {
 
 	// Verify successful cleanup
 	config := appstest.TestDeploymentConfig(appstest.OkDeploymentConfig(1))
-	deployment, _ := appsutil.MakeDeploymentV1(config, codec)
+	deployment, _ := appsutil.MakeDeploymentV1(config)
 	deployment.CreationTimestamp = metav1.Now()
 	one := int32(1)
 	deployment.Spec.Replicas = &one
@@ -574,7 +572,7 @@ func TestHandle_cleanupPodOk(t *testing.T) {
 
 	// Verify successful cleanup
 	config := appstest.OkDeploymentConfig(1)
-	deployment, _ := appsutil.MakeDeploymentV1(config, codec)
+	deployment, _ := appsutil.MakeDeploymentV1(config)
 	deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusComplete)
 	deployment.CreationTimestamp = metav1.Now()
 
@@ -619,7 +617,7 @@ func TestHandle_cleanupPodOkTest(t *testing.T) {
 
 	// Verify successful cleanup
 	config := appstest.TestDeploymentConfig(appstest.OkDeploymentConfig(1))
-	deployment, _ := appsutil.MakeDeploymentV1(config, codec)
+	deployment, _ := appsutil.MakeDeploymentV1(config)
 	deployment.CreationTimestamp = metav1.Now()
 	one := int32(1)
 	deployment.Spec.Replicas = &one
@@ -664,7 +662,7 @@ func TestHandle_cleanupPodNoop(t *testing.T) {
 
 	// Verify no-op
 	config := appstest.OkDeploymentConfig(1)
-	deployment, _ := appsutil.MakeDeploymentV1(config, codec)
+	deployment, _ := appsutil.MakeDeploymentV1(config)
 	deployment.CreationTimestamp = metav1.Now()
 	deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusComplete)
 
@@ -696,7 +694,7 @@ func TestHandle_cleanupPodFail(t *testing.T) {
 
 	// Verify error
 	config := appstest.OkDeploymentConfig(1)
-	deployment, _ := appsutil.MakeDeploymentV1(config, codec)
+	deployment, _ := appsutil.MakeDeploymentV1(config)
 	deployment.CreationTimestamp = metav1.Now()
 	deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusComplete)
 
@@ -727,7 +725,7 @@ func TestHandle_cancelNew(t *testing.T) {
 		return true, rc, nil
 	})
 
-	deployment, _ := appsutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1), codec)
+	deployment, _ := appsutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1))
 	deployment.CreationTimestamp = metav1.Now()
 	deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusNew)
 	deployment.Annotations[appsapi.DeploymentCancelledAnnotation] = appsapi.DeploymentCancelledAnnotationValue
@@ -749,7 +747,7 @@ func TestHandle_cleanupNewWithDeployers(t *testing.T) {
 	var updatedDeployment *v1.ReplicationController
 	deletedDeployer := false
 
-	deployment, _ := appsutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1), codec)
+	deployment, _ := appsutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1))
 	deployment.CreationTimestamp = metav1.Now()
 	deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusNew)
 	deployment.Annotations[appsapi.DeploymentCancelledAnnotation] = appsapi.DeploymentCancelledAnnotationValue
@@ -844,7 +842,7 @@ func TestHandle_cleanupPostNew(t *testing.T) {
 			return true, nil, nil
 		})
 
-		deployment, _ := appsutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1), codec)
+		deployment, _ := appsutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1))
 		deployment.CreationTimestamp = metav1.Now()
 		deployment.Annotations[appsapi.DeploymentCancelledAnnotation] = appsapi.DeploymentCancelledAnnotationValue
 		deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(test.deploymentPhase)
@@ -903,7 +901,7 @@ func TestHandle_deployerPodDisappeared(t *testing.T) {
 			return true, nil, nil
 		})
 
-		deployment, err := appsutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1), codec)
+		deployment, err := appsutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1))
 		if err != nil {
 			t.Errorf("%s: unexpected error: %v", test.name, err)
 			continue
@@ -1043,7 +1041,7 @@ func TestHandle_transitionFromDeployer(t *testing.T) {
 			return true, nil, nil
 		})
 
-		deployment, _ := appsutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1), codec)
+		deployment, _ := appsutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1))
 		deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(test.deploymentPhase)
 		deployment.CreationTimestamp = metav1.Now()
 
@@ -1099,7 +1097,7 @@ func TestDeployerCustomLabelsAndAnnotations(t *testing.T) {
 		config.Spec.Strategy = test.strategy
 		config.Spec.Strategy.Labels = test.labels
 		config.Spec.Strategy.Annotations = test.annotations
-		deployment, _ := appsutil.MakeDeploymentV1(config, codec)
+		deployment, _ := appsutil.MakeDeploymentV1(config)
 
 		client := &fake.Clientset{}
 		client.AddReactor("create", "pods", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
@@ -1130,7 +1128,7 @@ func TestMakeDeployerPod(t *testing.T) {
 	client := &fake.Clientset{}
 	controller := okDeploymentController(client, nil, nil, true, v1.PodUnknown)
 	config := appstest.OkDeploymentConfig(1)
-	deployment, _ := appsutil.MakeDeploymentV1(config, codec)
+	deployment, _ := appsutil.MakeDeploymentV1(config)
 	container := controller.makeDeployerContainer(&config.Spec.Strategy)
 	container.Resources = appsutil.CopyApiResourcesToV1Resources(&config.Spec.Strategy.Resources)
 	defaultGracePeriod := int64(10)

pkg/apps/controller/deployer/factory.go

@@ -7,7 +7,6 @@ import (
 	"github.com/golang/glog"
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 	kcoreinformers "k8s.io/client-go/informers/core/v1"
@@ -31,7 +30,6 @@ func NewDeployerController(
 	sa,
 	image string,
 	env []kapi.EnvVar,
-	codec runtime.Codec,
 ) *DeploymentController {
 	eventBroadcaster := record.NewBroadcaster()
 	eventBroadcaster.StartLogging(glog.Infof)
@@ -53,7 +51,6 @@ func NewDeployerController(
 		deployerImage:  image,
 		environment:    appsutil.CopyApiEnvVarToV1EnvVar(env),
 		recorder:       recorder,
-		codec:          codec,
 	}
 
 	rcInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{

pkg/apps/controller/deploymentconfig/deploymentconfig_controller.go

@@ -150,7 +150,7 @@ func (c *DeploymentConfigController) Handle(config *appsapi.DeploymentConfig) er
 
 	configCopy := config.DeepCopy()
 	// Process triggers and start an initial rollouts
-	shouldTrigger, shouldSkip, err := triggerActivated(configCopy, latestExists, latestDeployment, c.codec)
+	shouldTrigger, shouldSkip, err := triggerActivated(configCopy, latestExists, latestDeployment)
 	if err != nil {
 		return fmt.Errorf("triggerActivated failed: %v", err)
 	}
@@ -179,7 +179,7 @@ func (c *DeploymentConfigController) Handle(config *appsapi.DeploymentConfig) er
 
 	// No deployments are running and the latest deployment doesn't exist, so
 	// create the new deployment.
-	deployment, err := appsutil.MakeDeploymentV1(config, c.codec)
+	deployment, err := appsutil.MakeDeploymentV1(config)
 	if err != nil {
 		return fatalError(fmt.Sprintf("couldn't make deployment from (potentially invalid) deployment config %s: %v", appsutil.LabelForDeploymentConfig(config), err))
 	}
@@ -541,7 +541,7 @@ func (c *DeploymentConfigController) cleanupOldDeployments(existingDeployments [
 // triggers were activated (config change or image change). The first bool indicates that
 // the triggers are active and second indicates if we should skip the rollout because we
 // are waiting for the trigger to complete update (waiting for image for example).
-func triggerActivated(config *appsapi.DeploymentConfig, latestExists bool, latestDeployment *v1.ReplicationController, codec runtime.Codec) (bool, bool, error) {
+func triggerActivated(config *appsapi.DeploymentConfig, latestExists bool, latestDeployment *v1.ReplicationController) (bool, bool, error) {
 	if config.Spec.Paused {
 		return false, false, nil
 	}
@@ -598,7 +598,7 @@ func triggerActivated(config *appsapi.DeploymentConfig, latestExists bool, lates
 	}
 
 	if configTrigger {
-		isLatest, changes, err := appsutil.HasLatestPodTemplate(config, latestDeployment, codec)
+		isLatest, changes, err := appsutil.HasLatestPodTemplate(config, latestDeployment)
 		if err != nil {
 			return false, false, fmt.Errorf("error while checking for latest pod template in replication controller: %v", err)
 		}

pkg/apps/controller/deploymentconfig/deploymentconfig_controller_test.go

@@ -17,11 +17,9 @@ import (
 	"k8s.io/client-go/kubernetes/fake"
 	clientgotesting "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/cache"
-	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	kapi "k8s.io/kubernetes/pkg/apis/core"
 	kapihelper "k8s.io/kubernetes/pkg/apis/core/helper"
 
-	appsv1 "github.com/openshift/api/apps/v1"
 	appsapi "github.com/openshift/origin/pkg/apps/apis/apps"
 	_ "github.com/openshift/origin/pkg/apps/apis/apps/install"
 	appstest "github.com/openshift/origin/pkg/apps/apis/apps/test"
@@ -53,7 +51,7 @@ func TestHandleScenarios(t *testing.T) {
 			config = appstest.TestDeploymentConfig(config)
 		}
 		config.Namespace = "test"
-		deployment, _ := appsutil.MakeDeploymentV1(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion))
+		deployment, _ := appsutil.MakeDeploymentV1(config)
 		deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(d.status)
 		if d.cancelled {
 			deployment.Annotations[appsapi.DeploymentCancelledAnnotation] = appsapi.DeploymentCancelledAnnotationValue
@@ -366,7 +364,6 @@ func TestHandleScenarios(t *testing.T) {
 			deployments[rc.Name] = rc
 			return true, rc, nil
 		})
-		codec := legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)
 
 		dcInformer := &fakeDeploymentConfigInformer{
 			informer: cache.NewSharedIndexInformer(
@@ -386,7 +383,7 @@ func TestHandleScenarios(t *testing.T) {
 
 		kubeInformerFactory := kinformers.NewSharedInformerFactory(kc, 0)
 		rcInformer := kubeInformerFactory.Core().V1().ReplicationControllers()
-		c := NewDeploymentConfigController(dcInformer, rcInformer, oc, kc, codec)
+		c := NewDeploymentConfigController(dcInformer, rcInformer, oc, kc)
 		c.dcStoreSynced = alwaysReady
 		c.rcListerSynced = alwaysReady
 

pkg/apps/controller/deploymentconfig/factory.go

@@ -7,7 +7,6 @@ import (
 	"github.com/golang/glog"
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 	kcoreinformers "k8s.io/client-go/informers/core/v1"
@@ -31,7 +30,6 @@ func NewDeploymentConfigController(
 	rcInformer kcoreinformers.ReplicationControllerInformer,
 	appsClientset appsclient.Interface,
 	kubeClientset kclientset.Interface,
-	codec runtime.Codec,
 ) *DeploymentConfigController {
 	eventBroadcaster := record.NewBroadcaster()
 	eventBroadcaster.StartLogging(glog.Infof)
@@ -52,7 +50,6 @@ func NewDeploymentConfigController(
 		},
 
 		recorder: recorder,
-		codec:    codec,
 	}
 
 	c.dcLister = dcInformer.Lister()