fix(executor): Correct usage of time.Duration. Fixes #5046 (#5049)

Signed-off-by: Alex Collins <[email protected]>
argoproj · Feb 10, 2021 · 21e137b · 21e137b
1 parent 19a34b1
commit 21e137b
Show file tree

Hide file tree

Showing 5 changed files with 7 additions and 7 deletions.
diff --git a/workflow/executor/common/common.go b/workflow/executor/common/common.go
@@ -96,7 +96,7 @@ func KillGracefully(ctx context.Context, c KubernetesClientInterface, containerI
 	if err != nil {
 		return err
 	}
-	err = WaitForTermination(ctx, c, containerID, terminationGracePeriodDuration*time.Second)
+	err = WaitForTermination(ctx, c, containerID, terminationGracePeriodDuration)
 	if err == nil {
 		log.Infof("ContainerID %q successfully killed", containerID)
 		return nil
@@ -106,7 +106,7 @@ func KillGracefully(ctx context.Context, c KubernetesClientInterface, containerI
 	if err != nil {
 		return err
 	}
-	err = WaitForTermination(ctx, c, containerID, terminationGracePeriodDuration*time.Second)
+	err = WaitForTermination(ctx, c, containerID, terminationGracePeriodDuration)
 	if err != nil {
 		return err
 	}

diff --git a/workflow/executor/common/common_test.go b/workflow/executor/common/common_test.go
@@ -169,5 +169,5 @@ func TestKillGracefully(t *testing.T) {
 	}
 	ctx := context.Background()
 	err := KillGracefully(ctx, mock, "container-id", 1)
-	assert.EqualError(t, err, "timeout after 1s")
+	assert.EqualError(t, err, "timeout after 1ns")
 }
diff --git a/workflow/executor/docker/docker.go b/workflow/executor/docker/docker.go
@@ -197,7 +197,7 @@ func (d *DockerExecutor) Kill(ctx context.Context, containerIDs []string, termin
 	select {
 	case err = <-waitCh:
 		// waitCmd completed
-	case <-time.After(terminationGracePeriodDuration * time.Second):
+	case <-time.After(terminationGracePeriodDuration):
 		log.Infof("Timed out (%ds) for containers to terminate gracefully. Killing forcefully", terminationGracePeriodDuration)
 		forceKillArgs := append([]string{"kill", "--signal", "KILL"}, containerIDs...)
 		forceKillCmd := exec.Command("docker", forceKillArgs...)

diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go
@@ -682,10 +682,10 @@ func (we *WorkflowExecutor) GetSecrets(ctx context.Context, namespace, name, key
 // GetTerminationGracePeriodDuration returns the terminationGracePeriodSeconds of podSpec in Time.Duration format
 func (we *WorkflowExecutor) GetTerminationGracePeriodDuration(ctx context.Context) (time.Duration, error) {
 	pod, err := we.getPod(ctx)
-	if err != nil {
+	if err != nil || pod.Spec.TerminationGracePeriodSeconds == nil {
 		return time.Duration(0), err
 	}
-	terminationGracePeriodDuration := time.Duration(*pod.Spec.TerminationGracePeriodSeconds)
+	terminationGracePeriodDuration := time.Second * time.Duration(*pod.Spec.TerminationGracePeriodSeconds)
 	return terminationGracePeriodDuration, nil
 }
 

diff --git a/workflow/executor/pns/pns.go b/workflow/executor/pns/pns.go
@@ -274,7 +274,7 @@ func (p *PNSExecutor) killContainer(containerID string, terminationGracePeriodDu
 	if err != nil {
 		log.Warnf("Failed to SIGTERM pid %d: %v", pid, err)
 	}
-	waitPIDOpts := executil.WaitPIDOpts{Timeout: terminationGracePeriodDuration * time.Second}
+	waitPIDOpts := executil.WaitPIDOpts{Timeout: terminationGracePeriodDuration}
 	err = executil.WaitPID(pid, waitPIDOpts)
 	if err == nil {
 		log.Infof("PID %d completed", pid)