fix: config selection improved logic (#54)

* fix: config selection improved logic Signed-off-by: Gosha <[email protected]> * fix: config selection improved logic Signed-off-by: Gosha <[email protected]> * fix: config selection improved logic Signed-off-by: Gosha <[email protected]> * fix: config selection improved logic Signed-off-by: Gosha <[email protected]> * fix: config selection improved logic Signed-off-by: Gosha <[email protected]> * fix: config selection improved logic Signed-off-by: Gosha <[email protected]> * fix: config selection improved logic Signed-off-by: Gosha <[email protected]> * fix: config selection improved logic Signed-off-by: Gosha <[email protected]> * fix: config selection improved logic Signed-off-by: Gosha <[email protected]> * fix: config selection improved logic Signed-off-by: Gosha <[email protected]> * fix: config selection improved logic Signed-off-by: Gosha <[email protected]> * fix: config selection improved logic Signed-off-by: Gosha <[email protected]> --------- Signed-off-by: Gosha <[email protected]> Signed-off-by: GoshaDozoretz <[email protected]>
Rookout · Jun 27, 2023 · 59f25dc · 59f25dc
1 parent 7d011c7
commit 59f25dc
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 27 deletions.
diff --git a/pkg/workflow_handler/workflows.go b/pkg/workflow_handler/workflows.go
@@ -49,8 +49,7 @@ func (wfc *WorkflowsClientImpl) ConstructTemplates(workflowsBatch *common.Workfl
 		return nil, err
 	}
 	if onExit == nil || len(onExit.DAG.Tasks) == 0 {
-		_, ok := wfc.cfg.WorkflowsConfig.Configs[configName]
-		if ok && wfc.cfg.WorkflowsConfig.Configs[configName].OnExit != nil {
+		if IsConfigExists(&wfc.cfg.WorkflowsConfig, configName) && IsConfigsOnExitExists(&wfc.cfg.WorkflowsConfig, configName) {
 			template := &v1alpha1.Template{
 				Name: ONEXIT,
 				DAG: &v1alpha1.DAGTemplate{
@@ -74,8 +73,7 @@ func (wfc *WorkflowsClientImpl) ConstructTemplates(workflowsBatch *common.Workfl
 
 func (wfc *WorkflowsClientImpl) ConstructSpec(templates []v1alpha1.Template, params []v1alpha1.Parameter, configName string) (*v1alpha1.WorkflowSpec, error) {
 	finalSpec := &v1alpha1.WorkflowSpec{}
-	_, ok := wfc.cfg.WorkflowsConfig.Configs[configName]
-	if ok {
+	if IsConfigExists(&wfc.cfg.WorkflowsConfig, configName) {
 		*finalSpec = wfc.cfg.WorkflowsConfig.Configs[configName].Spec
 		if len(wfc.cfg.WorkflowsConfig.Configs[configName].OnExit) != 0 {
 			finalSpec.OnExit = ONEXIT
@@ -110,35 +108,29 @@ func (wfc *WorkflowsClientImpl) CreateWorkflow(spec *v1alpha1.WorkflowSpec, work
 
 func (wfc *WorkflowsClientImpl) SelectConfig(workflowsBatch *common.WorkflowsBatch) (string, error) {
 	var configName string
-	ok := IsConfigExists(&wfc.cfg.WorkflowsConfig, "default")
-	if ok {
+	if IsConfigExists(&wfc.cfg.WorkflowsConfig, "default") {
 		configName = "default"
-		log.Printf(
-			"%s config selected for workflow in repo: %s branch %s",
-			configName,
-			workflowsBatch.Payload.Repo,
-			workflowsBatch.Payload.Branch,
-		) // Info
 	}
+
 	if *workflowsBatch.Config != "" {
-		ok = IsConfigExists(&wfc.cfg.WorkflowsConfig, *workflowsBatch.Config)
-		if ok {
+		if IsConfigExists(&wfc.cfg.WorkflowsConfig, *workflowsBatch.Config) {
 			configName = *workflowsBatch.Config
-			log.Printf(
-				"%s config overwrited for workflow in repo: %s branch %s",
-				*workflowsBatch.Config,
-				workflowsBatch.Payload.Repo,
-				workflowsBatch.Payload.Branch,
-			) // Info
 		} else {
-			log.Printf(
+			return configName, fmt.Errorf(
 				"error in selecting config, staying with default config for repo %s branch %s",
 				workflowsBatch.Payload.Repo,
 				workflowsBatch.Payload.Branch,
-			) // Error
+			)
 		}
 	}
 
+	log.Printf(
+		"%s config selected for workflow in repo: %s branch %s",
+		configName,
+		workflowsBatch.Payload.Repo,
+		workflowsBatch.Payload.Branch,
+	) // Info
+
 	return configName, nil
 }
 

diff --git a/pkg/workflow_handler/workflows_test.go b/pkg/workflow_handler/workflows_test.go
@@ -38,8 +38,8 @@ func TestSelectConfig(t *testing.T) {
 	returnConfigName, err := wfcImpl.SelectConfig(workflowsBatch)
 
 	// Assert the expected output
-	assert.Nil(err)
 	assert.Equal("default", returnConfigName)
+	assert.Nil(err)
 
 	// Test case 2
 	configName = "config1"
@@ -52,8 +52,8 @@ func TestSelectConfig(t *testing.T) {
 	returnConfigName, err = wfcImpl.SelectConfig(workflowsBatch)
 
 	// Assert the expected output
-	assert.Nil(err)
 	assert.Equal("config1", returnConfigName)
+	assert.Nil(err)
 
 	// Test case 3 - selection of non-existing config when default config exists
 	configName = "notInConfigs"
@@ -66,8 +66,8 @@ func TestSelectConfig(t *testing.T) {
 	returnConfigName, err = wfcImpl.SelectConfig(workflowsBatch)
 
 	// Assert the expected output
-	assert.Nil(err)
 	assert.Equal("default", returnConfigName)
+	assert.NotNil(err)
 
 	// Test case 4 - selection of non-existing config when default config not exists
 	configName = "notInConfig"
@@ -91,7 +91,6 @@ func TestSelectConfig(t *testing.T) {
 	returnConfigName, err = wfcImpl4.SelectConfig(workflowsBatch)
 
 	// Assert the expected output
-	assert.Nil(err)
 	assert.NotNil(returnConfigName)
-
+	assert.NotNil(err)
 }
diff --git a/pkg/workflow_handler/workflows_utils.go b/pkg/workflow_handler/workflows_utils.go
@@ -82,6 +82,10 @@ func IsConfigExists(cfg *conf.WorkflowsConfig, config string) bool {
 	return ok
 }
 
+func IsConfigsOnExitExists(cfg *conf.WorkflowsConfig, config string) bool {
+	return len(cfg.Configs[config].OnExit) != 0
+}
+
 func ValidateDAGTasks(tasks []v1alpha1.DAGTask) error {
 	for _, task := range tasks {
 		if task.Name == "" {