prow: sidecar: allow configuring ignore interrupts

Add a field `upload_ignores_interrupts` to decoration config spec to configure ignore interrupts while performing upload. It also tries to peform best-effort upload when `upload_ignores_interrupts` is set to `false` and interrupt is received. While performing best-effort upload if main upload is ready it interrupts best-effort upload and switches to main upload. Signed-off-by: Vivek Singh <[email protected]>
kubernetes · Apr 13, 2021 · 7a71b58 · 7a71b58
1 parent 57fae23
commit 7a71b58
Show file tree

Hide file tree

Showing 8 changed files with 256 additions and 14 deletions.
diff --git a/prow/apis/prowjobs/v1/types.go b/prow/apis/prowjobs/v1/types.go
@@ -373,6 +373,10 @@ type DecorationConfig struct {
 
 	// CensoringOptions exposes options for censoring output logs and artifacts.
 	CensoringOptions *CensoringOptions `json:"censoring_options,omitempty"`
+
+	// UploadIgnoresInterrupts causes sidecar to ignore interrupts for the upload process in
+	// hope that the test process exits cleanly before starting an upload.
+	UploadIgnoresInterrupts *bool `json:"upload_ignores_interrupts,omitempty"`
 }
 
 type CensoringOptions struct {
@@ -434,6 +438,7 @@ func (g *CensoringOptions) ApplyDefault(def *CensoringOptions) *CensoringOptions
 		merged.ExcludeDirectories = def.ExcludeDirectories
 	}
 	return &merged
+
 }
 
 // Resources holds resource requests and limits for
@@ -533,6 +538,10 @@ func (d *DecorationConfig) ApplyDefault(def *DecorationConfig) *DecorationConfig
 		merged.CensorSecrets = def.CensorSecrets
 	}
 
+	if merged.UploadIgnoresInterrupts == nil {
+		merged.UploadIgnoresInterrupts = def.UploadIgnoresInterrupts
+	}
+
 	return &merged
 }
 

diff --git a/prow/apis/prowjobs/v1/types_test.go b/prow/apis/prowjobs/v1/types_test.go
@@ -205,6 +205,26 @@ func TestDecorationDefaultingDoesntOverwrite(t *testing.T) {
 				return def
 			},
 		},
