prow: sidecar: allow configuring ignore interrupts

viveksyngh · viveksyngh · commit e19dc5d23be6 · 2021-04-07T21:55:26.000+05:30
Add a field `ignore_interrupts` to decoration config spec to configure ignore interrupts option
for the sidecar.

Signed-off-by: Vivek Singh &lt;svivekkumar@vmware.com&gt;
diff --git a/prow/apis/prowjobs/v1/types.go b/prow/apis/prowjobs/v1/types.go
@@ -371,6 +371,10 @@ type DecorationConfig struct {
 	// CensorSecrets determines if the sidecar should be instructed to censor
 	// secret data that is otherwise mounted to the ProwJob Pod
 	CensorSecrets *bool `json:"censor_secrets,omitempty"`
+
+	// IgnoreInterrupts causes sidecar to ignore interrupts and
+	// hope that the test process exits cleanly before starting an upload.
+	IgnoreInterrupts *bool `json:"ignore_interrupts,omitempty"`
 }
 
 // Resources holds resource requests and limits for
@@ -469,6 +473,10 @@ func (d *DecorationConfig) ApplyDefault(def *DecorationConfig) *DecorationConfig
 		merged.CensorSecrets = def.CensorSecrets
 	}
 
+	if merged.IgnoreInterrupts == nil {
+		merged.IgnoreInterrupts = def.IgnoreInterrupts
+	}
+
 	return &merged
 }
 
diff --git a/prow/config/prow-config-documented.yaml b/prow/config/prow-config-documented.yaml
@@ -515,6 +515,10 @@ plank:
             # secret data that is otherwise mounted to the ProwJob Pod
             censor_secrets: false
 
+            # IgnoreInterrupts causes sidecar to ignore interrupts and
+            # hope that the test process exits cleanly before starting an upload.
+            ignore_interrupts: false
+
             # CookieFileSecret is the name of a kubernetes secret that contains
             # a git http.cookiefile, which should be used during the cloning process.
             cookiefile_secret: ' '
@@ -659,6 +663,15 @@ plank:
             # secret data that is otherwise mounted to the ProwJob Pod
             censor_secrets: false
 
+            # If `ignore_interrupts` is set, `sidecar` will do nothing upon receipt of
+            # the interrupt signal; this implicitly means that upload of logs and artifacts
+            # will begin when the test process exits, which may be as long as the grace
+            # period if the test process does not gracefully handle interrupts. This will
+            # require that the user configures the Pod's termination grace period to be
+            # longer than the `entrypoint` grace period for the test process and the time
+            # taken by `sidecar` to upload all relevant artifacts.
+            ignore_interrupts: false
+
             # CookieFileSecret is the name of a kubernetes secret that contains
             # a git http.cookiefile, which should be used during the cloning process.
             cookiefile_secret: ' '
diff --git a/prow/pod-utils/decorate/podspec.go b/prow/pod-utils/decorate/podspec.go
@@ -742,7 +742,14 @@ 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...)
+	var ignoreInterrupts bool
+	if pj.Spec.DecorationConfig.IgnoreInterrupts == nil { // if nil, set it to false
+		ignoreInterrupts = false
+	} else {
+		ignoreInterrupts = *pj.Spec.DecorationConfig.IgnoreInterrupts
+	}
+
+	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 +800,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 := false
 	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},
+						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,
+						IgnoreInterrupts:          &ignoreInterrupts,
+					},
+					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,114 @@
+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"}],"censoring_options":{}, "ignore_interrupts":true}'
+  image: sidecarimage
+  name: sidecar
+  resources:
+    limits:
+      cpu: "0"
+    requests:
+      memory: "0"
+  volumeMounts:
+  - mountPath: /logs
+    name: logs
+  - mountPath: /secrets/gcs
+    name: gcs-credentials
+  - mountPath: /secret
+    name: secret
+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":"ci-robot@k8s.io","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,10 @@ func (o Options) Run(ctx context.Context) (int, error) {
 		return 0, fmt.Errorf("could not resolve job spec: %v", err)
 	}
 
+	entries := o.entries()
+	buildLogs := logReaders(entries)
+	metadata := combineMetadata(entries)
+
 	ctx, cancel := context.WithCancel(ctx)
 
 	interrupt := make(chan os.Signal)
@@ -125,7 +129,12 @@ 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()
+
+				err = o.doUpload(spec, false, true, metadata, buildLogs)
+				if err != nil {
+					logrus.Errorf("Failed to perform best-effort upload: %v", err)
+				}
+
 			}
 		case <-ctx.Done():
 		}
