refactor: design more akin to aws-asg, utilizing getConfigValue

Author: ashley-graham

plugins/builtin/target/gce-mig/plugin/gce.go

@@ -19,7 +19,6 @@ import (
 
 const (
 	defaultRetryInterval = 10 * time.Second
-	defaultRetryAttempts = 15
 
 	// nodeAttrGCEHostname is the node attribute to use when identifying the
 	// GCE hostname of a node.

plugins/builtin/target/gce-mig/plugin/plugin.go

@@ -22,12 +22,18 @@ const (
 	// pluginName is the unique name of the this plugin amongst Target plugins.
 	pluginName = "gce-mig"
 
+	// configKeys represents the known configuration parameters required at
+	// varying points throughout the plugins lifecycle.	
 	configKeyCredentials = "credentials"
 	configKeyProject     = "project"
 	configKeyRegion      = "region"
 	configKeyZone        = "zone"
 	configKeyMIGName     = "mig_name"
 	configKeyRetryAttempts  = "retry_attempts"
+
+	// configValues are the default values used when a configuration key is not
+	// supplied by the operator that are specific to the plugin.
+	configValueRetryAttemptsDefault = "15"
 )
 
 var (
@@ -63,7 +69,6 @@ type TargetPlugin struct {
 func NewGCEMIGPlugin(log hclog.Logger) *TargetPlugin {
 	return &TargetPlugin{
 		logger: log,
-		retryAttempts: defaultRetryAttempts,
 	}
 }
 
@@ -76,14 +81,6 @@ func (t *TargetPlugin) SetConfig(config map[string]string) error {
 		return err
 	}
 
-	if val, ok := t.config[configKeyRetryAttempts]; ok {
-        attempts, err := strconv.Atoi(val)
-        if err != nil {
-            return fmt.Errorf("invalid value for %s: %v", configKeyRetryAttempts, err)
-        }
-        t.retryAttempts = attempts
-    }
-
 	clusterUtils, err := scaleutils.NewClusterScaleUtils(nomad.ConfigFromNamespacedMap(config), t.logger)
 	if err != nil {
 		return err
@@ -93,6 +90,12 @@ func (t *TargetPlugin) SetConfig(config map[string]string) error {
 	t.clusterUtils = clusterUtils
 	t.clusterUtils.ClusterNodeIDLookupFunc = gceNodeIDMap
 
+	retryLimit, err := strconv.Atoi(getConfigValue(config, configKeyRetryAttempts, configValueRetryAttemptsDefault))
+	if err != nil {
+		return err
+	}
+	t.retryAttempts = retryLimit
+
 	return nil
 }
 
@@ -223,6 +226,8 @@ func (t *TargetPlugin) calculateMIG(config map[string]string) (instanceGroup, er
 	}
 }
 
+// getValue retrieves a configuration value from either the provided config
+// map or the plugin's stored config map.
 func (t *TargetPlugin) getValue(config map[string]string, name string) (string, bool) {
 	v, ok := config[name]
 	if ok {
@@ -236,3 +241,14 @@ func (t *TargetPlugin) getValue(config map[string]string, name string) (string,
 
 	return "", false
 }
+
+// getConfigValue handles parameters that are optional in the operator's config 
+// but required for the plugin's functionality.
+func getConfigValue(config map[string]string, key string, defaultValue string) string {
+	value, ok := config[key]
+	if !ok {
+		return defaultValue
+	}
+
+	return value
+}

plugins/builtin/target/gce-mig/plugin/plugin_test.go

@@ -54,21 +54,36 @@ func TestTargetPlugin_calculateDirection(t *testing.T) {
 }
 
 func TestTargetPlugin_SetConfig_RetryAttempts(t *testing.T) {
-    p := NewGCEMIGPlugin(hclog.NewNullLogger())
+    // Test case for the default value.
+    t.Run("default value is used when not provided", func(t *testing.T) {
+        p := NewGCEMIGPlugin(hclog.NewNullLogger())
+        config := map[string]string{}
+        err := p.SetConfig(config)
+        assert.NoError(t, err)
+    
+        defaultValue, _ := strconv.Atoi(configValueRetryAttemptsDefault)
+        assert.Equal(t, defaultValue, p.retryAttempts)
+    })
 
-    assert.Equal(t, defaultRetryAttempts, p.retryAttempts, "expected default retry attempts")
+    // Test case for a valid custom retry attempts value.
+    t.Run("valid custom value is used", func(t *testing.T) {
+        p := NewGCEMIGPlugin(hclog.NewNullLogger())
+        customAttempts := 20
+        config := map[string]string{
+            configKeyRetryAttempts: strconv.Itoa(customAttempts),
+        }
+        err := p.SetConfig(config)
+        assert.NoError(t, err, "expected no error for valid config")
+        assert.Equal(t, customAttempts, p.retryAttempts, "expected custom retry attempts")
+    })
 
-    customAttempts := 20
-    config := map[string]string{
-        configKeyRetryAttempts: strconv.Itoa(customAttempts),
-    }
-    err := p.SetConfig(config)
-    assert.NoError(t, err, "expected no error for valid config")
-    assert.Equal(t, customAttempts, p.retryAttempts, "expected custom retry attempts")
-
-    invalidConfig := map[string]string{
-        configKeyRetryAttempts: "not-a-number",
-    }
-    err = p.SetConfig(invalidConfig)
-    assert.Error(t, err, "expected an error for invalid retry attempts")
+    // Test case for an invalid retry attempts value (non-integer).
+    t.Run("invalid value returns an error", func(t *testing.T) {
+        p := NewGCEMIGPlugin(hclog.NewNullLogger())
+        invalidConfig := map[string]string{
+            configKeyRetryAttempts: "not-a-number",
+        }
+        err := p.SetConfig(invalidConfig)
+        assert.Error(t, err, "expected an error for invalid retry attempts")
+    })
 }