Merge pull request #808 from hashicorp/base-controller-5

Split out baseController from TasksManager pt2

config/merge.go

@@ -0,0 +1,45 @@
+package config
+
+func mergeMaps(c, o map[string]interface{}) map[string]interface{} {
+	r := make(map[string]interface{})
+	for k, v := range c {
+		r[k] = v
+	}
+
+	for k, v := range o {
+		r[k] = v
+	}
+
+	return r
+}
+
+func mergeSlices(orig, incoming []string) []string {
+	if orig == nil {
+		if incoming == nil {
+			return nil
+		}
+		return incoming
+	}
+
+	if incoming == nil {
+		return orig
+	}
+
+	for _, incomingVal := range incoming {
+		// only add an incoming value if it does not already exist in the
+		// original slice
+		exists := false
+		for _, origVal := range orig {
+			if incomingVal == origVal {
+				exists = true
+				break
+			}
+		}
+
+		if !exists {
+			orig = append(orig, incomingVal)
+		}
+	}
+
+	return orig
+}

config/monitor_services.go

@@ -102,7 +102,7 @@ func (c *ServicesMonitorConfig) Merge(o MonitorConfig) MonitorConfig {
 		r2.Regexp = StringCopy(o2.Regexp)
 	}
 
-	r2.Names = append(r2.Names, o2.Names...)
+	r2.Names = mergeSlices(r2.Names, o2.Names)
 
 	if o2.Datacenter != nil {
 		r2.Datacenter = StringCopy(o2.Datacenter)

config/monitor_services_test.go

@@ -131,6 +131,12 @@ func TestServicesMonitorConfig_Merge(t *testing.T) {
 			&ServicesMonitorConfig{Names: []string{"b"}},
 			&ServicesMonitorConfig{Names: []string{"a", "b"}},
 		},
+		{
+			"names_same_merges",
+			&ServicesMonitorConfig{Names: []string{"a"}},
+			&ServicesMonitorConfig{Names: []string{"a"}},
+			&ServicesMonitorConfig{Names: []string{"a"}},
+		},
 		{
 			"names_empty_one",
 			&ServicesMonitorConfig{Names: []string{"service"}},

config/task.go

@@ -181,9 +181,9 @@ func (c *TaskConfig) Merge(o *TaskConfig) *TaskConfig {
 		r.Name = StringCopy(o.Name)
 	}
 
-	r.Providers = append(r.Providers, o.Providers...)
+	r.Providers = mergeSlices(r.Providers, o.Providers)
 
-	r.DeprecatedServices = append(r.DeprecatedServices, o.DeprecatedServices...)
+	r.DeprecatedServices = mergeSlices(r.DeprecatedServices, o.DeprecatedServices)
 
 	if o.Module != nil {
 		r.Module = StringCopy(o.Module)
@@ -199,7 +199,7 @@ func (c *TaskConfig) Merge(o *TaskConfig) *TaskConfig {
 		r.DeprecatedSourceInputs = r.DeprecatedSourceInputs.Merge(o.DeprecatedSourceInputs)
 	}
 
-	r.VarFiles = append(r.VarFiles, o.VarFiles...)
+	r.VarFiles = mergeSlices(r.VarFiles, o.VarFiles)
 
 	for k, v := range o.Variables {
 		r.Variables[k] = v

config/task_test.go

@@ -169,6 +169,12 @@ func TestTaskConfig_Merge(t *testing.T) {
 			&TaskConfig{DeprecatedServices: []string{"b"}},
 			&TaskConfig{DeprecatedServices: []string{"a", "b"}},
 		},
+		{
+			"services_same_merges",
+			&TaskConfig{DeprecatedServices: []string{"a"}},
+			&TaskConfig{DeprecatedServices: []string{"a"}},
+			&TaskConfig{DeprecatedServices: []string{"a"}},
+		},
 		{
 			"services_empty_one",
 			&TaskConfig{DeprecatedServices: []string{"service"}},
@@ -187,6 +193,12 @@ func TestTaskConfig_Merge(t *testing.T) {
 			&TaskConfig{Providers: []string{"b"}},
 			&TaskConfig{Providers: []string{"a", "b"}},
 		},
+		{
+			"providers_same_merges",
+			&TaskConfig{Providers: []string{"a"}},
+			&TaskConfig{Providers: []string{"a"}},
+			&TaskConfig{Providers: []string{"a"}},
+		},
 		{
 			"providers_empty_one",
 			&TaskConfig{Providers: []string{"provider"}},
@@ -199,6 +211,30 @@ func TestTaskConfig_Merge(t *testing.T) {
 			&TaskConfig{Providers: []string{"provider"}},
 			&TaskConfig{Providers: []string{"provider"}},
 		},
+		{
+			"varfiles_merges",
+			&TaskConfig{VarFiles: []string{"a.tfvars"}},
+			&TaskConfig{VarFiles: []string{"b.tfvars"}},
+			&TaskConfig{VarFiles: []string{"a.tfvars", "b.tfvars"}},
+		},
+		{
+			"varfiles_same_merges",
+			&TaskConfig{VarFiles: []string{"a.tfvars"}},
+			&TaskConfig{VarFiles: []string{"a.tfvars"}},
+			&TaskConfig{VarFiles: []string{"a.tfvars"}},
+		},
+		{
+			"varfiles_empty_one",
+			&TaskConfig{VarFiles: []string{"a.tfvars"}},
+			&TaskConfig{},
+			&TaskConfig{VarFiles: []string{"a.tfvars"}},
+		},
+		{
+			"varfiles_empty_two",
+			&TaskConfig{},
+			&TaskConfig{VarFiles: []string{"a.tfvars"}},
+			&TaskConfig{VarFiles: []string{"a.tfvars"}},
+		},
 		{
 			"source_overrides",
 			&TaskConfig{DeprecatedSource: String("path")},

config/terraform.go

@@ -318,16 +318,3 @@ func (c *TerraformConfig) IsConsulBackend() bool {
 	_, ok := c.Backend["consul"]
 	return ok
 }
-
-func mergeMaps(c, o map[string]interface{}) map[string]interface{} {
-	r := make(map[string]interface{})
-	for k, v := range c {
-		r[k] = v
-	}
-
-	for k, v := range o {
-		r[k] = v
-	}
-
-	return r
-}

controller/base.go

@@ -18,7 +18,6 @@ import (
 
 type baseController struct {
 	newDriver func(*config.Config, *driver.Task, templates.Watcher) (driver.Driver, error)
-	drivers   *driver.Drivers
 	watcher   templates.Watcher
 	resolver  templates.Resolver
 	logger    logging.Logger
@@ -39,7 +38,6 @@ func newBaseController(conf *config.Config, watcher templates.Watcher) (*baseCon
 
 	return &baseController{
 		newDriver: nd,
-		drivers:   driver.NewDrivers(),
 		watcher:   watcher,
 		resolver:  hcat.NewResolver(),
 		logger:    logger,
@@ -48,7 +46,7 @@ func newBaseController(conf *config.Config, watcher templates.Watcher) (*baseCon
 }
 
 func (ctrl *baseController) init(ctx context.Context) error {
-	ctrl.logger.Info("initializing driver")
+	ctrl.logger.Info("initializing base controller")
 
 	// Load provider configuration and evaluate dynamic values
 	var err error
@@ -57,33 +55,6 @@ func (ctrl *baseController) init(ctx context.Context) error {
 		return err
 	}
 
-	// Future: improve by combining tasks into workflows.
-	ctrl.logger.Info("initializing all tasks")
-	ctrl.drivers.Reset()
-
-	// Create and initialize task drivers
-	for _, t := range *ctrl.initConf.Tasks {
-		select {
-		case <-ctx.Done():
-			// Stop initializing remaining tasks if context has stopped.
-			return ctx.Err()
-		default:
-		}
-
-		d, err := ctrl.makeDriver(ctx, ctrl.initConf, *t)
-		if err != nil {
-			return err
-		}
-
-		taskName := *t.Name
-		err = ctrl.drivers.Add(taskName, d)
-		if err != nil {
-			ctrl.logger.Error("error adding task driver to drivers list", taskNameLogKey, taskName)
-			return err
-		}
-	}
-
-	ctrl.logger.Info("drivers initialized")
 	return nil
 }
 

controller/base_test.go

@@ -18,59 +18,45 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestBaseControllerInit(t *testing.T) {
+func TestBaseController_init(t *testing.T) {
 	t.Parallel()
 
 	conf := singleTaskConfig()
 
 	cases := []struct {
 		name        string
 		expectError bool
-		initTaskErr error
 		config      *config.Config
+		// TODO: after baseController is refactored to driers package, add
+		// check for `expectedProviders` field. Need to wait because
+		// driver.TerraformProviderBlock fields are private
+
+		// expectedProviders []driver.TerraformProviderBlock
 	}{
-		{
-			"error on driver.InitTask()",
-			true,
-			errors.New("error on driver.InitTask()"),
-			conf,
-		},
 		{
 			"happy path",
 			false,
-			nil,
 			conf,
 		},
 	}
 
 	ctx := context.Background()
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			d := new(mocksD.Driver)
-			d.On("TemplateIDs").Return(nil)
-			d.On("InitTask", mock.Anything).Return(tc.initTaskErr).Once()
-			if tc.expectError {
-				d.On("DestroyTask", mock.Anything).Return().Once()
-			}
-
 			baseCtrl := baseController{
-				newDriver: func(*config.Config, *driver.Task, templates.Watcher) (driver.Driver, error) {
-					return d, nil
-				},
-				drivers:  driver.NewDrivers(),
 				initConf: tc.config,
 				logger:   logging.NewNullLogger(),
 			}
-			err := baseCtrl.drivers.Add("task", d)
-			require.NoError(t, err)
 
-			err = baseCtrl.init(ctx)
+			err := baseCtrl.init(ctx)
 
 			if tc.expectError {
 				require.Error(t, err)
 				assert.Contains(t, err.Error(), tc.name)
 			} else {
-				assert.NoError(t, err)
+				require.NoError(t, err)
+				// TODO: uncomment assert after refactor to driver package
+				// assert.Equal(t, tc.expectedProviders, baseCtrl.providers)
 			}
 		})
 	}