Repository: ovh/cds
Pull request #4900 (2020-01-20 11:29:30)

fix(api): remove only dead job
1. Description
1. Related issues
1. About tests
1. Mentions

@ovh/cds


ovh-cds: 
CDS Report build-all-cds#12113.0 ✘  
* Build
  * Build and Package All ✔ 
  * Build Debpacker ✔ 
  * Build OS-Ansible-Inventory ✔ 
  * Build SMTPMock ✔ 
  * Lint UI Translate ✔ 
  * Test CLI ✔ 
  * Test Engine ✔ 
  * Test SDK ✔ 
  * Test UI ✔ 
* Package
  * Docker ✔ 
  * Docs ✔ 
* Integration
  * Docker Compose Tests ✔ 
  * Helm Tests -   
  * Standalone Tests ✘  

ovh-cds: 
CDS Report build-all-cds#12113.1 ✘  
* Build
  * Build and Package All ✔ 
  * Build Debpacker ✔ 
  * Build OS-Ansible-Inventory ✔ 
  * Build SMTPMock ✔ 
  * Lint UI Translate ✔ 
  * Test CLI ✔ 
  * Test Engine ✔ 
  * Test SDK ✔ 
  * Test UI ✔ 
* Package
  * Docker ✔ 
  * Docs ✔ 
* Integration
  * Docker Compose Tests ✔ 
  * Helm Tests -   
  * Standalone Tests ✘  

ovh-cds: 
CDS Report build-all-cds#12113.2 ✘  
* Build
  * Build and Package All ✔ 
  * Build Debpacker ✔ 
  * Build OS-Ansible-Inventory ✔ 
  * Build SMTPMock ✔ 
  * Lint UI Translate ✔ 
  * Test CLI ✔ 
  * Test Engine ✔ 
  * Test SDK ✔ 
  * Test UI ✔ 
* Package
  * Docker ✔ 
  * Docs ✔ 
* Integration
  * Docker Compose Tests ✔ 
  * Helm Tests -   
  * Standalone Tests ✘  


Patch:
diff --git a/engine/api/workflow/dao_node_run_job.go b/engine/api/workflow/dao_node_run_job.go
index 21a894910..f4771730f 100644
--- a/engine/api/workflow/dao_node_run_job.go
+++ b/engine/api/workflow/dao_node_run_job.go
@@ -343,6 +343,13 @@ func DeleteNodeJobRuns(db gorp.SqlExecutor, nodeID int64) error {
 	return err
 }
 
