Skip to content

Commit 8e278cc

Browse files
committed
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]>
1 parent 57fae23 commit 8e278cc

File tree

13 files changed

+254
-27
lines changed

13 files changed

+254
-27
lines changed

prow/apis/prowjobs/v1/types.go

+9
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,10 @@ type DecorationConfig struct {
373373

374374
// CensoringOptions exposes options for censoring output logs and artifacts.
375375
CensoringOptions *CensoringOptions `json:"censoring_options,omitempty"`
376+
377+
// UploadIgnoresInterrupts causes sidecar to ignore interrupts for the upload process in
378+
// hope that the test process exits cleanly before starting an upload.
379+
UploadIgnoresInterrupts *bool `json:"upload_ignores_interrupts,omitempty"`
376380
}
377381

378382
type CensoringOptions struct {
@@ -434,6 +438,7 @@ func (g *CensoringOptions) ApplyDefault(def *CensoringOptions) *CensoringOptions
434438
merged.ExcludeDirectories = def.ExcludeDirectories
435439
}
436440
return &merged
441+
437442
}
438443

439444
// Resources holds resource requests and limits for
@@ -533,6 +538,10 @@ func (d *DecorationConfig) ApplyDefault(def *DecorationConfig) *DecorationConfig
533538
merged.CensorSecrets = def.CensorSecrets
534539
}
535540

541+
if merged.UploadIgnoresInterrupts == nil {
542+
merged.UploadIgnoresInterrupts = def.UploadIgnoresInterrupts
543+
}
544+
536545
return &merged
537546
}
538547

prow/apis/prowjobs/v1/types_test.go

+20
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,26 @@ func TestDecorationDefaultingDoesntOverwrite(t *testing.T) {
205205
return def
206206
},
207207
},
208+
{
209+
name: "ingnore interrupts set",
210+
provided: &DecorationConfig{
211+
UploadIgnoresInterrupts: &truth,
212+
},
213+
expected: func(orig, def *DecorationConfig) *DecorationConfig {
214+
def.UploadIgnoresInterrupts = orig.UploadIgnoresInterrupts
215+
return def
216+
},
217+
},
218+
{
219+
name: "do not ingnore interrupts ",
220+
provided: &DecorationConfig{
221+
UploadIgnoresInterrupts: &lies,
222+
},
223+
expected: func(orig, def *DecorationConfig) *DecorationConfig {
224+
def.UploadIgnoresInterrupts = orig.UploadIgnoresInterrupts
225+
return def
226+
},
227+
},
208228
}
209229

