Skip to content

Commit

Permalink
apps: remove more internal helpers
Browse files Browse the repository at this point in the history
  • Loading branch information
mfojtik committed Jul 19, 2018
1 parent d8f94cb commit 80f1367
Show file tree
Hide file tree
Showing 19 changed files with 175 additions and 208 deletions.
16 changes: 9 additions & 7 deletions pkg/apps/controller/deployer/deployer_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ import (
"k8s.io/client-go/util/workqueue"
kapi "k8s.io/kubernetes/pkg/apis/core"

appsv1 "github.com/openshift/api/apps/v1"
appsapi "github.com/openshift/origin/pkg/apps/apis/apps"
appsinternalutil "github.com/openshift/origin/pkg/apps/controller/util"
appsutil "github.com/openshift/origin/pkg/apps/util"
"github.com/openshift/origin/pkg/util"
)

Expand Down Expand Up @@ -190,7 +192,7 @@ func (c *DeploymentController) handle(deployment *v1.ReplicationController, will
// to ensure that changes to 'unrelated' pods don't result in updates to
// the deployment. So, the image check will have to be done in other areas
// of the code as well.
if appsinternalutil.DeploymentNameFor(deployer) != deployment.Name {
if appsutil.DeploymentNameFor(deployer) != deployment.Name {
nextStatus = appsapi.DeploymentStatusFailed
updatedAnnotations[appsapi.DeploymentStatusReasonAnnotation] = appsapi.DeploymentFailedUnrelatedDeploymentExists
c.emitDeploymentEvent(deployment, v1.EventTypeWarning, "FailedCreate", fmt.Sprintf("Error creating deployer pod since another pod with the same name (%q) exists", deployer.Name))
Expand Down Expand Up @@ -354,7 +356,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 := appsinternalutil.DecodeDeploymentConfig(deployment)
deploymentConfig, err := appsutil.DecodeDeploymentConfig(deployment)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -383,7 +385,7 @@ func (c *DeploymentController) makeDeployerPod(deployment *v1.ReplicationControl
Name: appsinternalutil.DeployerPodNameForDeployment(deployment.Name),
Annotations: map[string]string{
appsapi.DeploymentAnnotation: deployment.Name,
appsapi.DeploymentConfigAnnotation: appsinternalutil.DeploymentConfigNameFor(deployment),
appsapi.DeploymentConfigAnnotation: appsutil.DeploymentConfigNameFor(deployment),
},
Labels: map[string]string{
appsapi.DeployerPodForDeploymentLabel: deployment.Name,
Expand All @@ -406,7 +408,7 @@ func (c *DeploymentController) makeDeployerPod(deployment *v1.ReplicationControl
Args: container.Args,
Image: container.Image,
Env: envVars,
Resources: appsinternalutil.CopyApiResourcesToV1Resources(&deploymentConfig.Spec.Strategy.Resources),
Resources: deploymentConfig.Spec.Strategy.Resources,
},
},
ActiveDeadlineSeconds: &maxDeploymentDurationSeconds,
Expand Down Expand Up @@ -442,7 +444,7 @@ func (c *DeploymentController) makeDeployerPod(deployment *v1.ReplicationControl
// of the factory's Environment and the strategy's environment as the
// container environment.
//
func (c *DeploymentController) makeDeployerContainer(strategy *appsapi.DeploymentStrategy) *v1.Container {
func (c *DeploymentController) makeDeployerContainer(strategy *appsv1.DeploymentStrategy) *v1.Container {
image := c.deployerImage
var environment []v1.EnvVar
var command []string
Expand All @@ -456,7 +458,7 @@ func (c *DeploymentController) makeDeployerContainer(strategy *appsapi.Deploymen
if len(p.Command) > 0 {
command = p.Command
}
for _, env := range appsinternalutil.CopyApiEnvVarToV1EnvVar(strategy.CustomParams.Environment) {
for _, env := range strategy.CustomParams.Environment {
set.Insert(env.Name)
environment = append(environment, env)
}
Expand Down Expand Up @@ -484,7 +486,7 @@ func (c *DeploymentController) makeDeployerContainer(strategy *appsapi.Deploymen
}

func (c *DeploymentController) getDeployerPods(deployment *v1.ReplicationController) ([]*v1.Pod, error) {
return c.podLister.Pods(deployment.Namespace).List(appsinternalutil.DeployerPodSelector(deployment.Name))
return c.podLister.Pods(deployment.Namespace).List(appsutil.DeployerPodSelector(deployment.Name))
}

func (c *DeploymentController) setDeployerPodsOwnerRef(deployment *v1.ReplicationController) error {
Expand Down
51 changes: 30 additions & 21 deletions pkg/apps/controller/deployer/deployer_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func TestHandle_createPodOk(t *testing.T) {
// Verify new -> pending
config := appstest.OkDeploymentConfig(1)
config.Spec.Strategy = appstest.OkCustomStrategy()
deployment, _ := appsinternalutil.MakeDeploymentV1(config)
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(config)
deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusNew)
deployment.Spec.Template.Spec.NodeSelector = map[string]string{"labelKey1": "labelValue1", "labelKey2": "labelValue2"}
deployment.CreationTimestamp = metav1.Now()
Expand Down Expand Up @@ -216,11 +216,11 @@ func TestHandle_createPodOk(t *testing.T) {
t.Fatalf("expected deployment pod annotation %s, got %s", e, a)
}

if e := appsinternalutil.DeploymentNameFor(createdPod); len(e) == 0 {
if e := appsutil.DeploymentNameFor(createdPod); len(e) == 0 {
t.Fatalf("missing deployment annotation")
}

if e, a := updatedDeployment.Name, appsinternalutil.DeploymentNameFor(createdPod); e != a {
if e, a := updatedDeployment.Name, appsutil.DeploymentNameFor(createdPod); e != a {
t.Fatalf("expected pod deployment annotation %s, got %s", e, a)
}

Expand Down Expand Up @@ -276,7 +276,7 @@ func TestHandle_createPodFail(t *testing.T) {
})

config := appstest.OkDeploymentConfig(1)
deployment, _ := appsinternalutil.MakeDeploymentV1(config)
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(config)
deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusNew)
deployment.CreationTimestamp = metav1.Now()

Expand Down Expand Up @@ -332,7 +332,7 @@ func TestHandle_deployerPodAlreadyExists(t *testing.T) {
var updatedDeployment *v1.ReplicationController

config := appstest.OkDeploymentConfig(1)
deployment, _ := appsinternalutil.MakeDeploymentV1(config)
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(config)
deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusNew)
deployment.CreationTimestamp = metav1.Now()
deployerPodName := appsutil.DeployerPodNameForDeployment(deployment.Name)
Expand Down Expand Up @@ -373,7 +373,7 @@ func TestHandle_unrelatedPodAlreadyExists(t *testing.T) {
var updatedDeployment *v1.ReplicationController

config := appstest.OkDeploymentConfig(1)
deployment, _ := appsinternalutil.MakeDeploymentV1(config)
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(config)
deployment.CreationTimestamp = metav1.Now()
deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusNew)

Expand Down Expand Up @@ -414,7 +414,7 @@ func TestHandle_unrelatedPodAlreadyExistsTestScaled(t *testing.T) {
var updatedDeployment *v1.ReplicationController

config := appstest.TestDeploymentConfig(appstest.OkDeploymentConfig(1))
deployment, _ := appsinternalutil.MakeDeploymentV1(config)
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(config)
deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusNew)
deployment.CreationTimestamp = metav1.Now()
one := int32(1)
Expand Down Expand Up @@ -486,7 +486,7 @@ func TestHandle_noop(t *testing.T) {
for _, test := range tests {
client := fake.NewSimpleClientset()

deployment, _ := appsinternalutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1))
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(appstest.OkDeploymentConfig(1))
deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(test.deploymentPhase)
deployment.CreationTimestamp = metav1.Now()

Expand Down Expand Up @@ -531,7 +531,7 @@ func TestHandle_failedTest(t *testing.T) {

// Verify successful cleanup
config := appstest.TestDeploymentConfig(appstest.OkDeploymentConfig(1))
deployment, _ := appsinternalutil.MakeDeploymentV1(config)
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(config)
deployment.CreationTimestamp = metav1.Now()
one := int32(1)
deployment.Spec.Replicas = &one
Expand Down Expand Up @@ -574,7 +574,7 @@ func TestHandle_cleanupPodOk(t *testing.T) {

// Verify successful cleanup
config := appstest.OkDeploymentConfig(1)
deployment, _ := appsinternalutil.MakeDeploymentV1(config)
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(config)
deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusComplete)
deployment.CreationTimestamp = metav1.Now()

Expand Down Expand Up @@ -619,7 +619,7 @@ func TestHandle_cleanupPodOkTest(t *testing.T) {

// Verify successful cleanup
config := appstest.TestDeploymentConfig(appstest.OkDeploymentConfig(1))
deployment, _ := appsinternalutil.MakeDeploymentV1(config)
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(config)
deployment.CreationTimestamp = metav1.Now()
one := int32(1)
deployment.Spec.Replicas = &one
Expand Down Expand Up @@ -664,7 +664,7 @@ func TestHandle_cleanupPodNoop(t *testing.T) {

// Verify no-op
config := appstest.OkDeploymentConfig(1)
deployment, _ := appsinternalutil.MakeDeploymentV1(config)
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(config)
deployment.CreationTimestamp = metav1.Now()
deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusComplete)

Expand Down Expand Up @@ -696,7 +696,7 @@ func TestHandle_cleanupPodFail(t *testing.T) {

// Verify error
config := appstest.OkDeploymentConfig(1)
deployment, _ := appsinternalutil.MakeDeploymentV1(config)
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(config)
deployment.CreationTimestamp = metav1.Now()
deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusComplete)

Expand Down Expand Up @@ -727,7 +727,7 @@ func TestHandle_cancelNew(t *testing.T) {
return true, rc, nil
})

deployment, _ := appsinternalutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1))
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(appstest.OkDeploymentConfig(1))
deployment.CreationTimestamp = metav1.Now()
deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusNew)
deployment.Annotations[appsapi.DeploymentCancelledAnnotation] = appsapi.DeploymentCancelledAnnotationValue
Expand All @@ -749,7 +749,7 @@ func TestHandle_cleanupNewWithDeployers(t *testing.T) {
var updatedDeployment *v1.ReplicationController
deletedDeployer := false

deployment, _ := appsinternalutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1))
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(appstest.OkDeploymentConfig(1))
deployment.CreationTimestamp = metav1.Now()
deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(appsapi.DeploymentStatusNew)
deployment.Annotations[appsapi.DeploymentCancelledAnnotation] = appsapi.DeploymentCancelledAnnotationValue
Expand Down Expand Up @@ -844,7 +844,7 @@ func TestHandle_cleanupPostNew(t *testing.T) {
return true, nil, nil
})

deployment, _ := appsinternalutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1))
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(appstest.OkDeploymentConfig(1))
deployment.CreationTimestamp = metav1.Now()
deployment.Annotations[appsapi.DeploymentCancelledAnnotation] = appsapi.DeploymentCancelledAnnotationValue
deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(test.deploymentPhase)
Expand Down Expand Up @@ -903,7 +903,7 @@ func TestHandle_deployerPodDisappeared(t *testing.T) {
return true, nil, nil
})

deployment, err := appsinternalutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1))
deployment, err := appsinternalutil.MakeDeploymentV1FromInternalConfig(appstest.OkDeploymentConfig(1))
if err != nil {
t.Errorf("%s: unexpected error: %v", test.name, err)
continue
Expand Down Expand Up @@ -1042,7 +1042,7 @@ func TestHandle_transitionFromDeployer(t *testing.T) {
return true, nil, nil
})

deployment, _ := appsinternalutil.MakeDeploymentV1(appstest.OkDeploymentConfig(1))
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(appstest.OkDeploymentConfig(1))
deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(test.deploymentPhase)
deployment.CreationTimestamp = metav1.Now()

Expand Down Expand Up @@ -1098,7 +1098,7 @@ func TestDeployerCustomLabelsAndAnnotations(t *testing.T) {
config.Spec.Strategy = test.strategy
config.Spec.Strategy.Labels = test.labels
config.Spec.Strategy.Annotations = test.annotations
deployment, _ := appsinternalutil.MakeDeploymentV1(config)
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(config)

client := &fake.Clientset{}
client.AddReactor("create", "pods", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
Expand All @@ -1125,13 +1125,22 @@ func TestDeployerCustomLabelsAndAnnotations(t *testing.T) {
}
}

func copyApiResourcesToV1Resources(in *api.ResourceRequirements) v1.ResourceRequirements {
in = in.DeepCopy()
out := v1.ResourceRequirements{}
if err := kapiv1.Convert_core_ResourceRequirements_To_v1_ResourceRequirements(in, &out, nil); err != nil {
panic(err)
}
return out
}

func TestMakeDeployerPod(t *testing.T) {
client := &fake.Clientset{}
controller := okDeploymentController(client, nil, nil, true, v1.PodUnknown)
config := appstest.OkDeploymentConfig(1)
deployment, _ := appsinternalutil.MakeDeploymentV1(config)
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(config)
container := controller.makeDeployerContainer(&config.Spec.Strategy)
container.Resources = appsinternalutil.CopyApiResourcesToV1Resources(&config.Spec.Strategy.Resources)
container.Resources = copyApiResourcesToV1Resources(&config.Spec.Strategy.Resources)
defaultGracePeriod := int64(10)
defaultShareProcessNamespace := false
maxDeploymentDurationSeconds := appsapi.MaxDeploymentDurationSeconds
Expand Down
3 changes: 2 additions & 1 deletion pkg/apps/controller/deployer/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
kcontroller "k8s.io/kubernetes/pkg/controller"

appsinternalutil "github.com/openshift/origin/pkg/apps/controller/util"
appsutil "github.com/openshift/origin/pkg/apps/util"
)

// NewDeployerController creates a new DeploymentController.
Expand Down Expand Up @@ -153,7 +154,7 @@ func (c *DeploymentController) enqueueReplicationController(rc *v1.ReplicationCo
}

func (c *DeploymentController) rcForDeployerPod(pod *v1.Pod) (*v1.ReplicationController, error) {
rcName := appsinternalutil.DeploymentNameFor(pod)
rcName := appsutil.DeploymentNameFor(pod)
if len(rcName) == 0 {
// Not a deployer pod, so don't bother with it.
return nil, nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,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 := appsinternalutil.MakeDeploymentV1(config)
deployment, err := appsinternalutil.MakeDeploymentV1FromInternalConfig(config)
if err != nil {
return fatalError(fmt.Sprintf("couldn't make deployment from (potentially invalid) deployment config %s: %v", appsinternalutil.LabelForDeploymentConfig(config), err))
}
Expand Down Expand Up @@ -402,11 +402,11 @@ func calculateStatus(config *appsapi.DeploymentConfig, rcs []*v1.ReplicationCont
if !latestExists {
latestRC = nil
} else {
latestReplicas = appsinternalutil.GetStatusReplicaCountForDeployments([]*v1.ReplicationController{latestRC})
latestReplicas = appsutil.GetStatusReplicaCountForDeployments([]*v1.ReplicationController{latestRC})
}

available := appsinternalutil.GetAvailableReplicaCountForReplicationControllers(rcs)
total := appsinternalutil.GetReplicaCountForDeployments(rcs)
available := appsutil.GetAvailableReplicaCountForReplicationControllers(rcs)
total := appsutil.GetReplicaCountForDeployments(rcs)
unavailableReplicas := total - available
if unavailableReplicas < 0 {
unavailableReplicas = 0
Expand All @@ -421,10 +421,10 @@ func calculateStatus(config *appsapi.DeploymentConfig, rcs []*v1.ReplicationCont
LatestVersion: config.Status.LatestVersion,
Details: config.Status.Details,
ObservedGeneration: generation,
Replicas: appsinternalutil.GetStatusReplicaCountForDeployments(rcs),
Replicas: appsutil.GetStatusReplicaCountForDeployments(rcs),
UpdatedReplicas: latestReplicas,
AvailableReplicas: available,
ReadyReplicas: appsinternalutil.GetReadyReplicaCountForReplicationControllers(rcs),
ReadyReplicas: appsutil.GetReadyReplicaCountForReplicationControllers(rcs),
UnavailableReplicas: unavailableReplicas,
Conditions: config.Status.Conditions,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
appsinternalutil "github.com/openshift/origin/pkg/apps/controller/util"
appsfake "github.com/openshift/origin/pkg/apps/generated/internalclientset/fake"
"github.com/openshift/origin/pkg/apps/generated/listers/apps/internalversion"
appsutil "github.com/openshift/origin/pkg/apps/util"
)

func alwaysReady() bool { return true }
Expand All @@ -51,7 +52,7 @@ func TestHandleScenarios(t *testing.T) {
config = appstest.TestDeploymentConfig(config)
}
config.Namespace = "test"
deployment, _ := appsinternalutil.MakeDeploymentV1(config)
deployment, _ := appsinternalutil.MakeDeploymentV1FromInternalConfig(config)
deployment.Annotations[appsapi.DeploymentStatusAnnotation] = string(d.status)
if d.cancelled {
deployment.Annotations[appsapi.DeploymentCancelledAnnotation] = appsapi.DeploymentCancelledAnnotationValue
Expand Down Expand Up @@ -411,8 +412,8 @@ func TestHandleScenarios(t *testing.T) {
for _, deployment := range deployments {
actualDeployments = append(actualDeployments, deployment)
}
sort.Sort(appsinternalutil.ByLatestVersionDescV1(expectedDeployments))
sort.Sort(appsinternalutil.ByLatestVersionDescV1(actualDeployments))
sort.Sort(appsutil.ByLatestVersionDesc(expectedDeployments))
sort.Sort(appsutil.ByLatestVersionDesc(actualDeployments))

if updatedConfig != nil {
config = updatedConfig
Expand Down
Loading

0 comments on commit 80f1367

Please sign in to comment.