diff --git a/prow/apis/prowjobs/v1/types.go b/prow/apis/prowjobs/v1/types.go index 878fd9f75988..5fabefdbb770 100644 --- 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 index 9f60ed701062..b03677bfc5f9 100644 --- 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 index c7b0e8ffa60f..b9b4d2eb386c 100644 --- a/prow/apis/prowjobs/v1/zz_generated.deepcopy.go +++ b/prow/apis/prowjobs/v1/zz_generated.deepcopy.go @@ -137,6 +137,11 @@ func (in *DecorationConfig) DeepCopyInto(out *DecorationConfig) { *out = new(CensoringOptions) (*in).DeepCopyInto(*out) } + if in.UploadIgnoresInterrupts != nil { + in, out := &in.UploadIgnoresInterrupts, &out.UploadIgnoresInterrupts + *out = new(bool) + **out = **in + } return } diff --git a/prow/cmd/gcsupload/main.go b/prow/cmd/gcsupload/main.go index ce7ece75fae9..aa5d780170a9 100644 --- a/prow/cmd/gcsupload/main.go +++ b/prow/cmd/gcsupload/main.go @@ -19,6 +19,8 @@ limitations under the License. package main import ( + "context" + "github.com/sirupsen/logrus" "k8s.io/test-infra/prow/pod-utils/downwardapi" "k8s.io/test-infra/prow/pod-utils/options" @@ -45,7 +47,8 @@ func main() { logrus.WithError(err).Fatal("Could not resolve job spec") } - if err := o.Run(spec, map[string]gcs.UploadFunc{}); err != nil { + ctx := context.Background() + if err := o.Run(ctx, spec, map[string]gcs.UploadFunc{}); err != nil { logrus.WithError(err).Fatal("Failed to upload to GCS") } } diff --git a/prow/config/prow-config-documented.yaml b/prow/config/prow-config-documented.yaml index c3cf4987e38c..c2f4e083d2ac 100644 --- 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/gcsupload/run.go b/prow/gcsupload/run.go index c554b4a20717..c1454fe90b1e 100644 --- a/prow/gcsupload/run.go +++ b/prow/gcsupload/run.go @@ -17,6 +17,7 @@ limitations under the License. package gcsupload import ( + "context" "fmt" "mime" "net/url" @@ -37,7 +38,7 @@ import ( // a parameter and will have the prefix prepended // to their destination in GCS, so the caller can // operate relative to the base of the GCS dir. -func (o Options) Run(spec *downwardapi.JobSpec, extra map[string]gcs.UploadFunc) error { +func (o Options) Run(ctx context.Context, spec *downwardapi.JobSpec, extra map[string]gcs.UploadFunc) error { logrus.WithField("options", o).Debug("Uploading to blob storage") for extension, mediaType := range o.GCSConfiguration.MediaTypes { @@ -57,12 +58,12 @@ func (o Options) Run(spec *downwardapi.JobSpec, extra map[string]gcs.UploadFunc) } if o.LocalOutputDir == "" { - if err := gcs.Upload(o.Bucket, o.StorageClientOptions.GCSCredentialsFile, o.StorageClientOptions.S3CredentialsFile, uploadTargets); err != nil { + if err := gcs.Upload(ctx, o.Bucket, o.StorageClientOptions.GCSCredentialsFile, o.StorageClientOptions.S3CredentialsFile, uploadTargets); err != nil { return fmt.Errorf("failed to upload to blob storage: %w", err) } logrus.Info("Finished upload to blob storage") } else { - if err := gcs.LocalExport(o.LocalOutputDir, uploadTargets); err != nil { + if err := gcs.LocalExport(ctx, o.LocalOutputDir, uploadTargets); err != nil { return fmt.Errorf("failed to copy files to %q: %w", o.LocalOutputDir, err) } logrus.Infof("Finished copying files to %q.", o.LocalOutputDir) diff --git a/prow/initupload/run.go b/prow/initupload/run.go index a1422ad47452..7cd8e51e3ff7 100644 --- a/prow/initupload/run.go +++ b/prow/initupload/run.go @@ -18,6 +18,7 @@ package initupload import ( "bytes" + "context" "encoding/json" "errors" "fmt" @@ -106,7 +107,8 @@ func (o Options) Run() error { uploadTargets[prowv1.StartedStatusFile] = gcs.DataUpload(bytes.NewReader(startedData)) - if err := o.Options.Run(spec, uploadTargets); err != nil { + ctx := context.Background() + if err := o.Options.Run(ctx, spec, uploadTargets); err != nil { return fmt.Errorf("failed to upload to blob storage: %v", err) } diff --git a/prow/pod-utils/decorate/podspec.go b/prow/pod-utils/decorate/podspec.go index 0e69d9655e34..9481691aa02f 100644 --- 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 index 03327ac007e0..94bed95ad3e1 100644 --- 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 new file mode 100644 index 000000000000..8b321ae96b68 --- /dev/null +++ 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":"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/pod-utils/gcs/upload.go b/prow/pod-utils/gcs/upload.go index 442ecb94c94a..b41c974db12b 100644 --- a/prow/pod-utils/gcs/upload.go +++ b/prow/pod-utils/gcs/upload.go @@ -44,7 +44,7 @@ const retryCount = 4 // Upload uploads all of the data in the // uploadTargets map to blob storage in parallel. The map is // keyed on blob storage path under the bucket -func Upload(bucket, gcsCredentialsFile, s3CredentialsFile string, uploadTargets map[string]UploadFunc) error { +func Upload(ctx context.Context, bucket, gcsCredentialsFile, s3CredentialsFile string, uploadTargets map[string]UploadFunc) error { parsedBucket, err := url.Parse(bucket) if err != nil { return fmt.Errorf("cannot parse bucket name %s: %w", bucket, err) @@ -53,7 +53,6 @@ func Upload(bucket, gcsCredentialsFile, s3CredentialsFile string, uploadTargets parsedBucket.Scheme = providers.GS } - ctx := context.Background() opener, err := pkgio.NewOpener(ctx, gcsCredentialsFile, s3CredentialsFile) if err != nil { return fmt.Errorf("new opener: %w", err) @@ -66,8 +65,7 @@ func Upload(bucket, gcsCredentialsFile, s3CredentialsFile string, uploadTargets // LocalExport copies all of the data in the uploadTargets map to local files in parallel. The map // is keyed on file path under the exportDir. -func LocalExport(exportDir string, uploadTargets map[string]UploadFunc) error { - ctx := context.Background() +func LocalExport(ctx context.Context, exportDir string, uploadTargets map[string]UploadFunc) error { opener, err := pkgio.NewOpener(ctx, "", "") if err != nil { return fmt.Errorf("new opener: %w", err) diff --git a/prow/pod-utils/gcs/upload_test.go b/prow/pod-utils/gcs/upload_test.go index 87a5bf54485b..4a458c15db59 100644 --- a/prow/pod-utils/gcs/upload_test.go +++ b/prow/pod-utils/gcs/upload_test.go @@ -143,7 +143,8 @@ func TestUploadWithRetries(t *testing.T) { } - err := Upload("", "", "", uploadFuncs) + ctx := context.Background() + err := Upload(ctx, "", "", "", uploadFuncs) isErrExpected := false for _, currentTestState := range currentTestStates { diff --git a/prow/sidecar/run.go b/prow/sidecar/run.go index 14ea7fa0f073..3c0cdf9e7466 100644 --- 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,25 @@ 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) + + //Peform best-effort upload + err := o.doUpload(ctx, spec, false, true, metadata, buildLogs) + if err != nil { + logrus.Errorf("Failed to perform best-effort upload : %v", err) + } else { + logrus.Errorf("Best-effort upload was successful") + } } 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,14 +156,11 @@ 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) + return failures, o.doUpload(context.Background(), spec, passed, aborted, metadata, buildLogs) } const errorKey = "sidecar-errors" @@ -213,7 +220,21 @@ func combineMetadata(entries []wrapper.Options) map[string]interface{} { return metadata } -func (o Options) doUpload(spec *downwardapi.JobSpec, passed, aborted bool, metadata map[string]interface{}, logReaders map[string]io.Reader) error { +//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(ctx context.Context, spec *downwardapi.JobSpec, passed, aborted bool, metadata map[string]interface{}, logReaders map[string]io.Reader) error { uploadTargets := make(map[string]gcs.UploadFunc) for logName, reader := range logReaders { @@ -249,7 +270,7 @@ func (o Options) doUpload(spec *downwardapi.JobSpec, passed, aborted bool, metad uploadTargets[prowv1.FinishedStatusFile] = gcs.DataUpload(bytes.NewBuffer(finishedData)) } - if err := o.GcsOptions.Run(spec, uploadTargets); err != nil { + if err := o.GcsOptions.Run(ctx, spec, uploadTargets); err != nil { return fmt.Errorf("failed to upload to GCS: %v", err) }