Skip to content

Commit

Permalink
prow: sidecar: allow configuring ignore interrupts
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
viveksyngh committed Apr 23, 2021
1 parent 57fae23 commit 17bf4fb
Show file tree
Hide file tree
Showing 13 changed files with 256 additions and 27 deletions.
9 changes: 9 additions & 0 deletions prow/apis/prowjobs/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -434,6 +438,7 @@ func (g *CensoringOptions) ApplyDefault(def *CensoringOptions) *CensoringOptions
merged.ExcludeDirectories = def.ExcludeDirectories
}
return &merged

}

// Resources holds resource requests and limits for
Expand Down Expand Up @@ -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
}

Expand Down
20 changes: 20 additions & 0 deletions prow/apis/prowjobs/v1/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions prow/apis/prowjobs/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion prow/cmd/gcsupload/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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")
}
}
8 changes: 8 additions & 0 deletions prow/config/prow-config-documented.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
7 changes: 4 additions & 3 deletions prow/gcsupload/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package gcsupload

import (
"context"
"fmt"
"mime"
"net/url"
Expand All @@ -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 {
Expand All @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion prow/initupload/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package initupload

import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
Expand Down Expand Up @@ -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)
}

Expand Down
6 changes: 3 additions & 3 deletions prow/pod-utils/decorate/podspec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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) {
Expand Down
48 changes: 48 additions & 0 deletions prow/pod-utils/decorate/podspec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
6 changes: 2 additions & 4 deletions prow/pod-utils/gcs/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion prow/pod-utils/gcs/upload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 17bf4fb

Please sign in to comment.