Implement golangci review comments on PR-2

internal/pkg/controller/controller_test.go

@@ -65,7 +65,10 @@ func TestControllerUpdatingConfigmapShouldCreateEnvInDeployment(t *testing.T) {
 	}
 
 	// Creating deployment
-	testutil.CreateDeployment(client, configmapName, namespace)
+	_, err = testutil.CreateDeployment(client, configmapName, namespace)
+	if err != nil {
+		t.Errorf("Error  in deployment creation: %v", err)
+	}
 
 	// Updating configmap for first time
 	updateErr := testutil.UpdateConfigMap(configmapClient, namespace, configmapName, "", "www.stakater.com")
@@ -123,7 +126,10 @@ func TestControllerForUpdatingConfigmapShouldUpdateDeployment(t *testing.T) {
 	}
 
 	// Creating deployment
-	testutil.CreateDeployment(client, configmapName, namespace)
+	_, err = testutil.CreateDeployment(client, configmapName, namespace)
+	if err != nil {
+		t.Errorf("Error  in deployment creation: %v", err)
+	}
 
 	// Updating configmap for first time
 	updateErr := testutil.UpdateConfigMap(configmapClient, namespace, configmapName, "", "www.stakater.com")
@@ -200,7 +206,10 @@ func TestControllerUpdatingConfigmapLabelsShouldNotCreateorUpdateEnvInDeployment
 	}
 
 	// Creating deployment
-	testutil.CreateDeployment(client, configmapName, namespace)
+	_, err = testutil.CreateDeployment(client, configmapName, namespace)
+	if err != nil {
+		t.Errorf("Error  in deployment creation: %v", err)
+	}
 
 	// Updating configmap for first time
 	updateErr := testutil.UpdateConfigMap(configmapClient, namespace, configmapName, "test", "www.google.com")
@@ -285,7 +294,7 @@ func TestControllerUpdatingSecretShouldCreateEnvInDeployment(t *testing.T) {
 	}
 
 	//Deleting Secret
-	testutil.DeleteSecret(client, namespace, secretName)
+	err = testutil.DeleteSecret(client, namespace, secretName)
 	if err != nil {
 		logrus.Errorf("Error while deleting the secret %v", err)
 	}
@@ -358,7 +367,7 @@ func TestControllerUpdatingSecretShouldUpdateEnvInDeployment(t *testing.T) {
 	}
 
 	//Deleting Secret
-	testutil.DeleteSecret(client, namespace, secretName)
+	err = testutil.DeleteSecret(client, namespace, secretName)
 	if err != nil {
 		logrus.Errorf("Error while deleting the secret %v", err)
 	}
@@ -413,7 +422,7 @@ func TestControllerUpdatingSecretLabelsShouldNotCreateorUpdateEnvInDeployment(t
 	}
 
 	//Deleting Secret
-	testutil.DeleteSecret(client, namespace, secretName)
+	err = testutil.DeleteSecret(client, namespace, secretName)
 	if err != nil {
 		logrus.Errorf("Error while deleting the secret %v", err)
 	}

internal/pkg/handler/update.go