+// DeleteNodeJobRun deletes the given workflow_node_run_job
+func DeleteNodeJobRun(db gorp.SqlExecutor, nodeRunJob int64) error {
+	query := `delete from workflow_node_run_job where id = $1`
+	_, err := db.Exec(query, nodeRunJob)
+	return err
+}
+
 //UpdateNodeJobRun updates a workflow_node_run_job
 func UpdateNodeJobRun(ctx context.Context, db gorp.SqlExecutor, j *sdk.WorkflowNodeJobRun) error {
 	var end func()
diff --git a/engine/api/workflow/heartbeat.go b/engine/api/workflow/heartbeat.go
index 1edf5f757..c71c02d1a 100644
--- a/engine/api/workflow/heartbeat.go
+++ b/engine/api/workflow/heartbeat.go
@@ -35,7 +35,7 @@ func manageDeadJob(ctx context.Context, DBFunc func() *gorp.DbMap, store cache.S
 					continue
 				}
 
-				if err := DeleteNodeJobRuns(tx, deadJob.WorkflowNodeRunID); err != nil {
+				if err := DeleteNodeJobRun(tx, deadJob.ID); err != nil {
 					log.Error(ctx, "manageDeadJob> Cannot delete node run job %d : %v", deadJob.ID, err)
 					_ = tx.Rollback()
 					continue
@@ -48,7 +48,7 @@ func manageDeadJob(ctx context.Context, DBFunc func() *gorp.DbMap, store cache.S
 				}
 			}
 		} else if sdk.StatusIsTerminated(deadJob.Status) {
-			if err := DeleteNodeJobRuns(tx, deadJob.WorkflowNodeRunID); err != nil {
+			if err := DeleteNodeJobRun(tx, deadJob.ID); err != nil {
 				log.Error(ctx, "manageDeadJob> Cannot delete node run job %d : %v", deadJob.ID, err)
 				_ = tx.Rollback()
 				continue
diff --git a/engine/worker/cmd_upload.go b/engine/worker/cmd_upload.go
index ce935f831..f7f7c94f7 100644
--- a/engine/worker/cmd_upload.go
+++ b/engine/worker/cmd_upload.go
@@ -88,7 +88,7 @@ func uploadCmd() func(cmd *cobra.Command, args []string) {
 			}
 
 			if resp.StatusCode >= 300 {
-				sdk.Exit("cannot artefact upload HTTP %d\n", resp.StatusCode)
+				sdk.Exit("cannot artifact upload HTTP %d\n", resp.StatusCode)
 			}
 		}
 	}
diff --git a/engine/worker/internal/action/builtin_artifact_upload.go b/engine/worker/internal/action/builtin_artifact_upload.go
index 342a67f45..0a5624da3 100644
--- a/engine/worker/internal/action/builtin_artifact_upload.go
+++ b/engine/worker/internal/action/builtin_artifact_upload.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"path"
 	"path/filepath"
 	"strings"
 	"sync"
@@ -42,7 +41,7 @@ func RunArtifactUpload(ctx context.Context, wk workerruntime.Runtime, a sdk.Acti
 		abs = workdir.Name()
 	}
 
-	if !path.IsAbs(artifactPath) {
+	if !sdk.PathIsAbs(artifactPath) {
 		artifactPath = filepath.Join(abs, artifactPath)
 	}
 
diff --git a/engine/worker/internal/action/builtin_coverage.go b/engine/worker/internal/action/builtin_coverage.go
index 9580f36e7..9e97c5947 100644
--- a/engine/worker/internal/action/builtin_coverage.go
+++ b/engine/worker/internal/action/builtin_coverage.go
@@ -3,7 +3,6 @@ package action
 import (
 	"context"
 	"fmt"
-	"path"
 	"path/filepath"
 	"strconv"
 
@@ -64,7 +63,7 @@ func RunParseCoverageResultAction(ctx context.Context, wk workerruntime.Runtime,
 		abs = workdir.Name()
 	}
 
-	if !path.IsAbs(p) {
+	if !sdk.PathIsAbs(p) {
 		fpath = filepath.Join(abs, p)
 	} else {
 		fpath = p
diff --git a/engine/worker/internal/action/builtin_junit.go b/engine/worker/internal/action/builtin_junit.go
index 8a461401d..a16277537 100644
--- a/engine/worker/internal/action/builtin_junit.go
+++ b/engine/worker/internal/action/builtin_junit.go
@@ -5,7 +5,6 @@ import (
 	"encoding/xml"
 	"errors"
 	"fmt"
-	"path"
 	"path/filepath"
 
 	"github.com/spf13/afero"
@@ -43,7 +42,7 @@ func RunParseJunitTestResultAction(ctx context.Context, wk workerruntime.Runtime
 		abs = workdir.Name()
 	}
 
-	if !path.IsAbs(p) {
+	if !sdk.PathIsAbs(p) {
 		p = filepath.Join(abs, p)
 	}
 
diff --git a/engine/worker/internal/handler_upload.go b/engine/worker/internal/handler_upload.go
index e4c47435f..8db4e24e9 100644
--- a/engine/worker/internal/handler_upload.go
+++ b/engine/worker/internal/handler_upload.go
@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
-	"path"
 	"path/filepath"
 
 	"github.com/ovh/cds/engine/worker/internal/action"
@@ -31,7 +30,7 @@ func uploadHandler(ctx context.Context, wk *CurrentWorker) http.HandlerFunc {
 		}
 
 		artifactPath := art.Name
-		if !path.IsAbs(artifactPath) {
+		if !sdk.PathIsAbs(artifactPath) {
 			artifactPath = filepath.Join(art.WorkingDirectory, art.Name)
 		}
 
diff --git a/engine/worker/internal/run.go b/engine/worker/internal/run.go
index dceea93b9..c58976a6f 100644
--- a/engine/worker/internal/run.go
+++ b/engine/worker/internal/run.go
@@ -126,8 +126,11 @@ func (w *CurrentWorker) runJob(ctx context.Context, a *sdk.Action, jobID int64,
 			// Check if all newVariables are in currentJob.params
 			// variable can be add in w.currentJob.newVariables by worker command export
 			for _, newVariableFromHandler := range w.currentJob.newVariables {
-				if sdk.ParameterFind(w.currentJob.params, newVariableFromHandler.Name) == nil {
+				p := sdk.ParameterFind(w.currentJob.params, newVariableFromHandler.Name)
+				if p == nil {
 					w.currentJob.params = append(w.currentJob.params, newVariableFromHandler.ToParameter(""))
+				} else {
+					p.Value = newVariableFromHandler.Value
 				}
 			}
 
@@ -270,8 +273,11 @@ func (w *CurrentWorker) runSteps(ctx context.Context, steps []sdk.Action, a sdk.
 		// Check if all newVariables are in currentJob.params
 		// variable can be add in w.currentJob.newVariables by worker command export
 		for _, newVariableFromHandler := range w.currentJob.newVariables {
-			if sdk.ParameterFind(w.currentJob.params, newVariableFromHandler.Name) == nil {
+			p := sdk.ParameterFind(w.currentJob.params, newVariableFromHandler.Name)
+			if p == nil {
 				w.currentJob.params = append(w.currentJob.params, newVariableFromHandler.ToParameter(""))
+			} else {
+				p.Value = newVariableFromHandler.Value
 			}
 		}
 
diff --git a/sdk/cache.go b/sdk/cache.go
index 9afaac51e..d1be77ca3 100644
--- a/sdk/cache.go
+++ b/sdk/cache.go
@@ -7,7 +7,6 @@ import (
 	"io"
 	"net/url"
 	"os"
-	"path"
 	"path/filepath"
 	"strings"
 
@@ -58,7 +57,7 @@ func CreateTarFromPaths(fs afero.Fs, cwd string, paths []string, opts *TarOption
 		// ensure the src actually exists before trying to tar it
 
 		completePath := p
-		if !path.IsAbs(p) {
+		if !PathIsAbs(p) {
 			completePath = filepath.Join(cwd, p)
 		}
 
@@ -85,14 +84,13 @@ func CreateTarFromPaths(fs afero.Fs, cwd string, paths []string, opts *TarOption
 				header.Name = strings.TrimPrefix(strings.TrimPrefix(header.Name, opts.TrimDirName), string(filepath.Separator))
 			}
 			if fi.Mode()&os.ModeSymlink != 0 {
+
 				symlink, errEval := filepath.EvalSymlinks(file)
 				if errEval != nil {
 					return errEval
 				}
-				abs, errAbs := filepath.Abs(header.Name)
-				if errAbs != nil {
-					return errAbs
-				}
+
+				abs := filepath.Dir(filepath.Join(cwd, header.Name))
 
 				symlinkRel, errRel := filepath.Rel(abs, symlink)
 				if errRel != nil {
diff --git a/sdk/common.go b/sdk/common.go
index dfe3fc5da..02f9e31ef 100644
--- a/sdk/common.go
+++ b/sdk/common.go
@@ -11,8 +11,10 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path"
 	"reflect"
 	"regexp"
+	"runtime"
 	"strings"
 	"unicode"
 
@@ -269,3 +271,12 @@ func RemoveNotPrintableChar(in string) string {
 	}
 	return strings.Map(m, in)
 }
+
+var windowsPathRegex = regexp.MustCompile(`^[a-zA-Z]:\\[\\\S|*\S]?.*$`)
+
+func PathIsAbs(s string) bool {
+	if GOOS == "windows" || runtime.GOOS == "windows" {
+		return windowsPathRegex.MatchString(s)
+	}
+	return path.IsAbs(s)
+}


Test patch:
diff --git a/sdk/common_test.go b/sdk/common_test.go
index 595b67a4a..acd62275e 100644
--- a/sdk/common_test.go
+++ b/sdk/common_test.go
@@ -32,3 +32,12 @@ func TestRemoveNotPrintableChar(t *testing.T) {
 		})
 	}
 }
+
+func TestPathIsAbs(t *testing.T) {
+	GOOS = "windows"
+	assert.True(t, PathIsAbs(`C:\Program Files (x86)\Foo`))
+	assert.False(t, PathIsAbs(`Program Files (x86)\Foo`))
+	GOOS = "linux"
+	assert.True(t, PathIsAbs(`/tmp`))
+	assert.False(t, PathIsAbs(`tmp`))
+}
diff --git a/tests/fixtures/04SCWorkflowRunWorkerCmd/pipeline.yml b/tests/fixtures/04SCWorkflowRunWorkerCmd/pipeline.yml
index 9960a9485..d6fcd5273 100644
--- a/tests/fixtures/04SCWorkflowRunWorkerCmd/pipeline.yml
+++ b/tests/fixtures/04SCWorkflowRunWorkerCmd/pipeline.yml
@@ -15,6 +15,8 @@ jobs:
       - this a a line in the file, with a CDS variable of type password {{.cds.app.password}}
       - EOF
       - worker check-secret {{.cds.workspace}}/myFile
+    - script:
+      - sleep 5
   requirements:
   - os-architecture: linux/amd64
 - job: EXIT
@@ -62,6 +64,8 @@ jobs:
       - EOF
       - worker tmpl {{.cds.workspace}}/myFile {{.cds.workspace}}/outputFile
       - cat {{.cds.workspace}}/outputFile
+    - script:
+        - sleep 5
   requirements:
   - os-architecture: linux/amd64
 - job: UPLOAD
@@ -82,6 +86,8 @@ jobs:
       - set -ex
       - worker key install --env proj-ssh-04scworkflowrunworkercmd --file id_rsa
       - cat id_rsa
+    - script:
+        - sleep 5
   requirements:
   - os-architecture: linux/amd64
 - job: ARTIFACTLIST
@@ -90,6 +96,7 @@ jobs:
     - script:
       - set -ex
       - worker artifacts
+    - script:
       - sleep 5
   requirements:
   - os-architecture: linux/amd64
@@ -100,6 +107,7 @@ jobs:
       - set -ex
       - worker cache pull latest
       - cat fileCache
+    - script:
       - sleep 5
   requirements:
   - os-architecture: linux/amd64
@@ -110,6 +118,7 @@ jobs:
       - set -ex
       - worker download
       - cat myFileUploaded
+    - script:
       - sleep 5
   requirements:
   - os-architecture: linux/amd64
@@ -119,6 +128,7 @@ jobs:
     - script:
       - set -ex
       - echo "Export value {{.cds.build.foo}}"
+    - script:
       - sleep 5
   requirements:
   - os-architecture: linux/amd64