+		{
+			name: "ingnore interrupts set",
+			provided: &DecorationConfig{
+				UploadIgnoresInterrupts: &truth,
+			},
+			expected: func(orig, def *DecorationConfig) *DecorationConfig {
+				def.UploadIgnoresInterrupts = orig.UploadIgnoresInterrupts
+				return def
+			},
+		},
+		{
+			name: "do not ingnore interrupts ",
+			provided: &DecorationConfig{
+				UploadIgnoresInterrupts: &lies,
+			},
+			expected: func(orig, def *DecorationConfig) *DecorationConfig {
+				def.UploadIgnoresInterrupts = orig.UploadIgnoresInterrupts
+				return def
+			},
+		},
 	}
 
 	for _, testCase := range testCases {

diff --git a/prow/apis/prowjobs/v1/zz_generated.deepcopy.go b/prow/apis/prowjobs/v1/zz_generated.deepcopy.go
diff --git a/prow/config/prow-config-documented.yaml b/prow/config/prow-config-documented.yaml
@@ -670,6 +670,10 @@ plank:
             # before aborting a job with SIGINT.
             timeout: 0s
 
+            # UploadIgnoresInterrupts causes sidecar to ignore interrupts for the upload process in
+            # hope that the test process exits cleanly before starting an upload.
+            upload_ignores_interrupts: false
+
             # UtilityImages holds pull specs for utility container
             # images used to decorate a PodSpec.
             utility_images:
@@ -843,6 +847,10 @@ plank:
             # before aborting a job with SIGINT.
             timeout: 0s
 
+            # UploadIgnoresInterrupts causes sidecar to ignore interrupts for the upload process in
+            # hope that the test process exits cleanly before starting an upload.
+            upload_ignores_interrupts: false
+
             # UtilityImages holds pull specs for utility container
             # images used to decorate a PodSpec.
             utility_images:

diff --git a/prow/pod-utils/decorate/podspec.go b/prow/pod-utils/decorate/podspec.go
@@ -742,7 +742,9 @@ func decorate(spec *coreapi.PodSpec, pj *prowapi.ProwJob, rawEnv map[string]stri
 		wrappers = append(wrappers, *wrapperOptions)
 	}
 
-	sidecar, err := Sidecar(pj.Spec.DecorationConfig, blobStorageOptions, blobStorageMounts, logMount, outputMount, encodedJobSpec, !RequirePassingEntries, !IgnoreInterrupts, secretVolumeMounts, wrappers...)
+	ignoreInterrupts := pj.Spec.DecorationConfig.UploadIgnoresInterrupts != nil && *pj.Spec.DecorationConfig.UploadIgnoresInterrupts
+
+	sidecar, err := Sidecar(pj.Spec.DecorationConfig, blobStorageOptions, blobStorageMounts, logMount, outputMount, encodedJobSpec, !RequirePassingEntries, ignoreInterrupts, secretVolumeMounts, wrappers...)
 	if err != nil {
 		return fmt.Errorf("create sidecar: %v", err)
 	}
@@ -793,8 +795,6 @@ func DetermineWorkDir(baseDir string, refs []prowapi.Refs) string {
 const (
 	// RequirePassingEntries causes sidecar to return an error if any entry fails. Otherwise it exits cleanly so long as it can complete.
 	RequirePassingEntries = true
-	// IgnoreInterrupts causes sidecar to ignore interrupts and hope that the test process exits cleanly before starting an upload.
-	IgnoreInterrupts = true
 )
 
 func Sidecar(config *prowapi.DecorationConfig, gcsOptions gcsupload.Options, blobStorageMounts []coreapi.VolumeMount, logMount coreapi.VolumeMount, outputMount *coreapi.VolumeMount, encodedJobSpec string, requirePassingEntries, ignoreInterrupts bool, secretVolumeMounts []coreapi.VolumeMount, wrappers ...wrapper.Options) (*coreapi.Container, error) {

diff --git a/prow/pod-utils/decorate/podspec_test.go b/prow/pod-utils/decorate/podspec_test.go
@@ -1130,6 +1130,7 @@ func TestDecorate(t *testing.T) {
 	gCSCredentialsSecret := "gcs-secret"
 	defaultServiceAccountName := "default-sa"
 	censor := true
+	ignoreInterrupts := true
 	var testCases = []struct {
 		name      string
 		spec      *coreapi.PodSpec
@@ -1230,6 +1231,53 @@ func TestDecorate(t *testing.T) {
 			},
 			rawEnv: map[string]string{"custom": "env"},
 		},
+		{
+			name: "ignore interrupts in sidecar",
+			spec: &coreapi.PodSpec{
+				Volumes: []coreapi.Volume{
+					{Name: "secret", VolumeSource: coreapi.VolumeSource{Secret: &coreapi.SecretVolumeSource{SecretName: "secretname"}}},
+				},
+				Containers: []coreapi.Container{
+					{Name: "test", Command: []string{"/bin/ls"}, Args: []string{"-l", "-a"}, VolumeMounts: []coreapi.VolumeMount{{Name: "secret", MountPath: "/secret"}}},
+				},
+				ServiceAccountName: "tester",
+			},
+			pj: &prowapi.ProwJob{
+				Spec: prowapi.ProwJobSpec{
+					DecorationConfig: &prowapi.DecorationConfig{
+						Timeout:                 &prowapi.Duration{Duration: time.Minute},
+						GracePeriod:             &prowapi.Duration{Duration: time.Hour},
+						UploadIgnoresInterrupts: &ignoreInterrupts,
+						UtilityImages: &prowapi.UtilityImages{
+							CloneRefs:  "cloneimage",
+							InitUpload: "initimage",
+							Entrypoint: "entrypointimage",
+							Sidecar:    "sidecarimage",
+						},
+						Resources: &prowapi.Resources{
+							CloneRefs:       &coreapi.ResourceRequirements{Limits: coreapi.ResourceList{"cpu": resource.Quantity{}}, Requests: coreapi.ResourceList{"memory": resource.Quantity{}}},
+							InitUpload:      &coreapi.ResourceRequirements{Limits: coreapi.ResourceList{"cpu": resource.Quantity{}}, Requests: coreapi.ResourceList{"memory": resource.Quantity{}}},
+							PlaceEntrypoint: &coreapi.ResourceRequirements{Limits: coreapi.ResourceList{"cpu": resource.Quantity{}}, Requests: coreapi.ResourceList{"memory": resource.Quantity{}}},
+							Sidecar:         &coreapi.ResourceRequirements{Limits: coreapi.ResourceList{"cpu": resource.Quantity{}}, Requests: coreapi.ResourceList{"memory": resource.Quantity{}}},
+						},
+						GCSConfiguration: &prowapi.GCSConfiguration{
+							Bucket:       "bucket",
+							PathStrategy: "single",
+							DefaultOrg:   "org",
+							DefaultRepo:  "repo",
+						},
+						GCSCredentialsSecret:      &gCSCredentialsSecret,
+						DefaultServiceAccountName: &defaultServiceAccountName,
+					},
+					Refs: &prowapi.Refs{
+						Org: "org", Repo: "repo", BaseRef: "main", BaseSHA: "abcd1234",
+						Pulls: []prowapi.Pull{{Number: 1, SHA: "aksdjhfkds"}},
+					},
+					ExtraRefs: []prowapi.Refs{{Org: "other", Repo: "something", BaseRef: "release", BaseSHA: "sldijfsd"}},
+				},
+			},
+			rawEnv: map[string]string{"custom": "env"},
+		},
 	}
 
 	for _, testCase := range testCases {

diff --git a/prow/pod-utils/decorate/testdata/zz_fixture_TestDecorate_ignore_interrupts_in_sidecar.yaml b/prow/pod-utils/decorate/testdata/zz_fixture_TestDecorate_ignore_interrupts_in_sidecar.yaml
@@ -0,0 +1,112 @@
+containers:
+- command:
+  - /tools/entrypoint
+  env:
+  - name: ARTIFACTS
+    value: /logs/artifacts
+  - name: GOPATH
+    value: /home/prow/go
+  - name: custom
+    value: env
+  - name: ENTRYPOINT_OPTIONS
+    value: '{"timeout":60000000000,"grace_period":3600000000000,"artifact_dir":"/logs/artifacts","args":["/bin/ls","-l","-a"],"container_name":"test","process_log":"/logs/process-log.txt","marker_file":"/logs/marker-file.txt","metadata_file":"/logs/artifacts/metadata.json"}'
+  name: test
+  resources: {}
+  volumeMounts:
+  - mountPath: /secret
+    name: secret
+  - mountPath: /logs
+    name: logs
+  - mountPath: /tools
+    name: tools
+  - mountPath: /home/prow/go
+    name: code
+  workingDir: /home/prow/go/src/github.com/org/repo
+- command:
+  - /sidecar
+  env:
+  - name: JOB_SPEC
+  - name: SIDECAR_OPTIONS
+    value: '{"gcs_options":{"items":["/logs/artifacts"],"bucket":"bucket","path_strategy":"single","default_org":"org","default_repo":"repo","gcs_credentials_file":"/secrets/gcs/service-account.json","dry_run":false},"entries":[{"args":["/bin/ls","-l","-a"],"container_name":"test","process_log":"/logs/process-log.txt","marker_file":"/logs/marker-file.txt","metadata_file":"/logs/artifacts/metadata.json"}],"ignore_interrupts":true,"censoring_options":{}}'
+  image: sidecarimage
+  name: sidecar
+  resources:
+    limits:
+      cpu: "0"
+    requests:
+      memory: "0"
+  volumeMounts:
+  - mountPath: /logs
+    name: logs
+  - mountPath: /secrets/gcs
+    name: gcs-credentials
+initContainers:
+- command:
+  - /clonerefs
+  env:
+  - name: CLONEREFS_OPTIONS
+    value: '{"src_root":"/home/prow/go","log":"/logs/clone.json","git_user_name":"ci-robot","git_user_email":"[email protected]","refs":[{"org":"org","repo":"repo","base_ref":"main","base_sha":"abcd1234","pulls":[{"number":1,"author":"","sha":"aksdjhfkds"}]},{"org":"other","repo":"something","base_ref":"release","base_sha":"sldijfsd"}]}'
+  image: cloneimage
+  name: clonerefs
+  resources:
+    limits:
+      cpu: "0"
+    requests:
+      memory: "0"
+  volumeMounts:
+  - mountPath: /logs
+    name: logs
+  - mountPath: /home/prow/go
+    name: code
+  - mountPath: /tmp
+    name: clonerefs-tmp
+- command:
+  - /initupload
+  env:
+  - name: INITUPLOAD_OPTIONS
+    value: '{"bucket":"bucket","path_strategy":"single","default_org":"org","default_repo":"repo","gcs_credentials_file":"/secrets/gcs/service-account.json","dry_run":false,"log":"/logs/clone.json"}'
+  - name: JOB_SPEC
+  image: initimage
+  name: initupload
+  resources:
+    limits:
+      cpu: "0"
+    requests:
+      memory: "0"
+  volumeMounts:
+  - mountPath: /logs
+    name: logs
+  - mountPath: /secrets/gcs
+    name: gcs-credentials
+- args:
+  - /entrypoint
+  - /tools/entrypoint
+  command:
+  - /bin/cp
+  image: entrypointimage
+  name: place-entrypoint
+  resources:
+    limits:
+      cpu: "0"
+    requests:
+      memory: "0"
+  volumeMounts:
+  - mountPath: /tools
+    name: tools
+serviceAccountName: tester
+terminationGracePeriodSeconds: 4500
+volumes:
+- name: secret
+  secret:
+    secretName: secretname
+- emptyDir: {}
+  name: logs
+- emptyDir: {}
+  name: tools
+- name: gcs-credentials
+  secret:
+    secretName: gcs-secret
+- emptyDir: {}
+  name: clonerefs-tmp
+- emptyDir: {}
+  name: code
diff --git a/prow/sidecar/run.go b/prow/sidecar/run.go
@@ -108,6 +108,8 @@ func (o Options) Run(ctx context.Context) (int, error) {
 		return 0, fmt.Errorf("could not resolve job spec: %v", err)
 	}
 
+	entries := o.entries()
+
 	ctx, cancel := context.WithCancel(ctx)
 
 	interrupt := make(chan os.Signal)
@@ -125,17 +127,19 @@ func (o Options) Run(ctx context.Context) (int, error) {
 				// second upload but we can tolerate this as we'd rather get SOME
 				// data into GCS than attempt to cancel these uploads and get none.
 				logrus.Errorf("Received an interrupt: %s, cancelling...", s)
-				cancel()
+
+				// perform pre upload tasks
+				o.preUpload()
+
+				buildLogs := logReaders(entries)
+				metadata := combineMetadata(entries)
+				o.doBestEffortUpload(ctx, spec, false, true, metadata, buildLogs)
+
 			}
 		case <-ctx.Done():
 		}
 	}()
 
-	if o.DeprecatedWrapperOptions != nil {
-		// This only fires if the prowjob controller and sidecar are at different commits
-		logrus.Warnf("Using deprecated wrapper_options instead of entries. Please update prow/pod-utils/decorate before June 2019")
-	}
-	entries := o.entries()
 	passed, aborted, failures := wait(ctx, entries)
 
 	cancel()
@@ -146,11 +150,8 @@ func (o Options) Run(ctx context.Context) (int, error) {
 	// uploading, so we ignore the signals.
 	signal.Ignore(os.Interrupt, syscall.SIGTERM)
 
-	if o.CensoringOptions != nil {
-		if err := o.censor(); err != nil {
-			logrus.Warnf("Failed to censor data: %v", err)
-		}
-	}
+	o.preUpload()
+
 	buildLogs := logReaders(entries)
 	metadata := combineMetadata(entries)
 	return failures, o.doUpload(spec, passed, aborted, metadata, buildLogs)
@@ -213,6 +214,45 @@ func combineMetadata(entries []wrapper.Options) map[string]interface{} {
 	return metadata
 }
 
+//doBestEffortUpload performs best-effort upload and interrupts if main upload is ready
+func (o Options) doBestEffortUpload(ctx context.Context, spec *downwardapi.JobSpec, passed, aborted bool, metadata map[string]interface{}, logReaders map[string]io.Reader) {
+	errChan := make(chan error)
+	defer close(errChan)
+
+	go func() {
+		errChan <- o.doUpload(spec, passed, aborted, metadata, logReaders)
+	}()
+
+	for {
+		select {
+		case <-ctx.Done():
+			logrus.Infof("Interrupting best-effort upload as main upload is ready")
+			return
+		case err := <-errChan:
+			if err != nil {
+				logrus.Errorf("Failed to perform best-effort upload : %v", err)
+			} else {
+				logrus.Infof("Best-effort upload was successful")
+			}
+			return
+		}
+	}
+}
+
+//preUpload peforms steps required before actual upload
+func (o Options) preUpload() {
+	if o.DeprecatedWrapperOptions != nil {
+		// This only fires if the prowjob controller and sidecar are at different commits
+		logrus.Warnf("Using deprecated wrapper_options instead of entries. Please update prow/pod-utils/decorate before June 2019")
+	}
+
+	if o.CensoringOptions != nil {
+		if err := o.censor(); err != nil {
+			logrus.Warnf("Failed to censor data: %v", err)
+		}
+	}
+}
+
 func (o Options) doUpload(spec *downwardapi.JobSpec, passed, aborted bool, metadata map[string]interface{}, logReaders map[string]io.Reader) error {
 	uploadTargets := make(map[string]gcs.UploadFunc)