@@ -38,34 +38,38 @@ func rollingUpgrade(r ResourceUpdatedHandler, rollingUpgradeType string) {
 	if err != nil {
 		logrus.Fatalf("Unable to create Kubernetes client error = %v", err)
 	}
-	var namespace, resourceName, shaData, oldSHAdata, envarPostfix, annotation string
-	if _, ok := r.Resource.(*v1.ConfigMap); ok {
-		logrus.Infof("Performing 'Updated' action for resource of type 'configmap'")
-		namespace = r.Resource.(*v1.ConfigMap).Namespace
-		resourceName = r.Resource.(*v1.ConfigMap).Name
-		envarPostfix = common.ConfigmapEnvarPostfix
-		annotation = common.ConfigmapUpdateOnChangeAnnotation
-		shaData = getSHAfromConfigmapData(r.Resource.(*v1.ConfigMap).Data)
-		oldSHAdata = getSHAfromConfigmapData(r.OldResource.(*v1.ConfigMap).Data)
-	} else if _, ok := r.Resource.(*v1.Secret); ok {
-		logrus.Infof("Performing 'Updated' action for resource of type 'secret'")
-		namespace = r.Resource.(*v1.Secret).Namespace
-		resourceName = r.Resource.(*v1.Secret).Name
-		envarPostfix = common.SecretEnvarPostfix
-		annotation = common.SecretUpdateOnChangeAnnotation
-		shaData = getSHAfromSecretData(r.Resource.(*v1.Secret).Data)
-		oldSHAdata = getSHAfromSecretData(r.OldResource.(*v1.Secret).Data)
-	} else {
-		logrus.Warnf("Invalid resource: Resource should be 'Secret' or 'Configmap' but found, %v", r.Resource)
-	}
 
+	var shaData, oldSHAdata string
+	shaData = getSHAfromData(r.Resource)
+	oldSHAdata = getSHAfromData(r.OldResource)
 	if shaData != oldSHAdata {
+		var namespace, resourceName, envarPostfix, annotation string
+		if _, ok := r.Resource.(*v1.ConfigMap); ok {
+			logrus.Infof("Performing 'Updated' action for resource of type 'configmap'")
+			namespace = r.Resource.(*v1.ConfigMap).Namespace
+			resourceName = r.Resource.(*v1.ConfigMap).Name
+			envarPostfix = common.ConfigmapEnvarPostfix
+			annotation = common.ConfigmapUpdateOnChangeAnnotation
+		} else if _, ok := r.Resource.(*v1.Secret); ok {
+			logrus.Infof("Performing 'Updated' action for resource of type 'secret'")
+			namespace = r.Resource.(*v1.Secret).Namespace
+			resourceName = r.Resource.(*v1.Secret).Name
+			envarPostfix = common.SecretEnvarPostfix
+			annotation = common.SecretUpdateOnChangeAnnotation
+		} else {
+			logrus.Warnf("Invalid resource: Resource should be 'Secret' or 'Configmap' but found, %v", r.Resource)
+		}
+	
 		if rollingUpgradeType == "deployments" {
-			RollingUpgradeDeployment(client, namespace, resourceName, shaData, envarPostfix, annotation)
+			err = RollingUpgradeDeployment(client, namespace, resourceName, shaData, envarPostfix, annotation)
 		} else if rollingUpgradeType == "daemonsets" {
-			RollingUpgradeDaemonSets(client, namespace, resourceName, shaData, envarPostfix, annotation)
+			err = RollingUpgradeDaemonSets(client, namespace, resourceName, shaData, envarPostfix, annotation)
 		} else if rollingUpgradeType == "statefulSets" {
-			RollingUpgradeStatefulSets(client, namespace, resourceName, shaData, envarPostfix, annotation)
+			err = RollingUpgradeStatefulSets(client, namespace, resourceName, shaData, envarPostfix, annotation)
+		}
+
+		if err != nil {
+			logrus.Errorf("Rolling upgrade failed for %s of resource type %s", resourceName, rollingUpgradeType)
 		}
 	} else {
 		logrus.Infof("Resource update will not happen because no data change detected")
@@ -205,22 +209,19 @@ func updateEnvVar(envs []v1.EnvVar, envar string, shaData string) bool {
 	return false
 }
 
-func getSHAfromConfigmapData(data map[string]string) string {
-	logrus.Infof("Generating SHA for configmap data")
+func getSHAfromData(resource interface{}) string {
 	values := []string{}
-	for k, v := range data {
-		values = append(values, k+"="+v)
+	if _, ok := resource.(*v1.ConfigMap); ok {
+		logrus.Infof("Generating SHA for configmap data")
+		for k, v := range resource.(*v1.ConfigMap).Data {
+			values = append(values, k+"="+v)
+		}
+	} else if _, ok := resource.(*v1.Secret); ok{
+		logrus.Infof("Generating SHA for secret data")
+		for k, v := range resource.(*v1.Secret).Data {
+			values = append(values, k+"="+string(v[:]))
+		}
 	}
 	sort.Strings(values)
 	return crypto.GenerateSHA(strings.Join(values, ";"))
-}
-
-func getSHAfromSecretData(data map[string][]byte) string {
-	logrus.Infof("Generating SHA for secret data")
-	values := []string{}
-	for k, v := range data {
-		values = append(values, k+"="+string(v[:]))
-	}
-	sort.Strings(values)
-	return crypto.GenerateSHA(strings.Join(values, ";"))
-}
+}

internal/pkg/handler/update_test.go

@@ -59,37 +59,37 @@ func setup() {
 	}
 
 	// Creating Deployment with configmap
-	testutil.CreateDeployment(client, configmapName, namespace)
+	_, err = testutil.CreateDeployment(client, configmapName, namespace)
 	if err != nil {
 		logrus.Errorf("Error in Deployment with configmap creation: %v", err)
 	}
 
 	// Creating Deployment with secret
-	testutil.CreateDeployment(client, secretName, namespace)
+	_, err = testutil.CreateDeployment(client, secretName, namespace)
 	if err != nil {
 		logrus.Errorf("Error in Deployment with secret creation: %v", err)
 	}
 
 	// Creating Daemonset with configmap
-	testutil.CreateDaemonset(client, configmapName, namespace)
+	_, err = testutil.CreateDaemonset(client, configmapName, namespace)
 	if err != nil {
 		logrus.Errorf("Error in Daemonset with configmap creation: %v", err)
 	}
 
 	// Creating Daemonset with secret
-	testutil.CreateDaemonset(client, secretName, namespace)
+	_, err = testutil.CreateDaemonset(client, secretName, namespace)
 	if err != nil {
 		logrus.Errorf("Error in Daemonset with secret creation: %v", err)
 	}
 
 	// Creating Statefulset with configmap
-	testutil.CreateStatefulset(client, configmapName, namespace)
+	_, err = testutil.CreateStatefulset(client, configmapName, namespace)
 	if err != nil {
 		logrus.Errorf("Error in Statefulset with configmap creation: %v", err)
 	}
 
 	// Creating Statefulset with secret
-	testutil.CreateStatefulset(client, secretName, namespace)
+	_, err = testutil.CreateStatefulset(client, secretName, namespace)
 	if err != nil {
 		logrus.Errorf("Error in Statefulset with secret creation: %v", err)
 	}
@@ -152,8 +152,11 @@ func teardown() {
 
 func TestRollingUpgradeForDeploymentWithConfigmap(t *testing.T) {
 	shaData := testutil.ConvertResourceToSHA(testutil.SecretResourceType, namespace, configmapName, "www.stakater.com")
-	RollingUpgradeDeployment(client, namespace, configmapName, shaData, common.ConfigmapEnvarPostfix, common.ConfigmapUpdateOnChangeAnnotation)
+	err := RollingUpgradeDeployment(client, namespace, configmapName, shaData, common.ConfigmapEnvarPostfix, common.ConfigmapUpdateOnChangeAnnotation)
 	time.Sleep(5 * time.Second)
+	if err != nil {
+		t.Errorf("Rolling upgrade failed for Deployment with configmap")
+	}
 
 	logrus.Infof("Verifying deployment update")
 	updated := testutil.VerifyDeploymentUpdate(client, namespace, configmapName, common.ConfigmapEnvarPostfix, shaData, common.ConfigmapUpdateOnChangeAnnotation)
@@ -164,8 +167,11 @@ func TestRollingUpgradeForDeploymentWithConfigmap(t *testing.T) {
 
 func TestRollingUpgradeForDeploymentWithSecret(t *testing.T) {
 	shaData := testutil.ConvertResourceToSHA(testutil.SecretResourceType, namespace, secretName, "dGVzdFVwZGF0ZWRTZWNyZXRFbmNvZGluZ0ZvclJlbG9hZGVy")
-	RollingUpgradeDeployment(client, namespace, secretName, shaData, common.SecretEnvarPostfix, common.SecretUpdateOnChangeAnnotation)
+	err := RollingUpgradeDeployment(client, namespace, secretName, shaData, common.SecretEnvarPostfix, common.SecretUpdateOnChangeAnnotation)
 	time.Sleep(5 * time.Second)
+	if err != nil {
+		t.Errorf("Rolling upgrade failed for Deployment with Secret")
+	}
 
 	logrus.Infof("Verifying deployment update")
 	updated := testutil.VerifyDeploymentUpdate(client, namespace, secretName, common.SecretEnvarPostfix, shaData, common.SecretUpdateOnChangeAnnotation)
@@ -176,8 +182,11 @@ func TestRollingUpgradeForDeploymentWithSecret(t *testing.T) {
 
 func TestRollingUpgradeForDaemonsetWithConfigmap(t *testing.T) {
 	shaData := testutil.ConvertResourceToSHA(testutil.ConfigmapResourceType, namespace, configmapName, "www.facebook.com")
-	RollingUpgradeDaemonSets(client, namespace, configmapName, shaData, common.ConfigmapEnvarPostfix, common.ConfigmapUpdateOnChangeAnnotation)
+	err := RollingUpgradeDaemonSets(client, namespace, configmapName, shaData, common.ConfigmapEnvarPostfix, common.ConfigmapUpdateOnChangeAnnotation)
 	time.Sleep(5 * time.Second)
+	if err != nil {
+		t.Errorf("Rolling upgrade failed for DaemonSet with configmap")
+	}
 
 	logrus.Infof("Verifying daemonset update")
 	updated := testutil.VerifyDaemonsetUpdate(client, namespace, configmapName, common.ConfigmapEnvarPostfix, shaData, common.ConfigmapUpdateOnChangeAnnotation)
@@ -188,8 +197,11 @@ func TestRollingUpgradeForDaemonsetWithConfigmap(t *testing.T) {
 
 func TestRollingUpgradeForDaemonsetWithSecret(t *testing.T) {
 	shaData := testutil.ConvertResourceToSHA(testutil.SecretResourceType, namespace, secretName, "d3d3LmZhY2Vib29rLmNvbQ==")
-	RollingUpgradeDaemonSets(client, namespace, secretName, shaData, common.SecretEnvarPostfix, common.SecretUpdateOnChangeAnnotation)
+	err := RollingUpgradeDaemonSets(client, namespace, secretName, shaData, common.SecretEnvarPostfix, common.SecretUpdateOnChangeAnnotation)
 	time.Sleep(5 * time.Second)
+	if err != nil {
+		t.Errorf("Rolling upgrade failed for DaemonSet with secret")
+	}
 
 	logrus.Infof("Verifying daemonset update")
 	updated := testutil.VerifyDaemonsetUpdate(client, namespace, secretName, common.SecretEnvarPostfix, shaData, common.SecretUpdateOnChangeAnnotation)
@@ -200,8 +212,11 @@ func TestRollingUpgradeForDaemonsetWithSecret(t *testing.T) {
 
 func TestRollingUpgradeForStatefulsetWithConfigmap(t *testing.T) {
 	shaData := testutil.ConvertResourceToSHA(testutil.ConfigmapResourceType, namespace, configmapName, "www.twitter.com")
-	RollingUpgradeStatefulSets(client, namespace, configmapName, shaData, common.ConfigmapEnvarPostfix, common.ConfigmapUpdateOnChangeAnnotation)
+	err := RollingUpgradeStatefulSets(client, namespace, configmapName, shaData, common.ConfigmapEnvarPostfix, common.ConfigmapUpdateOnChangeAnnotation)
 	time.Sleep(5 * time.Second)
+	if err != nil {
+		t.Errorf("Rolling upgrade failed for StatefulSet with configmap")
+	}
 
 	logrus.Infof("Verifying statefulset update")
 	updated := testutil.VerifyStatefulsetUpdate(client, namespace, configmapName, common.ConfigmapEnvarPostfix, shaData, common.ConfigmapUpdateOnChangeAnnotation)
@@ -212,8 +227,11 @@ func TestRollingUpgradeForStatefulsetWithConfigmap(t *testing.T) {
 
 func TestRollingUpgradeForStatefulsetWithSecret(t *testing.T) {
 	shaData := testutil.ConvertResourceToSHA(testutil.SecretResourceType, namespace, secretName, "d3d3LnR3aXR0ZXIuY29t")
-	RollingUpgradeStatefulSets(client, namespace, secretName, shaData, common.SecretEnvarPostfix, common.SecretUpdateOnChangeAnnotation)
+	err := RollingUpgradeStatefulSets(client, namespace, secretName, shaData, common.SecretEnvarPostfix, common.SecretUpdateOnChangeAnnotation)
 	time.Sleep(5 * time.Second)
+	if err != nil {
+		t.Errorf("Rolling upgrade failed for StatefulSet with secret")
+	}
 
 	logrus.Infof("Verifying statefulset update")
 	updated := testutil.VerifyStatefulsetUpdate(client, namespace, secretName, common.SecretEnvarPostfix, shaData, common.SecretUpdateOnChangeAnnotation)

internal/pkg/testutil/kube.go

@@ -326,7 +326,7 @@ func ConvertResourceToSHA(resourceType string, namespace string, resourceName st
 	} else if resourceType == ConfigmapResourceType {
 		configmap := GetConfigmap(namespace, resourceName, data)
 		for k, v := range configmap.Data {
-			values = append(values, k+"="+string(v[:]))
+			values = append(values, k+"="+v)
 		}
 	}
 	sort.Strings(values)