From 618a87b97d7a22046f56aeb223acfc1b4f83ab07 Mon Sep 17 00:00:00 2001 From: David Eads Date: Mon, 2 Jul 2018 14:15:26 -0400 Subject: [PATCH 1/2] be tolerant on deployment decode and strict on encode --- pkg/apps/apiserver/apiserver.go | 8 +--- .../deployer/deployer_controller.go | 11 ++--- .../deployer/deployer_controller_test.go | 42 ++++++++-------- pkg/apps/controller/deployer/factory.go | 3 -- .../deploymentconfig_controller.go | 8 ++-- .../deploymentconfig_controller_test.go | 7 +-- .../controller/deploymentconfig/factory.go | 3 -- pkg/apps/registry/deploylog/rest_test.go | 6 ++- pkg/apps/registry/instantiate/rest.go | 22 ++++----- pkg/apps/registry/instantiate/rest_test.go | 8 +--- pkg/apps/registry/rollback/rest.go | 6 +-- pkg/apps/registry/rollback/rest_test.go | 25 ++++------ pkg/apps/strategy/recreate/recreate.go | 11 ++--- pkg/apps/strategy/recreate/recreate_test.go | 35 +++++--------- pkg/apps/strategy/rolling/rolling.go | 14 ++---- pkg/apps/strategy/rolling/rolling_test.go | 18 +++---- pkg/apps/strategy/support/lifecycle.go | 25 +++++----- pkg/apps/strategy/support/lifecycle_test.go | 27 +++++------ pkg/apps/strategy/util/util.go | 8 ++-- pkg/apps/util/scheme.go | 48 +++++++++++++++++++ pkg/apps/util/scheme_test.go | 47 ++++++++++++++++++ pkg/apps/util/util.go | 38 +++++---------- pkg/apps/util/util_test.go | 12 ++--- pkg/cmd/infra/deployer/deployer.go | 9 ++-- pkg/cmd/infra/deployer/deployer_test.go | 4 +- .../controller/apps.go | 10 ---- pkg/oc/cli/cmd/deploy_test.go | 5 +- pkg/oc/cli/cmd/rollback_test.go | 3 +- pkg/oc/cli/describe/deployments_test.go | 3 +- test/integration/imagestream_test.go | 7 ++- 30 files changed, 237 insertions(+), 236 deletions(-) create mode 100644 pkg/apps/util/scheme.go create mode 100644 pkg/apps/util/scheme_test.go diff --git a/pkg/apps/apiserver/apiserver.go b/pkg/apps/apiserver/apiserver.go index 86198015d87f..8512676688ff 100644 --- a/pkg/apps/apiserver/apiserver.go +++ b/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 diff --git a/pkg/apps/controller/deployer/deployer_controller.go b/pkg/apps/controller/deployer/deployer_controller.go index 5701dea88ec1..4fec23423400 100755 --- a/pkg/apps/controller/deployer/deployer_controller.go +++ b/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) diff --git a/pkg/apps/controller/deployer/deployer_controller_test.go b/pkg/apps/controller/deployer/deployer_controller_test.go index 878ee9bcd90b..2956ef028d16 100644 --- a/pkg/apps/controller/deployer/deployer_controller_test.go +++ b/pkg/apps/controller/deployer/deployer_controller_test.go @@ -27,7 +27,6 @@ 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" @@ -35,8 +34,7 @@ import ( ) 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) diff --git a/pkg/apps/controller/deployer/factory.go b/pkg/apps/controller/deployer/factory.go index b19d531ac6df..528116f69648 100644 --- a/pkg/apps/controller/deployer/factory.go +++ b/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{ diff --git a/pkg/apps/controller/deploymentconfig/deploymentconfig_controller.go b/pkg/apps/controller/deploymentconfig/deploymentconfig_controller.go index 8eae0738db08..942dbcb19556 100644 --- a/pkg/apps/controller/deploymentconfig/deploymentconfig_controller.go +++ b/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) } diff --git a/pkg/apps/controller/deploymentconfig/deploymentconfig_controller_test.go b/pkg/apps/controller/deploymentconfig/deploymentconfig_controller_test.go index 0d0f11ed9f42..4e476149cc69 100644 --- a/pkg/apps/controller/deploymentconfig/deploymentconfig_controller_test.go +++ b/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 diff --git a/pkg/apps/controller/deploymentconfig/factory.go b/pkg/apps/controller/deploymentconfig/factory.go index 48c478372ab3..14b409b26dda 100644 --- a/pkg/apps/controller/deploymentconfig/factory.go +++ b/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() diff --git a/pkg/apps/registry/deploylog/rest_test.go b/pkg/apps/registry/deploylog/rest_test.go index d76b8fad92a7..7b9de348470b 100644 --- a/pkg/apps/registry/deploylog/rest_test.go +++ b/pkg/apps/registry/deploylog/rest_test.go @@ -12,7 +12,6 @@ import ( apirequest "k8s.io/apiserver/pkg/endpoints/request" fakeexternal "k8s.io/client-go/kubernetes/fake" clientgotesting "k8s.io/client-go/testing" - "k8s.io/kubernetes/pkg/api/legacyscheme" kapi "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" @@ -28,7 +27,10 @@ import ( var testSelector = map[string]string{"test": "rest"} func makeDeployment(version int64) kapi.ReplicationController { - deployment, _ := appsutil.MakeDeployment(appstest.OkDeploymentConfig(version), legacyscheme.Codecs.LegacyCodec(appsapi.SchemeGroupVersion)) + deployment, err := appsutil.MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(version)) + if err != nil { + panic(err) + } deployment.Namespace = metav1.NamespaceDefault deployment.Spec.Selector = testSelector return *deployment diff --git a/pkg/apps/registry/instantiate/rest.go b/pkg/apps/registry/instantiate/rest.go index a269259ac953..6b5df0b2fba4 100644 --- a/pkg/apps/registry/instantiate/rest.go +++ b/pkg/apps/registry/instantiate/rest.go @@ -30,20 +30,19 @@ import ( ) // NewREST provides new REST storage for the apps API group. -func NewREST(store registry.Store, imagesclient imageclientinternal.Interface, kc kclientset.Interface, decoder runtime.Decoder, admission admission.Interface) *REST { +func NewREST(store registry.Store, imagesclient imageclientinternal.Interface, kc kclientset.Interface, admission admission.Interface) *REST { store.UpdateStrategy = Strategy - return &REST{store: &store, is: imagesclient.Image(), rn: kc.Core(), decoder: decoder, admit: admission} + return &REST{store: &store, is: imagesclient.Image(), rn: kc.Core(), admit: admission} } // REST implements the Creater interface. var _ = rest.Creater(&REST{}) type REST struct { - store *registry.Store - is images.ImageStreamsGetter - rn kcoreclient.ReplicationControllersGetter - decoder runtime.Decoder - admit admission.Interface + store *registry.Store + is images.ImageStreamsGetter + rn kcoreclient.ReplicationControllersGetter + admit admission.Interface } func (s *REST) New() runtime.Object { @@ -77,7 +76,7 @@ func (s *REST) Create(ctx context.Context, obj runtime.Object, createValidation } } - canTrigger, causes, err := canTrigger(config, s.rn, s.decoder, req.Force) + canTrigger, causes, err := canTrigger(config, s.rn, req.Force) if err != nil { return err } @@ -217,11 +216,10 @@ func containsTriggerType(types []appsapi.DeploymentTriggerType, triggerType apps func canTrigger( config *appsapi.DeploymentConfig, rn kcoreclient.ReplicationControllersGetter, - decoder runtime.Decoder, force bool, ) (bool, []appsapi.DeploymentCause, error) { - decoded, err := decodeFromLatestDeployment(config, rn, decoder) + decoded, err := decodeFromLatestDeployment(config, rn) if err != nil { return false, nil, err } @@ -297,7 +295,7 @@ func canTrigger( // decodeFromLatestDeployment will try to return the decoded version of the current deploymentconfig // found in the annotations of its latest deployment. If there is no previous deploymentconfig (ie. // latestVersion == 0), the returned deploymentconfig will be the same. -func decodeFromLatestDeployment(config *appsapi.DeploymentConfig, rn kcoreclient.ReplicationControllersGetter, decoder runtime.Decoder) (*appsapi.DeploymentConfig, error) { +func decodeFromLatestDeployment(config *appsapi.DeploymentConfig, rn kcoreclient.ReplicationControllersGetter) (*appsapi.DeploymentConfig, error) { if config.Status.LatestVersion == 0 { return config, nil } @@ -310,7 +308,7 @@ func decodeFromLatestDeployment(config *appsapi.DeploymentConfig, rn kcoreclient // to make the deployment for the config, so return early. return nil, err } - decoded, err := appsutil.DecodeDeploymentConfig(deployment, decoder) + decoded, err := appsutil.DecodeDeploymentConfig(deployment) if err != nil { return nil, errors.NewInternalError(err) } diff --git a/pkg/apps/registry/instantiate/rest_test.go b/pkg/apps/registry/instantiate/rest_test.go index fca645283004..aa42bc418513 100644 --- a/pkg/apps/registry/instantiate/rest_test.go +++ b/pkg/apps/registry/instantiate/rest_test.go @@ -7,12 +7,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" clientgotesting "k8s.io/client-go/testing" - "k8s.io/kubernetes/pkg/api/legacyscheme" kapi "k8s.io/kubernetes/pkg/apis/core" kapihelper "k8s.io/kubernetes/pkg/apis/core/helper" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" - 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" @@ -21,8 +19,6 @@ import ( imagefake "github.com/openshift/origin/pkg/image/generated/internalclientset/fake" ) -var codec = legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion) - // TestProcess_changeForNonAutomaticTag ensures that an image update for which // there is a matching trigger results in a no-op due to the trigger's // automatic flag being set to false or updates the config if forced. @@ -707,13 +703,13 @@ func TestCanTrigger(t *testing.T) { config = test.config } config = appstest.RoundTripConfig(t, config) - deployment, _ := appsutil.MakeDeployment(config, codec) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) return true, deployment, nil }) test.config = appstest.RoundTripConfig(t, test.config) - got, gotCauses, err := canTrigger(test.config, client.Core(), codec, test.force) + got, gotCauses, err := canTrigger(test.config, client.Core(), test.force) if err != nil && !test.expectedErr { t.Errorf("unexpected error: %v", err) continue diff --git a/pkg/apps/registry/rollback/rest.go b/pkg/apps/registry/rollback/rest.go index 72bd2786334a..917c5ff7b1e3 100644 --- a/pkg/apps/registry/rollback/rest.go +++ b/pkg/apps/registry/rollback/rest.go @@ -25,18 +25,16 @@ type REST struct { generator RollbackGenerator dn apps.DeploymentConfigsGetter rn kcoreclient.ReplicationControllersGetter - codec runtime.Codec } var _ rest.Creater = &REST{} // NewREST safely creates a new REST. -func NewREST(appsclient appsclientinternal.Interface, kc kclientset.Interface, codec runtime.Codec) *REST { +func NewREST(appsclient appsclientinternal.Interface, kc kclientset.Interface) *REST { return &REST{ generator: NewRollbackGenerator(), dn: appsclient.Apps(), rn: kc.Core(), - codec: codec, } } @@ -89,7 +87,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation return nil, newInvalidError(rollback, err.Error()) } - to, err := appsutil.DecodeDeploymentConfig(targetDeployment, r.codec) + to, err := appsutil.DecodeDeploymentConfig(targetDeployment) if err != nil { return nil, newInvalidError(rollback, fmt.Sprintf("couldn't decode deployment config from deployment: %v", err)) } diff --git a/pkg/apps/registry/rollback/rest_test.go b/pkg/apps/registry/rollback/rest_test.go index 23e201465edc..41f689bf890a 100644 --- a/pkg/apps/registry/rollback/rest_test.go +++ b/pkg/apps/registry/rollback/rest_test.go @@ -10,11 +10,9 @@ import ( apirequest "k8s.io/apiserver/pkg/endpoints/request" apiserverrest "k8s.io/apiserver/pkg/registry/rest" clientgotesting "k8s.io/client-go/testing" - "k8s.io/kubernetes/pkg/api/legacyscheme" kapi "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" - 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" @@ -22,8 +20,6 @@ import ( appsutil "github.com/openshift/origin/pkg/apps/util" ) -var codec = legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion) - type terribleGenerator struct{} func (tg *terribleGenerator) GenerateRollback(from, to *appsapi.DeploymentConfig, spec *appsapi.DeploymentConfigRollbackSpec) (*appsapi.DeploymentConfig, error) { @@ -65,11 +61,11 @@ func TestCreateOk(t *testing.T) { }) kc := &fake.Clientset{} kc.AddReactor("get", "replicationcontrollers", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { - deployment, _ := appsutil.MakeDeployment(appstest.OkDeploymentConfig(1), codec) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(1)) return true, deployment, nil }) - obj, err := NewREST(oc, kc, codec).Create(apirequest.NewDefaultContext(), &appsapi.DeploymentConfigRollback{ + obj, err := NewREST(oc, kc).Create(apirequest.NewDefaultContext(), &appsapi.DeploymentConfigRollback{ Name: "config", Spec: appsapi.DeploymentConfigRollbackSpec{ Revision: 1, @@ -95,7 +91,7 @@ func TestCreateRollbackToLatest(t *testing.T) { return true, appstest.OkDeploymentConfig(2), nil }) - _, err := NewREST(oc, &fake.Clientset{}, codec).Create(apirequest.NewDefaultContext(), &appsapi.DeploymentConfigRollback{ + _, err := NewREST(oc, &fake.Clientset{}).Create(apirequest.NewDefaultContext(), &appsapi.DeploymentConfigRollback{ Name: "config", Spec: appsapi.DeploymentConfigRollbackSpec{ Revision: 2, @@ -117,7 +113,7 @@ func TestCreateGeneratorError(t *testing.T) { }) kc := &fake.Clientset{} kc.AddReactor("get", "replicationcontrollers", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { - deployment, _ := appsutil.MakeDeployment(appstest.OkDeploymentConfig(1), codec) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(1)) return true, deployment, nil }) @@ -125,7 +121,6 @@ func TestCreateGeneratorError(t *testing.T) { generator: &terribleGenerator{}, dn: oc.Apps(), rn: kc.Core(), - codec: legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion), } _, err := rest.Create(apirequest.NewDefaultContext(), &appsapi.DeploymentConfigRollback{ @@ -147,11 +142,11 @@ func TestCreateMissingDeployment(t *testing.T) { }) kc := &fake.Clientset{} kc.AddReactor("get", "replicationcontrollers", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { - deployment, _ := appsutil.MakeDeployment(appstest.OkDeploymentConfig(1), codec) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(1)) return true, nil, kerrors.NewNotFound(kapi.Resource("replicationController"), deployment.Name) }) - obj, err := NewREST(oc, kc, codec).Create(apirequest.NewDefaultContext(), &appsapi.DeploymentConfigRollback{ + obj, err := NewREST(oc, kc).Create(apirequest.NewDefaultContext(), &appsapi.DeploymentConfigRollback{ Name: "config", Spec: appsapi.DeploymentConfigRollbackSpec{ Revision: 1, @@ -175,12 +170,12 @@ func TestCreateInvalidDeployment(t *testing.T) { kc := &fake.Clientset{} kc.AddReactor("get", "replicationcontrollers", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { // invalidate the encoded config - deployment, _ := appsutil.MakeDeployment(appstest.OkDeploymentConfig(1), codec) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(1)) deployment.Annotations[appsapi.DeploymentEncodedConfigAnnotation] = "" return true, deployment, nil }) - obj, err := NewREST(oc, kc, codec).Create(apirequest.NewDefaultContext(), &appsapi.DeploymentConfigRollback{ + obj, err := NewREST(oc, kc).Create(apirequest.NewDefaultContext(), &appsapi.DeploymentConfigRollback{ Name: "config", Spec: appsapi.DeploymentConfigRollbackSpec{ Revision: 1, @@ -204,11 +199,11 @@ func TestCreateMissingDeploymentConfig(t *testing.T) { }) kc := &fake.Clientset{} kc.AddReactor("get", "replicationcontrollers", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { - deployment, _ := appsutil.MakeDeployment(appstest.OkDeploymentConfig(1), codec) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(1)) return true, deployment, nil }) - obj, err := NewREST(oc, kc, codec).Create(apirequest.NewDefaultContext(), &appsapi.DeploymentConfigRollback{ + obj, err := NewREST(oc, kc).Create(apirequest.NewDefaultContext(), &appsapi.DeploymentConfigRollback{ Name: "config", Spec: appsapi.DeploymentConfigRollbackSpec{ Revision: 1, diff --git a/pkg/apps/strategy/recreate/recreate.go b/pkg/apps/strategy/recreate/recreate.go index fb12a6117e7a..37e158207dbd 100644 --- a/pkg/apps/strategy/recreate/recreate.go +++ b/pkg/apps/strategy/recreate/recreate.go @@ -61,7 +61,7 @@ type RecreateDeploymentStrategy struct { // NewRecreateDeploymentStrategy makes a RecreateDeploymentStrategy backed by // a real HookExecutor and client. func NewRecreateDeploymentStrategy(client kclientset.Interface, tagClient imageclient.ImageStreamTagsGetter, - events record.EventSink, decoder runtime.Decoder, out, errOut io.Writer, until string) *RecreateDeploymentStrategy { + events record.EventSink, out, errOut io.Writer, until string) *RecreateDeploymentStrategy { if out == nil { out = ioutil.Discard } @@ -81,8 +81,7 @@ func NewRecreateDeploymentStrategy(client kclientset.Interface, tagClient imagec getUpdateAcceptor: func(timeout time.Duration, minReadySeconds int32) strat.UpdateAcceptor { return stratsupport.NewAcceptAvailablePods(out, client.Core(), timeout) }, - decoder: decoder, - hookExecutor: stratsupport.NewHookExecutor(client.Core(), tagClient, client.Core(), os.Stdout, decoder), + hookExecutor: stratsupport.NewHookExecutor(client.Core(), tagClient, client.Core(), os.Stdout), } } @@ -99,7 +98,7 @@ func (s *RecreateDeploymentStrategy) Deploy(from *kapi.ReplicationController, to // This is currently only used in conjunction with the rolling update strategy // for initial deployments. func (s *RecreateDeploymentStrategy) DeployWithAcceptor(from *kapi.ReplicationController, to *kapi.ReplicationController, desiredReplicas int, updateAcceptor strat.UpdateAcceptor) error { - config, err := appsutil.DecodeDeploymentConfig(to, s.decoder) + config, err := appsutil.DecodeDeploymentConfig(to) if err != nil { return fmt.Errorf("couldn't decode config from deployment %s: %v", to.Name, err) } @@ -134,8 +133,8 @@ func (s *RecreateDeploymentStrategy) DeployWithAcceptor(from *kapi.ReplicationCo } // Record all warnings - defer stratutil.RecordConfigWarnings(s.eventClient, from, s.decoder, s.out) - defer stratutil.RecordConfigWarnings(s.eventClient, to, s.decoder, s.out) + defer stratutil.RecordConfigWarnings(s.eventClient, from, s.out) + defer stratutil.RecordConfigWarnings(s.eventClient, to, s.out) // Scale down the from deployment. if from != nil { diff --git a/pkg/apps/strategy/recreate/recreate_test.go b/pkg/apps/strategy/recreate/recreate_test.go index cd56895e9f5a..925db2fc2cd7 100644 --- a/pkg/apps/strategy/recreate/recreate_test.go +++ b/pkg/apps/strategy/recreate/recreate_test.go @@ -10,12 +10,10 @@ import ( "k8s.io/apimachinery/pkg/runtime" scalefake "k8s.io/client-go/scale/fake" clientgotesting "k8s.io/client-go/testing" - "k8s.io/kubernetes/pkg/api/legacyscheme" kapi "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" kcoreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion" - appsv1 "github.com/openshift/api/apps/v1" appsapi "github.com/openshift/origin/pkg/apps/apis/apps" appstest "github.com/openshift/origin/pkg/apps/apis/apps/test" "github.com/openshift/origin/pkg/apps/strategy" @@ -136,14 +134,13 @@ func TestRecreate_initialDeployment(t *testing.T) { strategy := &RecreateDeploymentStrategy{ out: &bytes.Buffer{}, errOut: &bytes.Buffer{}, - decoder: legacyscheme.Codecs.UniversalDecoder(), getUpdateAcceptor: getUpdateAcceptor, eventClient: fake.NewSimpleClientset().Core(), } config := appstest.OkDeploymentConfig(1) config.Spec.Strategy = recreateParams(30, "", "", "") - deployment, _ = appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ = appsutil.MakeTestOnlyInternalDeployment(config) controllerClient := newFakeControllerClient(deployment) strategy.rcClient = controllerClient @@ -163,14 +160,13 @@ func TestRecreate_initialDeployment(t *testing.T) { func TestRecreate_deploymentPreHookSuccess(t *testing.T) { config := appstest.OkDeploymentConfig(1) config.Spec.Strategy = recreateParams(30, appsapi.LifecycleHookFailurePolicyAbort, "", "") - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) controllerClient := newFakeControllerClient(deployment) hookExecuted := false strategy := &RecreateDeploymentStrategy{ out: &bytes.Buffer{}, errOut: &bytes.Buffer{}, - decoder: legacyscheme.Codecs.UniversalDecoder(), getUpdateAcceptor: getUpdateAcceptor, eventClient: fake.NewSimpleClientset().Core(), rcClient: controllerClient, @@ -196,13 +192,12 @@ func TestRecreate_deploymentPreHookSuccess(t *testing.T) { func TestRecreate_deploymentPreHookFail(t *testing.T) { config := appstest.OkDeploymentConfig(1) config.Spec.Strategy = recreateParams(30, appsapi.LifecycleHookFailurePolicyAbort, "", "") - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) controllerClient := newFakeControllerClient(deployment) strategy := &RecreateDeploymentStrategy{ out: &bytes.Buffer{}, errOut: &bytes.Buffer{}, - decoder: legacyscheme.Codecs.UniversalDecoder(), getUpdateAcceptor: getUpdateAcceptor, eventClient: fake.NewSimpleClientset().Core(), rcClient: controllerClient, @@ -228,13 +223,12 @@ func TestRecreate_deploymentPreHookFail(t *testing.T) { func TestRecreate_deploymentMidHookSuccess(t *testing.T) { config := appstest.OkDeploymentConfig(1) config.Spec.Strategy = recreateParams(30, "", appsapi.LifecycleHookFailurePolicyAbort, "") - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) controllerClient := newFakeControllerClient(deployment) strategy := &RecreateDeploymentStrategy{ out: &bytes.Buffer{}, errOut: &bytes.Buffer{}, - decoder: legacyscheme.Codecs.UniversalDecoder(), rcClient: controllerClient, scaleClient: controllerClient.fakeScaleClient(), eventClient: fake.NewSimpleClientset().Core(), @@ -260,14 +254,13 @@ func TestRecreate_deploymentMidHookSuccess(t *testing.T) { func TestRecreate_deploymentPostHookSuccess(t *testing.T) { config := appstest.OkDeploymentConfig(1) config.Spec.Strategy = recreateParams(30, "", "", appsapi.LifecycleHookFailurePolicyAbort) - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) controllerClient := newFakeControllerClient(deployment) hookExecuted := false strategy := &RecreateDeploymentStrategy{ out: &bytes.Buffer{}, errOut: &bytes.Buffer{}, - decoder: legacyscheme.Codecs.UniversalDecoder(), rcClient: controllerClient, scaleClient: controllerClient.fakeScaleClient(), eventClient: fake.NewSimpleClientset().Core(), @@ -293,14 +286,13 @@ func TestRecreate_deploymentPostHookSuccess(t *testing.T) { func TestRecreate_deploymentPostHookFail(t *testing.T) { config := appstest.OkDeploymentConfig(1) config.Spec.Strategy = recreateParams(30, "", "", appsapi.LifecycleHookFailurePolicyAbort) - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) controllerClient := newFakeControllerClient(deployment) hookExecuted := false strategy := &RecreateDeploymentStrategy{ out: &bytes.Buffer{}, errOut: &bytes.Buffer{}, - decoder: legacyscheme.Codecs.UniversalDecoder(), rcClient: controllerClient, scaleClient: controllerClient.fakeScaleClient(), eventClient: fake.NewSimpleClientset().Core(), @@ -329,7 +321,6 @@ func TestRecreate_acceptorSuccess(t *testing.T) { out: &bytes.Buffer{}, errOut: &bytes.Buffer{}, eventClient: fake.NewSimpleClientset().Core(), - decoder: legacyscheme.Codecs.UniversalDecoder(), } acceptorCalled := false @@ -340,8 +331,8 @@ func TestRecreate_acceptorSuccess(t *testing.T) { }, } - oldDeployment, _ := appsutil.MakeDeployment(appstest.OkDeploymentConfig(1), legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) - deployment, _ = appsutil.MakeDeployment(appstest.OkDeploymentConfig(2), legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + oldDeployment, _ := appsutil.MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(1)) + deployment, _ = appsutil.MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(2)) controllerClient := newFakeControllerClient(deployment) strategy.rcClient = controllerClient strategy.scaleClient = controllerClient.fakeScaleClient() @@ -374,7 +365,6 @@ func TestRecreate_acceptorSuccessWithColdCaches(t *testing.T) { out: &bytes.Buffer{}, errOut: &bytes.Buffer{}, eventClient: fake.NewSimpleClientset().Core(), - decoder: legacyscheme.Codecs.UniversalDecoder(), } acceptorCalled := false @@ -385,8 +375,8 @@ func TestRecreate_acceptorSuccessWithColdCaches(t *testing.T) { }, } - oldDeployment, _ := appsutil.MakeDeployment(appstest.OkDeploymentConfig(1), legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) - deployment, _ = appsutil.MakeDeployment(appstest.OkDeploymentConfig(2), legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + oldDeployment, _ := appsutil.MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(1)) + deployment, _ = appsutil.MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(2)) controllerClient := newFakeControllerClient(deployment) strategy.rcClient = controllerClient @@ -419,7 +409,6 @@ func TestRecreate_acceptorFail(t *testing.T) { strategy := &RecreateDeploymentStrategy{ out: &bytes.Buffer{}, errOut: &bytes.Buffer{}, - decoder: legacyscheme.Codecs.UniversalDecoder(), eventClient: fake.NewSimpleClientset().Core(), } @@ -429,8 +418,8 @@ func TestRecreate_acceptorFail(t *testing.T) { }, } - oldDeployment, _ := appsutil.MakeDeployment(appstest.OkDeploymentConfig(1), legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) - deployment, _ = appsutil.MakeDeployment(appstest.OkDeploymentConfig(2), legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + oldDeployment, _ := appsutil.MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(1)) + deployment, _ = appsutil.MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(2)) rcClient := newFakeControllerClient(deployment) strategy.rcClient = rcClient strategy.scaleClient = rcClient.fakeScaleClient() diff --git a/pkg/apps/strategy/rolling/rolling.go b/pkg/apps/strategy/rolling/rolling.go index d73c2f4ae377..c57ca25aa275 100644 --- a/pkg/apps/strategy/rolling/rolling.go +++ b/pkg/apps/strategy/rolling/rolling.go @@ -10,7 +10,6 @@ import ( kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/wait" kapi "k8s.io/kubernetes/pkg/apis/core" kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" @@ -63,8 +62,6 @@ type RollingDeploymentStrategy struct { tags imageclient.ImageStreamTagsGetter // rollingUpdate knows how to perform a rolling update. rollingUpdate func(config *kubectl.RollingUpdaterConfig) error - // decoder is used to access the encoded config on a deployment. - decoder runtime.Decoder // hookExecutor can execute a lifecycle hook. hookExecutor stratsupport.HookExecutor // getUpdateAcceptor returns an UpdateAcceptor to verify the first replica @@ -86,7 +83,7 @@ type acceptingDeploymentStrategy interface { } // NewRollingDeploymentStrategy makes a new RollingDeploymentStrategy. -func NewRollingDeploymentStrategy(namespace string, client kclientset.Interface, tags imageclient.ImageStreamTagsGetter, decoder runtime.Decoder, initialStrategy acceptingDeploymentStrategy, out, errOut io.Writer, until string) *RollingDeploymentStrategy { +func NewRollingDeploymentStrategy(namespace string, client kclientset.Interface, tags imageclient.ImageStreamTagsGetter, initialStrategy acceptingDeploymentStrategy, out, errOut io.Writer, until string) *RollingDeploymentStrategy { if out == nil { out = ioutil.Discard } @@ -98,7 +95,6 @@ func NewRollingDeploymentStrategy(namespace string, client kclientset.Interface, out: out, errOut: errOut, until: until, - decoder: decoder, initialStrategy: initialStrategy, rcClient: client.Core(), eventClient: client.Core(), @@ -109,7 +105,7 @@ func NewRollingDeploymentStrategy(namespace string, client kclientset.Interface, updater := kubectl.NewRollingUpdater(namespace, client.Core(), client.Core(), appsutil.NewReplicationControllerV1ScaleClient(client)) return updater.Update(config) }, - hookExecutor: stratsupport.NewHookExecutor(client.Core(), tags, client.Core(), os.Stdout, decoder), + hookExecutor: stratsupport.NewHookExecutor(client.Core(), tags, client.Core(), os.Stdout), getUpdateAcceptor: func(timeout time.Duration, minReadySeconds int32) strat.UpdateAcceptor { return stratsupport.NewAcceptAvailablePods(out, client.Core(), timeout) }, @@ -117,7 +113,7 @@ func NewRollingDeploymentStrategy(namespace string, client kclientset.Interface, } func (s *RollingDeploymentStrategy) Deploy(from *kapi.ReplicationController, to *kapi.ReplicationController, desiredReplicas int) error { - config, err := appsutil.DecodeDeploymentConfig(to, s.decoder) + config, err := appsutil.DecodeDeploymentConfig(to) if err != nil { return fmt.Errorf("couldn't decode DeploymentConfig from deployment %s: %v", appsutil.LabelForDeployment(to), err) } @@ -156,8 +152,8 @@ func (s *RollingDeploymentStrategy) Deploy(from *kapi.ReplicationController, to } // Record all warnings - defer stratutil.RecordConfigWarnings(s.eventClient, from, s.decoder, s.out) - defer stratutil.RecordConfigWarnings(s.eventClient, to, s.decoder, s.out) + defer stratutil.RecordConfigWarnings(s.eventClient, from, s.out) + defer stratutil.RecordConfigWarnings(s.eventClient, to, s.out) // Prepare for a rolling update. // Execute any pre-hook. diff --git a/pkg/apps/strategy/rolling/rolling_test.go b/pkg/apps/strategy/rolling/rolling_test.go index cc7125f71fa7..0306a14cff58 100644 --- a/pkg/apps/strategy/rolling/rolling_test.go +++ b/pkg/apps/strategy/rolling/rolling_test.go @@ -8,12 +8,10 @@ import ( "k8s.io/apimachinery/pkg/runtime" clientgotesting "k8s.io/client-go/testing" - "k8s.io/kubernetes/pkg/api/legacyscheme" kapi "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" "k8s.io/kubernetes/pkg/kubectl" - appsv1 "github.com/openshift/api/apps/v1" appsapi "github.com/openshift/origin/pkg/apps/apis/apps" appstest "github.com/openshift/origin/pkg/apps/apis/apps/test" strat "github.com/openshift/origin/pkg/apps/strategy" @@ -26,7 +24,6 @@ func TestRolling_deployInitial(t *testing.T) { initialStrategyInvoked := false strategy := &RollingDeploymentStrategy{ - decoder: legacyscheme.Codecs.UniversalDecoder(), rcClient: fake.NewSimpleClientset().Core(), eventClient: fake.NewSimpleClientset().Core(), initialStrategy: &testStrategy{ @@ -46,7 +43,7 @@ func TestRolling_deployInitial(t *testing.T) { config := appstest.OkDeploymentConfig(1) config.Spec.Strategy = appstest.OkRollingStrategy() - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) strategy.out, strategy.errOut = &bytes.Buffer{}, &bytes.Buffer{} err := strategy.Deploy(nil, deployment, 2) if err != nil { @@ -60,10 +57,10 @@ func TestRolling_deployInitial(t *testing.T) { func TestRolling_deployRolling(t *testing.T) { latestConfig := appstest.OkDeploymentConfig(1) latestConfig.Spec.Strategy = appstest.OkRollingStrategy() - latest, _ := appsutil.MakeDeployment(latestConfig, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + latest, _ := appsutil.MakeTestOnlyInternalDeployment(latestConfig) config := appstest.OkDeploymentConfig(2) config.Spec.Strategy = appstest.OkRollingStrategy() - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) deployments := map[string]*kapi.ReplicationController{ latest.Name: latest, @@ -84,7 +81,6 @@ func TestRolling_deployRolling(t *testing.T) { var rollingConfig *kubectl.RollingUpdaterConfig strategy := &RollingDeploymentStrategy{ - decoder: legacyscheme.Codecs.UniversalDecoder(), rcClient: client.Core(), eventClient: fake.NewSimpleClientset().Core(), initialStrategy: &testStrategy{ @@ -157,7 +153,7 @@ func (h *hookExecutorImpl) Execute(hook *appsapi.LifecycleHook, rc *kapi.Replica func TestRolling_deployRollingHooks(t *testing.T) { config := appstest.OkDeploymentConfig(1) config.Spec.Strategy = appstest.OkRollingStrategy() - latest, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + latest, _ := appsutil.MakeTestOnlyInternalDeployment(config) var hookError error @@ -174,7 +170,6 @@ func TestRolling_deployRollingHooks(t *testing.T) { }) strategy := &RollingDeploymentStrategy{ - decoder: legacyscheme.Codecs.UniversalDecoder(), rcClient: client.Core(), eventClient: fake.NewSimpleClientset().Core(), initialStrategy: &testStrategy{ @@ -210,7 +205,7 @@ func TestRolling_deployRollingHooks(t *testing.T) { for _, tc := range cases { config := appstest.OkDeploymentConfig(2) config.Spec.Strategy.RollingParams = tc.params - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) deployments[deployment.Name] = deployment hookError = nil if tc.hookShouldFail { @@ -236,7 +231,6 @@ func TestRolling_deployInitialHooks(t *testing.T) { var hookError error strategy := &RollingDeploymentStrategy{ - decoder: legacyscheme.Codecs.UniversalDecoder(), rcClient: fake.NewSimpleClientset().Core(), eventClient: fake.NewSimpleClientset().Core(), initialStrategy: &testStrategy{ @@ -271,7 +265,7 @@ func TestRolling_deployInitialHooks(t *testing.T) { for i, tc := range cases { config := appstest.OkDeploymentConfig(2) config.Spec.Strategy.RollingParams = tc.params - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) hookError = nil if tc.hookShouldFail { hookError = fmt.Errorf("hook failure") diff --git a/pkg/apps/strategy/support/lifecycle.go b/pkg/apps/strategy/support/lifecycle.go index 7052ed2519fc..aee193e03d40 100644 --- a/pkg/apps/strategy/support/lifecycle.go +++ b/pkg/apps/strategy/support/lifecycle.go @@ -48,8 +48,6 @@ type hookExecutor struct { tags imageclient.ImageStreamTagsGetter // out is where hook pod logs should be written to. out io.Writer - // decoder is used for encoding/decoding. - decoder runtime.Decoder // recorder is used to emit events from hooks events kcoreclient.EventsGetter // getPodLogs knows how to get logs from a pod and is used for testing @@ -57,13 +55,12 @@ type hookExecutor struct { } // NewHookExecutor makes a HookExecutor from a client. -func NewHookExecutor(pods kcoreclient.PodsGetter, tags imageclient.ImageStreamTagsGetter, events kcoreclient.EventsGetter, out io.Writer, decoder runtime.Decoder) HookExecutor { +func NewHookExecutor(pods kcoreclient.PodsGetter, tags imageclient.ImageStreamTagsGetter, events kcoreclient.EventsGetter, out io.Writer) HookExecutor { executor := &hookExecutor{ - tags: tags, - pods: pods, - events: events, - out: out, - decoder: decoder, + tags: tags, + pods: pods, + events: events, + out: out, } executor.getPodLogs = func(pod *kapi.Pod) (io.ReadCloser, error) { opts := &kapi.PodLogOptions{ @@ -89,17 +86,17 @@ func (e *hookExecutor) Execute(hook *appsapi.LifecycleHook, rc *kapi.Replication tagEventMessages = append(tagEventMessages, fmt.Sprintf("image %q as %q", image, t.To.Name)) } } - strategyutil.RecordConfigEvent(e.events, rc, e.decoder, kapi.EventTypeNormal, "Started", + strategyutil.RecordConfigEvent(e.events, rc, kapi.EventTypeNormal, "Started", fmt.Sprintf("Running %s-hook (TagImages) %s for rc %s/%s", label, strings.Join(tagEventMessages, ","), rc.Namespace, rc.Name)) err = e.tagImages(hook, rc, suffix, label) case hook.ExecNewPod != nil: - strategyutil.RecordConfigEvent(e.events, rc, e.decoder, kapi.EventTypeNormal, "Started", + strategyutil.RecordConfigEvent(e.events, rc, kapi.EventTypeNormal, "Started", fmt.Sprintf("Running %s-hook (%q) for rc %s/%s", label, strings.Join(hook.ExecNewPod.Command, " "), rc.Namespace, rc.Name)) err = e.executeExecNewPod(hook, rc, suffix, label) } if err == nil { - strategyutil.RecordConfigEvent(e.events, rc, e.decoder, kapi.EventTypeNormal, "Completed", + strategyutil.RecordConfigEvent(e.events, rc, kapi.EventTypeNormal, "Completed", fmt.Sprintf("The %s-hook for rc %s/%s completed successfully", label, rc.Namespace, rc.Name)) return nil } @@ -107,11 +104,11 @@ func (e *hookExecutor) Execute(hook *appsapi.LifecycleHook, rc *kapi.Replication // Retry failures are treated the same as Abort. switch hook.FailurePolicy { case appsapi.LifecycleHookFailurePolicyAbort, appsapi.LifecycleHookFailurePolicyRetry: - strategyutil.RecordConfigEvent(e.events, rc, e.decoder, kapi.EventTypeWarning, "Failed", + strategyutil.RecordConfigEvent(e.events, rc, kapi.EventTypeWarning, "Failed", fmt.Sprintf("The %s-hook failed: %v, aborting rollout of %s/%s", label, err, rc.Namespace, rc.Name)) return fmt.Errorf("the %s hook failed: %v, aborting rollout of %s/%s", label, err, rc.Namespace, rc.Name) case appsapi.LifecycleHookFailurePolicyIgnore: - strategyutil.RecordConfigEvent(e.events, rc, e.decoder, kapi.EventTypeWarning, "Failed", + strategyutil.RecordConfigEvent(e.events, rc, kapi.EventTypeWarning, "Failed", fmt.Sprintf("The %s-hook failed: %v (ignore), rollout of %s/%s will continue", label, err, rc.Namespace, rc.Name)) return nil default: @@ -177,7 +174,7 @@ func (e *hookExecutor) tagImages(hook *appsapi.LifecycleHook, rc *kapi.Replicati // * Working directory // * Resources func (e *hookExecutor) executeExecNewPod(hook *appsapi.LifecycleHook, rc *kapi.ReplicationController, suffix, label string) error { - config, err := appsutil.DecodeDeploymentConfig(rc, e.decoder) + config, err := appsutil.DecodeDeploymentConfig(rc) if err != nil { return err } diff --git a/pkg/apps/strategy/support/lifecycle_test.go b/pkg/apps/strategy/support/lifecycle_test.go index 96520cfb045b..05936545d7a9 100644 --- a/pkg/apps/strategy/support/lifecycle_test.go +++ b/pkg/apps/strategy/support/lifecycle_test.go @@ -17,12 +17,10 @@ import ( "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/watch" clientgotesting "k8s.io/client-go/testing" - "k8s.io/kubernetes/pkg/api/legacyscheme" kapi "k8s.io/kubernetes/pkg/apis/core" kapihelper "k8s.io/kubernetes/pkg/apis/core/helper" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" - appsv1 "github.com/openshift/api/apps/v1" appsapi "github.com/openshift/origin/pkg/apps/apis/apps" appstest "github.com/openshift/origin/pkg/apps/apis/apps/test" appsutil "github.com/openshift/origin/pkg/apps/util" @@ -64,14 +62,13 @@ func TestHookExecutor_executeExecNewCreatePodFailure(t *testing.T) { }, } dc := appstest.OkDeploymentConfig(1) - deployment, _ := appsutil.MakeDeployment(dc, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(dc) client := newTestClient(dc) client.AddReactor("create", "pods", func(a clientgotesting.Action) (handled bool, ret runtime.Object, err error) { return true, nil, errors.New("could not create the pod") }) executor := &hookExecutor{ - pods: client.Core(), - decoder: legacyscheme.Codecs.UniversalDecoder(), + pods: client.Core(), } if err := executor.executeExecNewPod(hook, deployment, "hook", "test"); err == nil { @@ -88,7 +85,7 @@ func TestHookExecutor_executeExecNewPodSucceeded(t *testing.T) { } config := appstest.OkDeploymentConfig(1) - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) deployment.Spec.Template.Spec.NodeSelector = map[string]string{"labelKey1": "labelValue1", "labelKey2": "labelValue2"} client := newTestClient(config) @@ -116,9 +113,8 @@ func TestHookExecutor_executeExecNewPodSucceeded(t *testing.T) { }() executor := &hookExecutor{ - pods: client.Core(), - out: podLogs, - decoder: legacyscheme.Codecs.UniversalDecoder(), + pods: client.Core(), + out: podLogs, getPodLogs: func(*kapi.Pod) (io.ReadCloser, error) { return ioutil.NopCloser(strings.NewReader("test")), nil }, @@ -156,7 +152,7 @@ func TestHookExecutor_executeExecNewPodFailed(t *testing.T) { } config := appstest.OkDeploymentConfig(1) - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) client := newTestClient(config) podCreated := make(chan struct{}) @@ -181,9 +177,8 @@ func TestHookExecutor_executeExecNewPodFailed(t *testing.T) { }() executor := &hookExecutor{ - pods: client.Core(), - out: ioutil.Discard, - decoder: legacyscheme.Codecs.UniversalDecoder(), + pods: client.Core(), + out: ioutil.Discard, getPodLogs: func(*kapi.Pod) (io.ReadCloser, error) { return ioutil.NopCloser(strings.NewReader("test")), nil }, @@ -205,7 +200,7 @@ func TestHookExecutor_makeHookPodInvalidContainerRef(t *testing.T) { } config := appstest.OkDeploymentConfig(1) - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) _, err := makeHookPod(hook, deployment, &config.Spec.Strategy, "hook", nowFunc().Time) if err == nil { @@ -527,7 +522,7 @@ func TestHookExecutor_makeHookPodRestart(t *testing.T) { } config := appstest.OkDeploymentConfig(1) - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) pod, err := makeHookPod(hook, deployment, &config.Spec.Strategy, "hook", nowFunc().Time) if err != nil { @@ -617,7 +612,7 @@ func deployment(name, namespace string, strategyLabels, strategyAnnotations map[ }, }, } - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) deployment.Namespace = namespace return config, deployment } diff --git a/pkg/apps/strategy/util/util.go b/pkg/apps/strategy/util/util.go index fc308739642c..eda51f3cb10f 100644 --- a/pkg/apps/strategy/util/util.go +++ b/pkg/apps/strategy/util/util.go @@ -19,10 +19,10 @@ import ( // RecordConfigEvent records an event for the deployment config referenced by the // deployment. -func RecordConfigEvent(client kcoreclient.EventsGetter, deployment *kapi.ReplicationController, decoder runtime.Decoder, eventType, reason, msg string) { +func RecordConfigEvent(client kcoreclient.EventsGetter, deployment *kapi.ReplicationController, eventType, reason, msg string) { t := metav1.Time{Time: time.Now()} var obj runtime.Object = deployment - if config, err := appsutil.DecodeDeploymentConfig(deployment, decoder); err == nil { + if config, err := appsutil.DecodeDeploymentConfig(deployment); err == nil { obj = config } else { glog.Errorf("Unable to decode deployment config from %s/%s: %v", deployment.Namespace, deployment.Name, err) @@ -55,7 +55,7 @@ func RecordConfigEvent(client kcoreclient.EventsGetter, deployment *kapi.Replica // RecordConfigWarnings records all warning events from the replication controller to the // associated deployment config. -func RecordConfigWarnings(client kcoreclient.EventsGetter, rc *kapi.ReplicationController, decoder runtime.Decoder, out io.Writer) { +func RecordConfigWarnings(client kcoreclient.EventsGetter, rc *kapi.ReplicationController, out io.Writer) { if rc == nil { return } @@ -68,7 +68,7 @@ func RecordConfigWarnings(client kcoreclient.EventsGetter, rc *kapi.ReplicationC for _, e := range events.Items { if e.Type == kapi.EventTypeWarning { fmt.Fprintf(out, "--> %s: %s %s\n", e.Reason, rc.Name, e.Message) - RecordConfigEvent(client, rc, decoder, e.Type, e.Reason, e.Message) + RecordConfigEvent(client, rc, e.Type, e.Reason, e.Message) } } } diff --git a/pkg/apps/util/scheme.go b/pkg/apps/util/scheme.go new file mode 100644 index 000000000000..d3493e165a90 --- /dev/null +++ b/pkg/apps/util/scheme.go @@ -0,0 +1,48 @@ +package util + +import ( + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + coreapi "k8s.io/kubernetes/pkg/apis/core" + + appsv1 "github.com/openshift/api/apps/v1" + appsapi "github.com/openshift/origin/pkg/apps/apis/apps" + appsv1helpers "github.com/openshift/origin/pkg/apps/apis/apps/v1" +) + +var ( + // for decoding, we want to be tolerant of groupified and non-groupified + annotationDecodingScheme = runtime.NewScheme() + annotationDecoder runtime.Decoder + + // for encoding, we want to be strict on groupified + annotationEncodingScheme = runtime.NewScheme() + annotationEncoder runtime.Encoder +) + +func init() { + // TODO automatically do this in appsv1 AddToScheme + utilruntime.Must(corev1.AddToScheme(annotationDecodingScheme)) + utilruntime.Must(appsv1.AddToScheme(annotationDecodingScheme)) + utilruntime.Must(appsv1.AddToSchemeInCoreGroup(annotationDecodingScheme)) + // TODO eventually we shouldn't deal in internal versions, but for now decode into one. + utilruntime.Must(appsv1helpers.AddToScheme(annotationDecodingScheme)) + utilruntime.Must(appsv1helpers.AddToSchemeInCoreGroup(annotationDecodingScheme)) + utilruntime.Must(coreapi.AddToScheme(annotationDecodingScheme)) + utilruntime.Must(appsapi.AddToScheme(annotationDecodingScheme)) + utilruntime.Must(appsapi.AddToSchemeInCoreGroup(annotationDecodingScheme)) + annotationDecoderCodecFactory := serializer.NewCodecFactory(annotationDecodingScheme) + annotationDecoder = annotationDecoderCodecFactory.UniversalDecoder(appsapi.SchemeGroupVersion) + + // TODO automatically do this in appsv1 AddToScheme + utilruntime.Must(corev1.AddToScheme(annotationEncodingScheme)) + utilruntime.Must(appsv1.AddToScheme(annotationEncodingScheme)) + // TODO eventually we shouldn't deal in internal versions, but for now decode into one. + utilruntime.Must(appsv1helpers.AddToScheme(annotationEncodingScheme)) + utilruntime.Must(coreapi.AddToScheme(annotationEncodingScheme)) + utilruntime.Must(appsapi.AddToScheme(annotationEncodingScheme)) + annotationEncoderCodecFactory := serializer.NewCodecFactory(annotationEncodingScheme) + annotationEncoder = annotationEncoderCodecFactory.LegacyCodec(appsv1.SchemeGroupVersion) +} diff --git a/pkg/apps/util/scheme_test.go b/pkg/apps/util/scheme_test.go new file mode 100644 index 000000000000..3ade1c7a85d7 --- /dev/null +++ b/pkg/apps/util/scheme_test.go @@ -0,0 +1,47 @@ +package util + +import ( + "strings" + "testing" + + "github.com/davecgh/go-spew/spew" + + "k8s.io/apimachinery/pkg/runtime" + + appsapi "github.com/openshift/origin/pkg/apps/apis/apps" +) + +const legacyDC = `{ + "apiVersion": "v1", + "kind": "DeploymentConfig", + "metadata": { + "name": "sinatra-app-example-a" + } +} +` + +func TestLegacyDecoding(t *testing.T) { + result, err := runtime.Decode(annotationDecoder, []byte(legacyDC)) + if err != nil { + t.Fatal(err) + } + if result.(*appsapi.DeploymentConfig).Name != "sinatra-app-example-a" { + t.Fatal(spew.Sdump(result)) + } + + groupfiedBytes, err := runtime.Encode(annotationEncoder, result) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(groupfiedBytes), "apps.openshift.io/v1") { + t.Fatal(string(groupfiedBytes)) + } + + result2, err := runtime.Decode(annotationDecoder, groupfiedBytes) + if err != nil { + t.Fatal(err) + } + if result2.(*appsapi.DeploymentConfig).Name != "sinatra-app-example-a" { + t.Fatal(spew.Sdump(result2)) + } +} diff --git a/pkg/apps/util/util.go b/pkg/apps/util/util.go index 684afeccce90..766083fb68f2 100644 --- a/pkg/apps/util/util.go +++ b/pkg/apps/util/util.go @@ -314,8 +314,8 @@ func CopyApiEnvVarToV1EnvVar(in []api.EnvVar) []v1.EnvVar { // template and deployment config template encoded in the latest replication // controller. If they are different it will return an string diff containing // the change. -func HasLatestPodTemplate(currentConfig *appsapi.DeploymentConfig, rc *v1.ReplicationController, codec runtime.Codec) (bool, string, error) { - latestConfig, err := DecodeDeploymentConfig(rc, codec) +func HasLatestPodTemplate(currentConfig *appsapi.DeploymentConfig, rc *v1.ReplicationController) (bool, string, error) { + latestConfig, err := DecodeDeploymentConfig(rc) if err != nil { return true, "", err } @@ -352,9 +352,9 @@ func HasUpdatedImages(dc *appsapi.DeploymentConfig, rc *v1.ReplicationController // DecodeDeploymentConfig decodes a DeploymentConfig from controller using codec. An error is returned // if the controller doesn't contain an encoded config. -func DecodeDeploymentConfig(controller runtime.Object, decoder runtime.Decoder) (*appsapi.DeploymentConfig, error) { - encodedConfig := []byte(EncodedDeploymentConfigFor(controller)) - decoded, err := runtime.Decode(decoder, encodedConfig) +func DecodeDeploymentConfig(controller metav1.ObjectMetaAccessor) (*appsapi.DeploymentConfig, error) { + encodedConfig := controller.GetObjectMeta().GetAnnotations()[appsapi.DeploymentEncodedConfigAnnotation] + decoded, err := runtime.Decode(annotationDecoder, []byte(encodedConfig)) if err != nil { return nil, fmt.Errorf("failed to decode DeploymentConfig from controller: %v", err) } @@ -365,15 +365,6 @@ func DecodeDeploymentConfig(controller runtime.Object, decoder runtime.Decoder) return config, nil } -// EncodeDeploymentConfig encodes config as a string using codec. -func EncodeDeploymentConfig(config *appsapi.DeploymentConfig, codec runtime.Codec) (string, error) { - bytes, err := runtime.Encode(codec, config) - if err != nil { - return "", err - } - return string(bytes[:]), nil -} - func NewControllerRef(config *appsapi.DeploymentConfig) *metav1.OwnerReference { blockOwnerDeletion := true isController := true @@ -390,8 +381,8 @@ func NewControllerRef(config *appsapi.DeploymentConfig) *metav1.OwnerReference { // MakeDeployment creates a deployment represented as an internal ReplicationController and based on the given // DeploymentConfig. The controller replica count will be zero. // DEPRECATED: Will be replaced with external version eventually. -func MakeDeployment(config *appsapi.DeploymentConfig, codec runtime.Codec) (*api.ReplicationController, error) { - obj, err := MakeDeploymentV1(config, codec) +func MakeTestOnlyInternalDeployment(config *appsapi.DeploymentConfig) (*api.ReplicationController, error) { + obj, err := MakeDeploymentV1(config) if err != nil { return nil, err } @@ -406,11 +397,10 @@ func MakeDeployment(config *appsapi.DeploymentConfig, codec runtime.Codec) (*api // MakeDeploymentV1 creates a deployment represented as a ReplicationController and based on the given // DeploymentConfig. The controller replica count will be zero. -func MakeDeploymentV1(config *appsapi.DeploymentConfig, codec runtime.Codec) (*v1.ReplicationController, error) { - var err error - var encodedConfig string - - if encodedConfig, err = EncodeDeploymentConfig(config, codec); err != nil { +func MakeDeploymentV1(config *appsapi.DeploymentConfig) (*v1.ReplicationController, error) { + // EncodeDeploymentConfig encodes config as a string using codec. + encodedConfig, err := runtime.Encode(annotationEncoder, config) + if err != nil { return nil, err } @@ -471,7 +461,7 @@ func MakeDeploymentV1(config *appsapi.DeploymentConfig, codec runtime.Codec) (*v Annotations: map[string]string{ appsapi.DeploymentConfigAnnotation: config.Name, appsapi.DeploymentStatusAnnotation: string(appsapi.DeploymentStatusNew), - appsapi.DeploymentEncodedConfigAnnotation: encodedConfig, + appsapi.DeploymentEncodedConfigAnnotation: string(encodedConfig), appsapi.DeploymentVersionAnnotation: strconv.FormatInt(config.Status.LatestVersion, 10), // This is the target replica count for the new deployment. appsapi.DesiredReplicasAnnotation: strconv.Itoa(int(config.Spec.Replicas)), @@ -576,10 +566,6 @@ func DeploymentDesiredReplicas(obj runtime.Object) (int32, bool) { return int32AnnotationFor(obj, appsapi.DesiredReplicasAnnotation) } -func EncodedDeploymentConfigFor(obj runtime.Object) string { - return annotationFor(obj, appsapi.DeploymentEncodedConfigAnnotation) -} - func DeploymentVersionFor(obj runtime.Object) int64 { v, err := strconv.ParseInt(annotationFor(obj, appsapi.DeploymentVersionAnnotation), 10, 64) if err != nil { diff --git a/pkg/apps/util/util_test.go b/pkg/apps/util/util_test.go index a3120b9f9bb2..b63755f17532 100644 --- a/pkg/apps/util/util_test.go +++ b/pkg/apps/util/util_test.go @@ -8,10 +8,8 @@ import ( "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/kubernetes/pkg/api/legacyscheme" kapi "k8s.io/kubernetes/pkg/apis/core" - appsv1 "github.com/openshift/api/apps/v1" appsapi "github.com/openshift/origin/pkg/apps/apis/apps" appstest "github.com/openshift/origin/pkg/apps/apis/apps/test" @@ -68,7 +66,7 @@ func TestPodName(t *testing.T) { func TestMakeDeploymentOk(t *testing.T) { config := appstest.OkDeploymentConfig(1) - deployment, err := MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, err := MakeTestOnlyInternalDeployment(config) if err != nil { t.Fatalf("unexpected error: %#v", err) @@ -98,11 +96,11 @@ func TestMakeDeploymentOk(t *testing.T) { } } - if len(EncodedDeploymentConfigFor(deployment)) == 0 { + if len(deployment.Annotations[appsapi.DeploymentEncodedConfigAnnotation]) == 0 { t.Fatalf("expected deployment with DeploymentEncodedConfigAnnotation annotation") } - if decodedConfig, err := DecodeDeploymentConfig(deployment, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)); err != nil { + if decodedConfig, err := DecodeDeploymentConfig(deployment); err != nil { t.Fatalf("invalid encoded config on deployment: %v", err) } else { if e, a := config.Name, decodedConfig.Name; e != a { @@ -138,7 +136,7 @@ func TestMakeDeploymentOk(t *testing.T) { func TestDeploymentsByLatestVersion_sorting(t *testing.T) { mkdeployment := func(version int64) *kapi.ReplicationController { - deployment, _ := MakeDeployment(appstest.OkDeploymentConfig(version), legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(version)) return deployment } deployments := []*kapi.ReplicationController{ @@ -680,7 +678,7 @@ func TestRolloutExceededTimeoutSeconds(t *testing.T) { for _, tc := range tests { config := tc.config - deployment, err := MakeDeploymentV1(config, legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, err := MakeDeploymentV1(config) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/cmd/infra/deployer/deployer.go b/pkg/cmd/infra/deployer/deployer.go index b354afe5bb31..11099cbbb4f5 100644 --- a/pkg/cmd/infra/deployer/deployer.go +++ b/pkg/cmd/infra/deployer/deployer.go @@ -13,7 +13,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kv1core "k8s.io/client-go/kubernetes/typed/core/v1" restclient "k8s.io/client-go/rest" - "k8s.io/kubernetes/pkg/api/legacyscheme" kapi "k8s.io/kubernetes/pkg/apis/core" kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" "k8s.io/kubernetes/pkg/kubectl" @@ -141,11 +140,11 @@ func NewDeployer(kubeClient kubernetes.Interface, client kclientset.Interface, i switch config.Spec.Strategy.Type { case appsapi.DeploymentStrategyTypeRecreate: return recreate.NewRecreateDeploymentStrategy(client, images.Image(), - &kv1core.EventSinkImpl{Interface: kubeClient.CoreV1().Events("")}, legacyscheme.Codecs.UniversalDecoder(), out, errOut, until), nil + &kv1core.EventSinkImpl{Interface: kubeClient.CoreV1().Events("")}, out, errOut, until), nil case appsapi.DeploymentStrategyTypeRolling: recreateDeploymentStrategy := recreate.NewRecreateDeploymentStrategy(client, images.Image(), - &kv1core.EventSinkImpl{Interface: kubeClient.CoreV1().Events("")}, legacyscheme.Codecs.UniversalDecoder(), out, errOut, until) - return rolling.NewRollingDeploymentStrategy(config.Namespace, client, images.Image(), legacyscheme.Codecs.UniversalDecoder(), recreateDeploymentStrategy, out, errOut, until), nil + &kv1core.EventSinkImpl{Interface: kubeClient.CoreV1().Events("")}, out, errOut, until) + return rolling.NewRollingDeploymentStrategy(config.Namespace, client, images.Image(), recreateDeploymentStrategy, out, errOut, until), nil default: return nil, fmt.Errorf("unsupported strategy type: %s", config.Spec.Strategy.Type) } @@ -185,7 +184,7 @@ func (d *Deployer) Deploy(namespace, rcName string) error { } // Decode the config from the deployment. - config, err := appsutil.DecodeDeploymentConfig(to, legacyscheme.Codecs.UniversalDecoder()) + config, err := appsutil.DecodeDeploymentConfig(to) if err != nil { return fmt.Errorf("couldn't decode deployment config from deployment %s: %v", to.Name, err) } diff --git a/pkg/cmd/infra/deployer/deployer_test.go b/pkg/cmd/infra/deployer/deployer_test.go index df3c95cc087c..cfc4c6bca6ae 100644 --- a/pkg/cmd/infra/deployer/deployer_test.go +++ b/pkg/cmd/infra/deployer/deployer_test.go @@ -6,10 +6,8 @@ import ( "strconv" "testing" - "k8s.io/kubernetes/pkg/api/legacyscheme" kapi "k8s.io/kubernetes/pkg/apis/core" - appsv1 "github.com/openshift/api/apps/v1" appsapi "github.com/openshift/origin/pkg/apps/apis/apps" appstest "github.com/openshift/origin/pkg/apps/apis/apps/test" "github.com/openshift/origin/pkg/apps/strategy" @@ -218,7 +216,7 @@ func TestDeployer_deployScenarios(t *testing.T) { } func mkdeployment(version int64, status appsapi.DeploymentStatus) *kapi.ReplicationController { - deployment, _ := appsutil.MakeDeployment(appstest.OkDeploymentConfig(version), legacyscheme.Codecs.LegacyCodec(appsv1.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(version)) deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(status) return deployment } diff --git a/pkg/cmd/openshift-controller-manager/controller/apps.go b/pkg/cmd/openshift-controller-manager/controller/apps.go index dae32ec6d0ae..45a14808032a 100644 --- a/pkg/cmd/openshift-controller-manager/controller/apps.go +++ b/pkg/cmd/openshift-controller-manager/controller/apps.go @@ -1,9 +1,7 @@ package controller import ( - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes" - "k8s.io/kubernetes/pkg/api/legacyscheme" deployercontroller "github.com/openshift/origin/pkg/apps/controller/deployer" deployconfigcontroller "github.com/openshift/origin/pkg/apps/controller/deploymentconfig" @@ -22,9 +20,6 @@ func RunDeployerController(ctx ControllerContext) (bool, error) { return true, err } - groupVersion := schema.GroupVersion{Group: "", Version: "v1"} - annotationCodec := legacyscheme.Codecs.LegacyCodec(groupVersion) - imageTemplate := variable.NewDefaultImageTemplate() imageTemplate.Format = ctx.OpenshiftControllerConfig.Deployer.ImageTemplateFormat.Format imageTemplate.Latest = ctx.OpenshiftControllerConfig.Deployer.ImageTemplateFormat.Latest @@ -36,7 +31,6 @@ func RunDeployerController(ctx ControllerContext) (bool, error) { bootstrappolicy.DeployerServiceAccountName, imageTemplate.ExpandOrDie("deployer"), nil, - annotationCodec, ).Run(5, ctx.Stop) return true, nil @@ -50,15 +44,11 @@ func RunDeploymentConfigController(ctx ControllerContext) (bool, error) { return true, err } - groupVersion := schema.GroupVersion{Group: "apps.openshift.io", Version: "v1"} - annotationCodec := legacyscheme.Codecs.LegacyCodec(groupVersion) - go deployconfigcontroller.NewDeploymentConfigController( ctx.AppInformers.Apps().InternalVersion().DeploymentConfigs(), ctx.ExternalKubeInformers.Core().V1().ReplicationControllers(), ctx.ClientBuilder.OpenshiftInternalAppsClientOrDie(saName), kubeClient, - annotationCodec, ).Run(5, ctx.Stop) return true, nil diff --git a/pkg/oc/cli/cmd/deploy_test.go b/pkg/oc/cli/cmd/deploy_test.go index 1a00769a1949..530ea8e21299 100644 --- a/pkg/oc/cli/cmd/deploy_test.go +++ b/pkg/oc/cli/cmd/deploy_test.go @@ -11,7 +11,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" clientgotesting "k8s.io/client-go/testing" - "k8s.io/kubernetes/pkg/api/legacyscheme" kapi "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" @@ -26,7 +25,7 @@ import ( ) func deploymentFor(config *appsapi.DeploymentConfig, status appsapi.DeploymentStatus) *kapi.ReplicationController { - d, err := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsapi.SchemeGroupVersion)) + d, err := appsutil.MakeTestOnlyInternalDeployment(config) if err != nil { panic(err) } @@ -243,7 +242,7 @@ func TestCmdDeploy_cancelOk(t *testing.T) { config := appstest.OkDeploymentConfig(scenario.version) existingDeployments := &kapi.ReplicationControllerList{} for _, e := range scenario.existing { - d, _ := appsutil.MakeDeployment(appstest.OkDeploymentConfig(e.version), legacyscheme.Codecs.LegacyCodec(appsapi.SchemeGroupVersion)) + d, _ := appsutil.MakeTestOnlyInternalDeployment(appstest.OkDeploymentConfig(e.version)) d.Annotations[appsapi.DeploymentStatusAnnotation] = string(e.status) existingDeployments.Items = append(existingDeployments.Items, *d) } diff --git a/pkg/oc/cli/cmd/rollback_test.go b/pkg/oc/cli/cmd/rollback_test.go index 61a7ac01cda0..b8e9d22d68f6 100644 --- a/pkg/oc/cli/cmd/rollback_test.go +++ b/pkg/oc/cli/cmd/rollback_test.go @@ -3,7 +3,6 @@ package cmd import ( "testing" - "k8s.io/kubernetes/pkg/api/legacyscheme" kapi "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" @@ -78,7 +77,7 @@ func TestRollbackOptions_findTargetDeployment(t *testing.T) { existingControllers := &kapi.ReplicationControllerList{} for _, existing := range test.existing { config := appstest.OkDeploymentConfig(existing.version) - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsapi.SchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(existing.status) existingControllers.Items = append(existingControllers.Items, *deployment) } diff --git a/pkg/oc/cli/describe/deployments_test.go b/pkg/oc/cli/describe/deployments_test.go index bf125f1183d2..dd7f03e9406b 100644 --- a/pkg/oc/cli/describe/deployments_test.go +++ b/pkg/oc/cli/describe/deployments_test.go @@ -6,7 +6,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" clientgotesting "k8s.io/client-go/testing" - "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/autoscaling" kapi "k8s.io/kubernetes/pkg/apis/core" kfake "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" @@ -20,7 +19,7 @@ import ( func TestDeploymentConfigDescriber(t *testing.T) { config := appsapitest.OkDeploymentConfig(1) - deployment, _ := appsutil.MakeDeployment(config, legacyscheme.Codecs.LegacyCodec(appsapi.LegacySchemeGroupVersion)) + deployment, _ := appsutil.MakeTestOnlyInternalDeployment(config) podList := &kapi.PodList{} fake := &appsfake.Clientset{} diff --git a/test/integration/imagestream_test.go b/test/integration/imagestream_test.go index c3edb86de4b3..57de0e6a3f56 100644 --- a/test/integration/imagestream_test.go +++ b/test/integration/imagestream_test.go @@ -10,7 +10,6 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/diff" - "k8s.io/kubernetes/pkg/api/legacyscheme" kapi "k8s.io/kubernetes/pkg/apis/core" appsapi "github.com/openshift/origin/pkg/apps/apis/apps" @@ -418,7 +417,7 @@ func TestImageStreamTagLifecycleHook(t *testing.T) { imageClientset := imageclient.NewForConfigOrDie(testutil.GetClusterAdminClientConfigOrDie(clusterAdminKubeConfig)).Image() // can tag to a stream that exists - exec := stratsupport.NewHookExecutor(nil, imageClientset, clusterAdminKubeClientset.Core(), os.Stdout, legacyscheme.Codecs.UniversalDecoder()) + exec := stratsupport.NewHookExecutor(nil, imageClientset, clusterAdminKubeClientset.Core(), os.Stdout) err = exec.Execute( &appsapi.LifecycleHook{ TagImages: []appsapi.TagImageHook{ @@ -456,7 +455,7 @@ func TestImageStreamTagLifecycleHook(t *testing.T) { } // can execute a second time the same tag and it should work - exec = stratsupport.NewHookExecutor(nil, imageClientset, clusterAdminKubeClientset.Core(), os.Stdout, legacyscheme.Codecs.UniversalDecoder()) + exec = stratsupport.NewHookExecutor(nil, imageClientset, clusterAdminKubeClientset.Core(), os.Stdout) err = exec.Execute( &appsapi.LifecycleHook{ TagImages: []appsapi.TagImageHook{ @@ -488,7 +487,7 @@ func TestImageStreamTagLifecycleHook(t *testing.T) { } // can lifecycle tag a new image stream - exec = stratsupport.NewHookExecutor(nil, imageClientset, clusterAdminKubeClientset.Core(), os.Stdout, legacyscheme.Codecs.UniversalDecoder()) + exec = stratsupport.NewHookExecutor(nil, imageClientset, clusterAdminKubeClientset.Core(), os.Stdout) err = exec.Execute( &appsapi.LifecycleHook{ TagImages: []appsapi.TagImageHook{ From e09f5f4c7593139bd9dfa05984728bd3ae97b948 Mon Sep 17 00:00:00 2001 From: David Eads Date: Tue, 3 Jul 2018 08:02:05 -0400 Subject: [PATCH 2/2] move deployer test mock to point of use --- pkg/apps/util/test/support.go | 47 ----------------------- pkg/cmd/infra/deployer/deployer_test.go | 50 +++++++++++++++++++++---- 2 files changed, 43 insertions(+), 54 deletions(-) delete mode 100644 pkg/apps/util/test/support.go diff --git a/pkg/apps/util/test/support.go b/pkg/apps/util/test/support.go deleted file mode 100644 index 23210162578c..000000000000 --- a/pkg/apps/util/test/support.go +++ /dev/null @@ -1,47 +0,0 @@ -package test - -import ( - "fmt" - - "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/kubernetes/pkg/kubectl" -) - -type FakeScaler struct { - Events []ScaleEvent -} - -type ScaleEvent struct { - Name string - Size uint -} - -func (t *FakeScaler) Scale(namespace, name string, newSize uint, preconditions *kubectl.ScalePrecondition, retry, wait *kubectl.RetryParams, resource schema.GroupResource) error { - t.Events = append(t.Events, ScaleEvent{name, newSize}) - return nil -} - -func (t *FakeScaler) ScaleSimple(namespace, name string, preconditions *kubectl.ScalePrecondition, newSize uint, resource schema.GroupResource) (string, error) { - return "", fmt.Errorf("unexpected call to ScaleSimple") -} - -type FakeLaggedScaler struct { - Events []ScaleEvent - RetryCount int -} - -func (t *FakeLaggedScaler) Scale(namespace, name string, newSize uint, preconditions *kubectl.ScalePrecondition, retry, wait *kubectl.RetryParams, resource schema.GroupResource) error { - if t.RetryCount != 2 { - t.RetryCount += 1 - // This is faking a real error from the - // "k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle" package. - return errors.NewForbidden(resource, name, fmt.Errorf("%s: not yet ready to handle request", name)) - } - t.Events = append(t.Events, ScaleEvent{name, newSize}) - return nil -} - -func (t *FakeLaggedScaler) ScaleSimple(namespace, name string, preconditions *kubectl.ScalePrecondition, newSize uint, resource schema.GroupResource) (string, error) { - return "", nil -} diff --git a/pkg/cmd/infra/deployer/deployer_test.go b/pkg/cmd/infra/deployer/deployer_test.go index cfc4c6bca6ae..612c4612a321 100644 --- a/pkg/cmd/infra/deployer/deployer_test.go +++ b/pkg/cmd/infra/deployer/deployer_test.go @@ -6,17 +6,15 @@ import ( "strconv" "testing" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" kapi "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/kubernetes/pkg/kubectl" appsapi "github.com/openshift/origin/pkg/apps/apis/apps" appstest "github.com/openshift/origin/pkg/apps/apis/apps/test" "github.com/openshift/origin/pkg/apps/strategy" appsutil "github.com/openshift/origin/pkg/apps/util" - cmdtest "github.com/openshift/origin/pkg/apps/util/test" - - // install all APIs - _ "github.com/openshift/origin/pkg/api/install" - _ "k8s.io/kubernetes/pkg/apis/core/install" ) func TestDeployer_getDeploymentFail(t *testing.T) { @@ -32,7 +30,7 @@ func TestDeployer_getDeploymentFail(t *testing.T) { t.Fatal("unexpected call") return nil, nil }, - scaler: &cmdtest.FakeScaler{}, + scaler: &FakeScaler{}, } err := deployer.Deploy("namespace", "name") @@ -146,7 +144,7 @@ func TestDeployer_deployScenarios(t *testing.T) { var actualFrom, actualTo *kapi.ReplicationController var actualDesired int32 to := findDeployment(s.toVersion) - scaler := &cmdtest.FakeScaler{} + scaler := &FakeScaler{} deployer := &Deployer{ out: &bytes.Buffer{}, @@ -228,3 +226,41 @@ type testStrategy struct { func (t *testStrategy) Deploy(from *kapi.ReplicationController, to *kapi.ReplicationController, desiredReplicas int) error { return t.deployFunc(from, to, desiredReplicas) } + +type FakeScaler struct { + Events []ScaleEvent +} + +type ScaleEvent struct { + Name string + Size uint +} + +func (t *FakeScaler) Scale(namespace, name string, newSize uint, preconditions *kubectl.ScalePrecondition, retry, wait *kubectl.RetryParams, resource schema.GroupResource) error { + t.Events = append(t.Events, ScaleEvent{name, newSize}) + return nil +} + +func (t *FakeScaler) ScaleSimple(namespace, name string, preconditions *kubectl.ScalePrecondition, newSize uint, resource schema.GroupResource) (string, error) { + return "", fmt.Errorf("unexpected call to ScaleSimple") +} + +type FakeLaggedScaler struct { + Events []ScaleEvent + RetryCount int +} + +func (t *FakeLaggedScaler) Scale(namespace, name string, newSize uint, preconditions *kubectl.ScalePrecondition, retry, wait *kubectl.RetryParams, resource schema.GroupResource) error { + if t.RetryCount != 2 { + t.RetryCount += 1 + // This is faking a real error from the + // "k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle" package. + return errors.NewForbidden(resource, name, fmt.Errorf("%s: not yet ready to handle request", name)) + } + t.Events = append(t.Events, ScaleEvent{name, newSize}) + return nil +} + +func (t *FakeLaggedScaler) ScaleSimple(namespace, name string, preconditions *kubectl.ScalePrecondition, newSize uint, resource schema.GroupResource) (string, error) { + return "", nil +}