From c8c0f98c1d526915ce113d952f84c79886aab6f6 Mon Sep 17 00:00:00 2001 From: faizanahmad055 Date: Thu, 26 Jul 2018 12:59:10 +0500 Subject: [PATCH] Implement PR-2 review comments --- internal/pkg/callbacks/rolling_upgrade.go | 25 +---- internal/pkg/controller/controller_test.go | 120 ++++++++++----------- internal/pkg/handler/update.go | 28 ++--- internal/pkg/handler/update_test.go | 48 ++++----- 4 files changed, 102 insertions(+), 119 deletions(-) diff --git a/internal/pkg/callbacks/rolling_upgrade.go b/internal/pkg/callbacks/rolling_upgrade.go index b44beef0..ba395097 100644 --- a/internal/pkg/callbacks/rolling_upgrade.go +++ b/internal/pkg/callbacks/rolling_upgrade.go @@ -19,14 +19,12 @@ type ContainersFunc func(interface{}) []v1.Container //UpdateFunc performs the resource update type UpdateFunc func(kubernetes.Interface, string, interface{}) error -type ResourceTypeFunc func() string - //RollingUpgradeFuncs contains generic functions to perform rolling upgrade type RollingUpgradeFuncs struct { - ItemsFunc ItemsFunc - ContainersFunc ContainersFunc - UpdateFunc UpdateFunc - ResourceTypeFunc ResourceTypeFunc + ItemsFunc ItemsFunc + ContainersFunc ContainersFunc + UpdateFunc UpdateFunc + ResourceType string } // GetDeploymentItems returns the deployments in given namespace @@ -71,21 +69,6 @@ func GetStatefulsetContainers(item interface{}) []v1.Container { return item.(apps_v1beta1.StatefulSet).Spec.Template.Spec.Containers } -// GetDeploymentTypeName returns Deployment resource type -func GetDeploymentTypeName() string { - return "Deployment" -} - -// GetDaemonSetTypeName returns DaemonSet resource type -func GetDaemonSetTypeName() string { - return "DaemonSet" -} - -// GetStatefulSetTypeName returns StatefulSet resource type -func GetStatefulSetTypeName() string { - return "StatefulSet" -} - // UpdateDeployment performs rolling upgrade on deployment func UpdateDeployment(client kubernetes.Interface, namespace string, resource interface{}) error { deployment := resource.(v1beta1.Deployment) diff --git a/internal/pkg/controller/controller_test.go b/internal/pkg/controller/controller_test.go index ef860f66..3fdd4b86 100644 --- a/internal/pkg/controller/controller_test.go +++ b/internal/pkg/controller/controller_test.go @@ -83,10 +83,10 @@ func TestControllerUpdatingConfigmapShouldCreateEnvInDeployment(t *testing.T) { Annotation: constants.ConfigmapUpdateOnChangeAnnotation, } deploymentFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDeploymentItems, - ContainersFunc: callbacks.GetDeploymentContainers, - UpdateFunc: callbacks.UpdateDeployment, - ResourceTypeFunc: callbacks.GetDeploymentTypeName, + ItemsFunc: callbacks.GetDeploymentItems, + ContainersFunc: callbacks.GetDeploymentContainers, + UpdateFunc: callbacks.UpdateDeployment, + ResourceType: "Deployment", } updated := testutil.VerifyResourceUpdate(client, config, constants.ConfigmapEnvVarPostfix, deploymentFuncs) if !updated { @@ -146,10 +146,10 @@ func TestControllerForUpdatingConfigmapShouldUpdateDeployment(t *testing.T) { } deploymentFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDeploymentItems, - ContainersFunc: callbacks.GetDeploymentContainers, - UpdateFunc: callbacks.UpdateDeployment, - ResourceTypeFunc: callbacks.GetDeploymentTypeName, + ItemsFunc: callbacks.GetDeploymentItems, + ContainersFunc: callbacks.GetDeploymentContainers, + UpdateFunc: callbacks.UpdateDeployment, + ResourceType: "Deployment", } updated := testutil.VerifyResourceUpdate(client, config, constants.ConfigmapEnvVarPostfix, deploymentFuncs) @@ -203,10 +203,10 @@ func TestControllerUpdatingConfigmapLabelsShouldNotCreateorUpdateEnvInDeployment Annotation: constants.ConfigmapUpdateOnChangeAnnotation, } deploymentFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDeploymentItems, - ContainersFunc: callbacks.GetDeploymentContainers, - UpdateFunc: callbacks.UpdateDeployment, - ResourceTypeFunc: callbacks.GetDeploymentTypeName, + ItemsFunc: callbacks.GetDeploymentItems, + ContainersFunc: callbacks.GetDeploymentContainers, + UpdateFunc: callbacks.UpdateDeployment, + ResourceType: "Deployment", } updated := testutil.VerifyResourceUpdate(client, config, constants.ConfigmapEnvVarPostfix, deploymentFuncs) if updated { @@ -259,10 +259,10 @@ func TestControllerUpdatingSecretShouldCreateEnvInDeployment(t *testing.T) { Annotation: constants.SecretUpdateOnChangeAnnotation, } deploymentFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDeploymentItems, - ContainersFunc: callbacks.GetDeploymentContainers, - UpdateFunc: callbacks.UpdateDeployment, - ResourceTypeFunc: callbacks.GetDeploymentTypeName, + ItemsFunc: callbacks.GetDeploymentItems, + ContainersFunc: callbacks.GetDeploymentContainers, + UpdateFunc: callbacks.UpdateDeployment, + ResourceType: "Deployment", } updated := testutil.VerifyResourceUpdate(client, config, constants.SecretEnvVarPostfix, deploymentFuncs) if !updated { @@ -321,10 +321,10 @@ func TestControllerUpdatingSecretShouldUpdateEnvInDeployment(t *testing.T) { Annotation: constants.SecretUpdateOnChangeAnnotation, } deploymentFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDeploymentItems, - ContainersFunc: callbacks.GetDeploymentContainers, - UpdateFunc: callbacks.UpdateDeployment, - ResourceTypeFunc: callbacks.GetDeploymentTypeName, + ItemsFunc: callbacks.GetDeploymentItems, + ContainersFunc: callbacks.GetDeploymentContainers, + UpdateFunc: callbacks.UpdateDeployment, + ResourceType: "Deployment", } updated := testutil.VerifyResourceUpdate(client, config, constants.SecretEnvVarPostfix, deploymentFuncs) if !updated { @@ -376,10 +376,10 @@ func TestControllerUpdatingSecretLabelsShouldNotCreateorUpdateEnvInDeployment(t Annotation: constants.SecretUpdateOnChangeAnnotation, } deploymentFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDeploymentItems, - ContainersFunc: callbacks.GetDeploymentContainers, - UpdateFunc: callbacks.UpdateDeployment, - ResourceTypeFunc: callbacks.GetDeploymentTypeName, + ItemsFunc: callbacks.GetDeploymentItems, + ContainersFunc: callbacks.GetDeploymentContainers, + UpdateFunc: callbacks.UpdateDeployment, + ResourceType: "Deployment", } updated := testutil.VerifyResourceUpdate(client, config, constants.SecretEnvVarPostfix, deploymentFuncs) if updated { @@ -432,10 +432,10 @@ func TestControllerUpdatingConfigmapShouldCreateEnvInDaemonSet(t *testing.T) { Annotation: constants.ConfigmapUpdateOnChangeAnnotation, } daemonSetFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDaemonSetItems, - ContainersFunc: callbacks.GetDaemonSetContainers, - UpdateFunc: callbacks.UpdateDaemonSet, - ResourceTypeFunc: callbacks.GetDaemonSetTypeName, + ItemsFunc: callbacks.GetDaemonSetItems, + ContainersFunc: callbacks.GetDaemonSetContainers, + UpdateFunc: callbacks.UpdateDaemonSet, + ResourceType: "DaemonSet", } updated := testutil.VerifyResourceUpdate(client, config, constants.ConfigmapEnvVarPostfix, daemonSetFuncs) if !updated { @@ -494,10 +494,10 @@ func TestControllerForUpdatingConfigmapShouldUpdateDaemonSet(t *testing.T) { Annotation: constants.ConfigmapUpdateOnChangeAnnotation, } daemonSetFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDaemonSetItems, - ContainersFunc: callbacks.GetDaemonSetContainers, - UpdateFunc: callbacks.UpdateDaemonSet, - ResourceTypeFunc: callbacks.GetDaemonSetTypeName, + ItemsFunc: callbacks.GetDaemonSetItems, + ContainersFunc: callbacks.GetDaemonSetContainers, + UpdateFunc: callbacks.UpdateDaemonSet, + ResourceType: "DaemonSet", } updated := testutil.VerifyResourceUpdate(client, config, constants.ConfigmapEnvVarPostfix, daemonSetFuncs) if !updated { @@ -550,10 +550,10 @@ func TestControllerUpdatingSecretShouldCreateEnvInDaemonSet(t *testing.T) { Annotation: constants.SecretUpdateOnChangeAnnotation, } daemonSetFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDaemonSetItems, - ContainersFunc: callbacks.GetDaemonSetContainers, - UpdateFunc: callbacks.UpdateDaemonSet, - ResourceTypeFunc: callbacks.GetDaemonSetTypeName, + ItemsFunc: callbacks.GetDaemonSetItems, + ContainersFunc: callbacks.GetDaemonSetContainers, + UpdateFunc: callbacks.UpdateDaemonSet, + ResourceType: "DaemonSet", } updated := testutil.VerifyResourceUpdate(client, config, constants.SecretEnvVarPostfix, daemonSetFuncs) if !updated { @@ -613,10 +613,10 @@ func TestControllerUpdatingSecretShouldUpdateEnvInDaemonSet(t *testing.T) { Annotation: constants.SecretUpdateOnChangeAnnotation, } daemonSetFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDaemonSetItems, - ContainersFunc: callbacks.GetDaemonSetContainers, - UpdateFunc: callbacks.UpdateDaemonSet, - ResourceTypeFunc: callbacks.GetDaemonSetTypeName, + ItemsFunc: callbacks.GetDaemonSetItems, + ContainersFunc: callbacks.GetDaemonSetContainers, + UpdateFunc: callbacks.UpdateDaemonSet, + ResourceType: "DaemonSet", } updated := testutil.VerifyResourceUpdate(client, config, constants.SecretEnvVarPostfix, daemonSetFuncs) if !updated { @@ -668,10 +668,10 @@ func TestControllerUpdatingSecretLabelsShouldNotCreateorUpdateEnvInDaemonSet(t * Annotation: constants.SecretUpdateOnChangeAnnotation, } daemonSetFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDaemonSetItems, - ContainersFunc: callbacks.GetDaemonSetContainers, - UpdateFunc: callbacks.UpdateDaemonSet, - ResourceTypeFunc: callbacks.GetDaemonSetTypeName, + ItemsFunc: callbacks.GetDaemonSetItems, + ContainersFunc: callbacks.GetDaemonSetContainers, + UpdateFunc: callbacks.UpdateDaemonSet, + ResourceType: "DaemonSet", } updated := testutil.VerifyResourceUpdate(client, config, constants.SecretEnvVarPostfix, daemonSetFuncs) if updated { @@ -724,10 +724,10 @@ func TestControllerUpdatingConfigmapShouldCreateEnvInStatefulSet(t *testing.T) { Annotation: constants.ConfigmapUpdateOnChangeAnnotation, } statefulSetFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetStatefulSetItems, - ContainersFunc: callbacks.GetStatefulsetContainers, - UpdateFunc: callbacks.UpdateStatefulset, - ResourceTypeFunc: callbacks.GetStatefulSetTypeName, + ItemsFunc: callbacks.GetStatefulSetItems, + ContainersFunc: callbacks.GetStatefulsetContainers, + UpdateFunc: callbacks.UpdateStatefulset, + ResourceType: "StatefulSet", } updated := testutil.VerifyResourceUpdate(client, config, constants.ConfigmapEnvVarPostfix, statefulSetFuncs) if !updated { @@ -786,10 +786,10 @@ func TestControllerForUpdatingConfigmapShouldUpdateStatefulSet(t *testing.T) { Annotation: constants.ConfigmapUpdateOnChangeAnnotation, } statefulSetFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetStatefulSetItems, - ContainersFunc: callbacks.GetStatefulsetContainers, - UpdateFunc: callbacks.UpdateStatefulset, - ResourceTypeFunc: callbacks.GetStatefulSetTypeName, + ItemsFunc: callbacks.GetStatefulSetItems, + ContainersFunc: callbacks.GetStatefulsetContainers, + UpdateFunc: callbacks.UpdateStatefulset, + ResourceType: "StatefulSet", } updated := testutil.VerifyResourceUpdate(client, config, constants.ConfigmapEnvVarPostfix, statefulSetFuncs) if !updated { @@ -842,10 +842,10 @@ func TestControllerUpdatingSecretShouldCreateEnvInStatefulSet(t *testing.T) { Annotation: constants.SecretUpdateOnChangeAnnotation, } statefulSetFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetStatefulSetItems, - ContainersFunc: callbacks.GetStatefulsetContainers, - UpdateFunc: callbacks.UpdateStatefulset, - ResourceTypeFunc: callbacks.GetStatefulSetTypeName, + ItemsFunc: callbacks.GetStatefulSetItems, + ContainersFunc: callbacks.GetStatefulsetContainers, + UpdateFunc: callbacks.UpdateStatefulset, + ResourceType: "StatefulSet", } updated := testutil.VerifyResourceUpdate(client, config, constants.SecretEnvVarPostfix, statefulSetFuncs) if !updated { @@ -904,10 +904,10 @@ func TestControllerUpdatingSecretShouldUpdateEnvInStatefulSet(t *testing.T) { Annotation: constants.SecretUpdateOnChangeAnnotation, } statefulSetFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetStatefulSetItems, - ContainersFunc: callbacks.GetStatefulsetContainers, - UpdateFunc: callbacks.UpdateStatefulset, - ResourceTypeFunc: callbacks.GetStatefulSetTypeName, + ItemsFunc: callbacks.GetStatefulSetItems, + ContainersFunc: callbacks.GetStatefulsetContainers, + UpdateFunc: callbacks.UpdateStatefulset, + ResourceType: "StatefulSet", } updated := testutil.VerifyResourceUpdate(client, config, constants.SecretEnvVarPostfix, statefulSetFuncs) if !updated { diff --git a/internal/pkg/handler/update.go b/internal/pkg/handler/update.go index 01dc04bf..d8bda1b4 100644 --- a/internal/pkg/handler/update.go +++ b/internal/pkg/handler/update.go @@ -28,22 +28,22 @@ func (r ResourceUpdatedHandler) Handle() error { logrus.Infof("Detected changes in object %s", r.Resource) // process resource based on its type rollingUpgrade(r, callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDeploymentItems, - ContainersFunc: callbacks.GetDeploymentContainers, - UpdateFunc: callbacks.UpdateDeployment, - ResourceTypeFunc: callbacks.GetDeploymentTypeName, + ItemsFunc: callbacks.GetDeploymentItems, + ContainersFunc: callbacks.GetDeploymentContainers, + UpdateFunc: callbacks.UpdateDeployment, + ResourceType: "Deployment", }) rollingUpgrade(r, callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDaemonSetItems, - ContainersFunc: callbacks.GetDaemonSetContainers, - UpdateFunc: callbacks.UpdateDaemonSet, - ResourceTypeFunc: callbacks.GetDaemonSetTypeName, + ItemsFunc: callbacks.GetDaemonSetItems, + ContainersFunc: callbacks.GetDaemonSetContainers, + UpdateFunc: callbacks.UpdateDaemonSet, + ResourceType: "DaemonSet", }) rollingUpgrade(r, callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetStatefulSetItems, - ContainersFunc: callbacks.GetStatefulsetContainers, - UpdateFunc: callbacks.UpdateStatefulset, - ResourceTypeFunc: callbacks.GetStatefulSetTypeName, + ItemsFunc: callbacks.GetStatefulSetItems, + ContainersFunc: callbacks.GetStatefulsetContainers, + UpdateFunc: callbacks.UpdateStatefulset, + ResourceType: "StatefulSet", }) } return nil @@ -124,9 +124,9 @@ func PerformRollingUpgrade(client kubernetes.Interface, config util.Config, enva } else { err = upgradeFuncs.UpdateFunc(client, config.Namespace, i) if err != nil { - logrus.Errorf("Update %s failed %v", upgradeFuncs.ResourceTypeFunc, err) + logrus.Errorf("Update %s failed %v", upgradeFuncs.ResourceType, err) } else { - logrus.Infof("Updated %s of type %s", config.ResourceName, upgradeFuncs.ResourceTypeFunc) + logrus.Infof("Updated %s of type %s", config.ResourceName, upgradeFuncs.ResourceType) } break } diff --git a/internal/pkg/handler/update_test.go b/internal/pkg/handler/update_test.go index 4c077b76..2e9c6f1c 100644 --- a/internal/pkg/handler/update_test.go +++ b/internal/pkg/handler/update_test.go @@ -152,10 +152,10 @@ func TestRollingUpgradeForDeploymentWithConfigmap(t *testing.T) { Annotation: constants.ConfigmapUpdateOnChangeAnnotation, } deploymentFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDeploymentItems, - ContainersFunc: callbacks.GetDeploymentContainers, - UpdateFunc: callbacks.UpdateDeployment, - ResourceTypeFunc: callbacks.GetDeploymentTypeName, + ItemsFunc: callbacks.GetDeploymentItems, + ContainersFunc: callbacks.GetDeploymentContainers, + UpdateFunc: callbacks.UpdateDeployment, + ResourceType: "Deployment", } err := PerformRollingUpgrade(client, config, constants.ConfigmapEnvVarPostfix, deploymentFuncs) @@ -180,10 +180,10 @@ func TestRollingUpgradeForDeploymentWithSecret(t *testing.T) { Annotation: constants.SecretUpdateOnChangeAnnotation, } deploymentFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDeploymentItems, - ContainersFunc: callbacks.GetDeploymentContainers, - UpdateFunc: callbacks.UpdateDeployment, - ResourceTypeFunc: callbacks.GetDeploymentTypeName, + ItemsFunc: callbacks.GetDeploymentItems, + ContainersFunc: callbacks.GetDeploymentContainers, + UpdateFunc: callbacks.UpdateDeployment, + ResourceType: "Deployment", } err := PerformRollingUpgrade(client, config, constants.SecretEnvVarPostfix, deploymentFuncs) @@ -208,10 +208,10 @@ func TestRollingUpgradeForDaemonSetWithConfigmap(t *testing.T) { Annotation: constants.ConfigmapUpdateOnChangeAnnotation, } daemonSetFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDaemonSetItems, - ContainersFunc: callbacks.GetDaemonSetContainers, - UpdateFunc: callbacks.UpdateDaemonSet, - ResourceTypeFunc: callbacks.GetDaemonSetTypeName, + ItemsFunc: callbacks.GetDaemonSetItems, + ContainersFunc: callbacks.GetDaemonSetContainers, + UpdateFunc: callbacks.UpdateDaemonSet, + ResourceType: "DaemonSet", } err := PerformRollingUpgrade(client, config, constants.ConfigmapEnvVarPostfix, daemonSetFuncs) @@ -237,10 +237,10 @@ func TestRollingUpgradeForDaemonSetWithSecret(t *testing.T) { Annotation: constants.SecretUpdateOnChangeAnnotation, } daemonSetFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetDaemonSetItems, - ContainersFunc: callbacks.GetDaemonSetContainers, - UpdateFunc: callbacks.UpdateDaemonSet, - ResourceTypeFunc: callbacks.GetDaemonSetTypeName, + ItemsFunc: callbacks.GetDaemonSetItems, + ContainersFunc: callbacks.GetDaemonSetContainers, + UpdateFunc: callbacks.UpdateDaemonSet, + ResourceType: "DaemonSet", } err := PerformRollingUpgrade(client, config, constants.SecretEnvVarPostfix, daemonSetFuncs) @@ -266,10 +266,10 @@ func TestRollingUpgradeForStatefulSetWithConfigmap(t *testing.T) { Annotation: constants.ConfigmapUpdateOnChangeAnnotation, } statefulSetFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetStatefulSetItems, - ContainersFunc: callbacks.GetStatefulsetContainers, - UpdateFunc: callbacks.UpdateStatefulset, - ResourceTypeFunc: callbacks.GetStatefulSetTypeName, + ItemsFunc: callbacks.GetStatefulSetItems, + ContainersFunc: callbacks.GetStatefulsetContainers, + UpdateFunc: callbacks.UpdateStatefulset, + ResourceType: "StatefulSet", } err := PerformRollingUpgrade(client, config, constants.ConfigmapEnvVarPostfix, statefulSetFuncs) @@ -295,10 +295,10 @@ func TestRollingUpgradeForStatefulSetWithSecret(t *testing.T) { Annotation: constants.SecretUpdateOnChangeAnnotation, } statefulSetFuncs := callbacks.RollingUpgradeFuncs{ - ItemsFunc: callbacks.GetStatefulSetItems, - ContainersFunc: callbacks.GetStatefulsetContainers, - UpdateFunc: callbacks.UpdateStatefulset, - ResourceTypeFunc: callbacks.GetStatefulSetTypeName, + ItemsFunc: callbacks.GetStatefulSetItems, + ContainersFunc: callbacks.GetStatefulsetContainers, + UpdateFunc: callbacks.UpdateStatefulset, + ResourceType: "StatefulSet", } err := PerformRollingUpgrade(client, config, constants.SecretEnvVarPostfix, statefulSetFuncs)