@@ -135,7 +144,7 @@ func (o Options) Run(ctx context.Context) (int, error) {
 		// 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()
@@ -151,8 +160,7 @@ func (o Options) Run(ctx context.Context) (int, error) {
 			logrus.Warnf("Failed to censor data: %v", err)
 		}
 	}
-	buildLogs := logReaders(entries)
-	metadata := combineMetadata(entries)
+
 	return failures, o.doUpload(spec, passed, aborted, metadata, buildLogs)
 }
 

Original file line number	Diff line number	Diff line change
`@@ -371,6 +371,10 @@ type DecorationConfig struct {`
`371`	`371`	`// CensorSecrets determines if the sidecar should be instructed to censor`
`372`	`372`	`// secret data that is otherwise mounted to the ProwJob Pod`
`373`	`373`	CensorSecrets *bool `json:"censor_secrets,omitempty"`
	`374`	`+`
	`375`	`+ // IgnoreInterrupts causes sidecar to ignore interrupts and`
	`376`	`+ // hope that the test process exits cleanly before starting an upload.`
	`377`	+ IgnoreInterrupts *bool `json:"ignore_interrupts,omitempty"`
`374`	`378`	`}`
`375`	`379`
`376`	`380`	`// Resources holds resource requests and limits for`
`@@ -469,6 +473,10 @@ func (d DecorationConfig) ApplyDefault(def DecorationConfig) *DecorationConfig`
`469`	`473`	`merged.CensorSecrets = def.CensorSecrets`
`470`	`474`	`}`
`471`	`475`
	`476`	`+ if merged.IgnoreInterrupts == nil {`
	`477`	`+ merged.IgnoreInterrupts = def.IgnoreInterrupts`
	`478`	`+ }`
	`479`	`+`
`472`	`480`	`return &merged`
`473`	`481`	`}`
`474`	`482`
Original file line number	Diff line number	Diff line change
`@@ -108,6 +108,10 @@ func (o Options) Run(ctx context.Context) (int, error) {`
`108`	`108`	`return 0, fmt.Errorf("could not resolve job spec: %v", err)`
`109`	`109`	`}`
`110`	`110`
	`111`	`+ entries := o.entries()`
	`112`	`+ buildLogs := logReaders(entries)`
	`113`	`+ metadata := combineMetadata(entries)`
	`114`	`+`
`111`	`115`	`ctx, cancel := context.WithCancel(ctx)`
`112`	`116`
`113`	`117`	`interrupt := make(chan os.Signal)`
`@@ -125,7 +129,12 @@ func (o Options) Run(ctx context.Context) (int, error) {`
`125`	`129`	`// second upload but we can tolerate this as we'd rather get SOME`
`126`	`130`	`// data into GCS than attempt to cancel these uploads and get none.`
`127`	`131`	`logrus.Errorf("Received an interrupt: %s, cancelling...", s)`
`128`		`- cancel()`
	`132`	`+`
	`133`	`+ err = o.doUpload(spec, false, true, metadata, buildLogs)`
	`134`	`+ if err != nil {`
	`135`	`+ logrus.Errorf("Failed to perform best-effort upload: %v", err)`
	`136`	`+ }`
	`137`	`+`
`129`	`138`	`}`
`130`	`139`	`case <-ctx.Done():`
`131`	`140`	`}`
`@@ -135,7 +144,7 @@ func (o Options) Run(ctx context.Context) (int, error) {`
`135`	`144`	`// This only fires if the prowjob controller and sidecar are at different commits`
`136`	`145`	`logrus.Warnf("Using deprecated wrapper_options instead of entries. Please update prow/pod-utils/decorate before June 2019")`
`137`	`146`	`}`
`138`		`- entries := o.entries()`
	`147`	`+`
`139`	`148`	`passed, aborted, failures := wait(ctx, entries)`
`140`	`149`
`141`	`150`	`cancel()`
`@@ -151,8 +160,7 @@ func (o Options) Run(ctx context.Context) (int, error) {`
`151`	`160`	`logrus.Warnf("Failed to censor data: %v", err)`
`152`	`161`	`}`
`153`	`162`	`}`
`154`		`- buildLogs := logReaders(entries)`
`155`		`- metadata := combineMetadata(entries)`
	`163`	`+`
`156`	`164`	`return failures, o.doUpload(spec, passed, aborted, metadata, buildLogs)`
`157`	`165`	`}`
`158`	`166`