210230
for _, testCase := range testCases {

prow/apis/prowjobs/v1/zz_generated.deepcopy.go

+5
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

prow/cmd/gcsupload/main.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ limitations under the License.
1919
package main
2020

2121
import (
22+
"context"
23+
2224
"github.com/sirupsen/logrus"
2325
"k8s.io/test-infra/prow/pod-utils/downwardapi"
2426
"k8s.io/test-infra/prow/pod-utils/options"
@@ -45,7 +47,8 @@ func main() {
4547
logrus.WithError(err).Fatal("Could not resolve job spec")
4648
}
4749

48-
if err := o.Run(spec, map[string]gcs.UploadFunc{}); err != nil {
50+
ctx := context.Background()
51+
if err := o.Run(ctx, spec, map[string]gcs.UploadFunc{}); err != nil {
4952
logrus.WithError(err).Fatal("Failed to upload to GCS")
5053
}
5154
}

prow/config/prow-config-documented.yaml

+8
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,10 @@ plank:
670670
# before aborting a job with SIGINT.
671671
timeout: 0s
672672

673+
# UploadIgnoresInterrupts causes sidecar to ignore interrupts for the upload process in
674+
# hope that the test process exits cleanly before starting an upload.
675+
upload_ignores_interrupts: false
676+
673677
# UtilityImages holds pull specs for utility container
674678
# images used to decorate a PodSpec.
675679
utility_images:
@@ -843,6 +847,10 @@ plank:
843847
# before aborting a job with SIGINT.
844848
timeout: 0s
845849

850+
# UploadIgnoresInterrupts causes sidecar to ignore interrupts for the upload process in
851+
# hope that the test process exits cleanly before starting an upload.
852+
upload_ignores_interrupts: false
853+
846854
# UtilityImages holds pull specs for utility container
847855
# images used to decorate a PodSpec.
848856
utility_images:

prow/gcsupload/run.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package gcsupload
1818

1919
import (
20+
"context"
2021
"fmt"
2122
"mime"
2223
"net/url"
@@ -37,7 +38,7 @@ import (
3738
// a parameter and will have the prefix prepended
3839
// to their destination in GCS, so the caller can
3940
// operate relative to the base of the GCS dir.
40-
func (o Options) Run(spec *downwardapi.JobSpec, extra map[string]gcs.UploadFunc) error {
41+
func (o Options) Run(ctx context.Context, spec *downwardapi.JobSpec, extra map[string]gcs.UploadFunc) error {
4142
logrus.WithField("options", o).Debug("Uploading to blob storage")
4243

4344
for extension, mediaType := range o.GCSConfiguration.MediaTypes {
@@ -57,12 +58,12 @@ func (o Options) Run(spec *downwardapi.JobSpec, extra map[string]gcs.UploadFunc)
5758
}
5859

5960
if o.LocalOutputDir == "" {
60-
if err := gcs.Upload(o.Bucket, o.StorageClientOptions.GCSCredentialsFile, o.StorageClientOptions.S3CredentialsFile, uploadTargets); err != nil {
61+
if err := gcs.Upload(ctx, o.Bucket, o.StorageClientOptions.GCSCredentialsFile, o.StorageClientOptions.S3CredentialsFile, uploadTargets); err != nil {
6162
return fmt.Errorf("failed to upload to blob storage: %w", err)
6263
}
6364
logrus.Info("Finished upload to blob storage")
6465
} else {
65-
if err := gcs.LocalExport(o.LocalOutputDir, uploadTargets); err != nil {
66+
if err := gcs.LocalExport(ctx, o.LocalOutputDir, uploadTargets); err != nil {
6667
return fmt.Errorf("failed to copy files to %q: %w", o.LocalOutputDir, err)
6768
}
6869
logrus.Infof("Finished copying files to %q.", o.LocalOutputDir)

prow/initupload/run.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package initupload
1818

1919
import (
2020
"bytes"
21+
"context"
2122
"encoding/json"
2223
"errors"
2324
"fmt"
@@ -106,7 +107,8 @@ func (o Options) Run() error {
106107

107108
uploadTargets[prowv1.StartedStatusFile] = gcs.DataUpload(bytes.NewReader(startedData))
108109

109-
if err := o.Options.Run(spec, uploadTargets); err != nil {
110+
ctx := context.Background()
111+
if err := o.Options.Run(ctx, spec, uploadTargets); err != nil {
110112
return fmt.Errorf("failed to upload to blob storage: %v", err)
111113
}
112114

prow/pod-utils/decorate/podspec.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,9 @@ func decorate(spec *coreapi.PodSpec, pj *prowapi.ProwJob, rawEnv map[string]stri
742742
wrappers = append(wrappers, *wrapperOptions)
743743
}
744744

745-
sidecar, err := Sidecar(pj.Spec.DecorationConfig, blobStorageOptions, blobStorageMounts, logMount, outputMount, encodedJobSpec, !RequirePassingEntries, !IgnoreInterrupts, secretVolumeMounts, wrappers...)
745+
ignoreInterrupts := pj.Spec.DecorationConfig.UploadIgnoresInterrupts != nil && *pj.Spec.DecorationConfig.UploadIgnoresInterrupts
746+
747+
sidecar, err := Sidecar(pj.Spec.DecorationConfig, blobStorageOptions, blobStorageMounts, logMount, outputMount, encodedJobSpec, !RequirePassingEntries, ignoreInterrupts, secretVolumeMounts, wrappers...)
746748
if err != nil {
747749
return fmt.Errorf("create sidecar: %v", err)
748750
}
@@ -793,8 +795,6 @@ func DetermineWorkDir(baseDir string, refs []prowapi.Refs) string {
793795
const (
794796
// RequirePassingEntries causes sidecar to return an error if any entry fails. Otherwise it exits cleanly so long as it can complete.
795797
RequirePassingEntries = true
796-
// IgnoreInterrupts causes sidecar to ignore interrupts and hope that the test process exits cleanly before starting an upload.
797-
IgnoreInterrupts = true
798798
)
799799

800800
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) {

prow/pod-utils/decorate/podspec_test.go

+48
Original file line numberDiff line numberDiff line change
@@ -1130,6 +1130,7 @@ func TestDecorate(t *testing.T) {
11301130
gCSCredentialsSecret := "gcs-secret"
11311131
defaultServiceAccountName := "default-sa"
11321132
censor := true
1133+
ignoreInterrupts := true
11331134
var testCases = []struct {
11341135
name string
11351136
spec *coreapi.PodSpec
@@ -1230,6 +1231,53 @@ func TestDecorate(t *testing.T) {
12301231
},
12311232
rawEnv: map[string]string{"custom": "env"},
12321233
},
1234+
{
1235+
name: "ignore interrupts in sidecar",
1236+
spec: &coreapi.PodSpec{
1237+
Volumes: []coreapi.Volume{
1238+
{Name: "secret", VolumeSource: coreapi.VolumeSource{Secret: &coreapi.SecretVolumeSource{SecretName: "secretname"}}},
1239+
},
1240+
Containers: []coreapi.Container{
1241+
{Name: "test", Command: []string{"/bin/ls"}, Args: []string{"-l", "-a"}, VolumeMounts: []coreapi.VolumeMount{{Name: "secret", MountPath: "/secret"}}},
1242+
},
1243+
ServiceAccountName: "tester",
1244+
},
1245+
pj: &prowapi.ProwJob{
1246+
Spec: prowapi.ProwJobSpec{
1247+
DecorationConfig: &prowapi.DecorationConfig{
1248+
Timeout: &prowapi.Duration{Duration: time.Minute},
1249+
GracePeriod: &prowapi.Duration{Duration: time.Hour},
1250+
UploadIgnoresInterrupts: &ignoreInterrupts,
1251+
UtilityImages: &prowapi.UtilityImages{
1252+
CloneRefs: "cloneimage",
1253+
InitUpload: "initimage",
1254+
Entrypoint: "entrypointimage",
1255+
Sidecar: "sidecarimage",
1256+
},
1257+
Resources: &prowapi.Resources{
1258+
CloneRefs: &coreapi.ResourceRequirements{Limits: coreapi.ResourceList{"cpu": resource.Quantity{}}, Requests: coreapi.ResourceList{"memory": resource.Quantity{}}},
1259+
InitUpload: &coreapi.ResourceRequirements{Limits: coreapi.ResourceList{"cpu": resource.Quantity{}}, Requests: coreapi.ResourceList{"memory": resource.Quantity{}}},
1260+
PlaceEntrypoint: &coreapi.ResourceRequirements{Limits: coreapi.ResourceList{"cpu": resource.Quantity{}}, Requests: coreapi.ResourceList{"memory": resource.Quantity{}}},
1261+
Sidecar: &coreapi.ResourceRequirements{Limits: coreapi.ResourceList{"cpu": resource.Quantity{}}, Requests: coreapi.ResourceList{"memory": resource.Quantity{}}},
1262+
},
1263+
GCSConfiguration: &prowapi.GCSConfiguration{
1264+
Bucket: "bucket",
1265+
PathStrategy: "single",
1266+
DefaultOrg: "org",
1267+
DefaultRepo: "repo",
1268+
},
1269+
GCSCredentialsSecret: &gCSCredentialsSecret,
1270+
DefaultServiceAccountName: &defaultServiceAccountName,
1271+
},
1272+
Refs: &prowapi.Refs{
1273+
Org: "org", Repo: "repo", BaseRef: "main", BaseSHA: "abcd1234",
1274+
Pulls: []prowapi.Pull{{Number: 1, SHA: "aksdjhfkds"}},
1275+
},
1276+
ExtraRefs: []prowapi.Refs{{Org: "other", Repo: "something", BaseRef: "release", BaseSHA: "sldijfsd"}},
1277+
},
1278+
},
1279+
rawEnv: map[string]string{"custom": "env"},
1280+
},
12331281
}
12341282

12351283
for _, testCase := range testCases {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
containers:
2+
- command:
3+
- /tools/entrypoint
4+
env:
5+
- name: ARTIFACTS
6+
value: /logs/artifacts
7+
- name: GOPATH
8+
value: /home/prow/go
9+
- name: custom
10+
value: env
11+
- name: ENTRYPOINT_OPTIONS
12+
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"}'
13+
name: test
14+
resources: {}
15+
volumeMounts:
16+
- mountPath: /secret
17+
name: secret
18+
- mountPath: /logs
19+
name: logs
20+
- mountPath: /tools
21+
name: tools
22+
- mountPath: /home/prow/go
23+
name: code
24+
workingDir: /home/prow/go/src/github.com/org/repo
25+
- command:
26+
- /sidecar
27+
env:
28+
- name: JOB_SPEC
29+
- name: SIDECAR_OPTIONS
30+
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":{}}'
31+
image: sidecarimage
32+
name: sidecar
33+
resources:
34+
limits:
35+
cpu: "0"
36+
requests:
37+
memory: "0"
38+
volumeMounts:
39+
- mountPath: /logs
40+
name: logs
41+
- mountPath: /secrets/gcs
42+
name: gcs-credentials
43+
initContainers:
44+
- command:
45+
- /clonerefs
46+
env:
47+
- name: CLONEREFS_OPTIONS
48+
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"}]}'
49+
image: cloneimage
50+
name: clonerefs
51+
resources:
52+
limits:
53+
cpu: "0"
54+
requests:
55+
memory: "0"
56+
volumeMounts:
57+
- mountPath: /logs
58+
name: logs
59+
- mountPath: /home/prow/go
60+
name: code
61+
- mountPath: /tmp
62+
name: clonerefs-tmp
63+
- command:
64+
- /initupload
65+
env:
66+
- name: INITUPLOAD_OPTIONS
67+
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"}'
68+
- name: JOB_SPEC
69+
image: initimage
70+
name: initupload
71+
resources:
72+
limits:
73+
cpu: "0"
74+
requests:
75+
memory: "0"
76+
volumeMounts:
77+
- mountPath: /logs
78+
name: logs
79+
- mountPath: /secrets/gcs
80+
name: gcs-credentials
81+
- args:
82+
- /entrypoint
83+
- /tools/entrypoint
84+
command:
85+
- /bin/cp
86+
image: entrypointimage
87+
name: place-entrypoint
88+
resources:
89+
limits:
90+
cpu: "0"
91+
requests:
92+
memory: "0"
93+
volumeMounts:
94+
- mountPath: /tools
95+
name: tools
96+
serviceAccountName: tester
97+
terminationGracePeriodSeconds: 4500
98+
volumes:
99+
- name: secret
100+
secret:
101+
secretName: secretname
102+
- emptyDir: {}
103+
name: logs
104+
- emptyDir: {}
105+
name: tools
106+
- name: gcs-credentials
107+
secret:
108+
secretName: gcs-secret
109+
- emptyDir: {}
110+
name: clonerefs-tmp
111+
- emptyDir: {}
112+
name: code

prow/pod-utils/gcs/upload.go

+2-4
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ const retryCount = 4
4444
// Upload uploads all of the data in the
4545
// uploadTargets map to blob storage in parallel. The map is
4646
// keyed on blob storage path under the bucket
47-
func Upload(bucket, gcsCredentialsFile, s3CredentialsFile string, uploadTargets map[string]UploadFunc) error {
47+
func Upload(ctx context.Context, bucket, gcsCredentialsFile, s3CredentialsFile string, uploadTargets map[string]UploadFunc) error {
4848
parsedBucket, err := url.Parse(bucket)
4949
if err != nil {
5050
return fmt.Errorf("cannot parse bucket name %s: %w", bucket, err)
@@ -53,7 +53,6 @@ func Upload(bucket, gcsCredentialsFile, s3CredentialsFile string, uploadTargets
5353
parsedBucket.Scheme = providers.GS
5454
}
5555

56-
ctx := context.Background()
5756
opener, err := pkgio.NewOpener(ctx, gcsCredentialsFile, s3CredentialsFile)
5857
if err != nil {
5958
return fmt.Errorf("new opener: %w", err)
@@ -66,8 +65,7 @@ func Upload(bucket, gcsCredentialsFile, s3CredentialsFile string, uploadTargets
6665

6766
// LocalExport copies all of the data in the uploadTargets map to local files in parallel. The map
6867
// is keyed on file path under the exportDir.
69-
func LocalExport(exportDir string, uploadTargets map[string]UploadFunc) error {
70-
ctx := context.Background()
68+
func LocalExport(ctx context.Context, exportDir string, uploadTargets map[string]UploadFunc) error {
7169
opener, err := pkgio.NewOpener(ctx, "", "")
7270
if err != nil {
7371
return fmt.Errorf("new opener: %w", err)

prow/pod-utils/gcs/upload_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ func TestUploadWithRetries(t *testing.T) {
143143

144144
}
145145

146-
err := Upload("", "", "", uploadFuncs)
146+
ctx := context.Background()
147+
err := Upload(ctx, "", "", "", uploadFuncs)
147148

148149
isErrExpected := false
149150
for _, currentTestState := range currentTestStates {

0 commit comments

Comments
 (0)