Don't run clonerefs if there are no refs to clone.

islinwb · May 24, 2018 · f2e8d97 · f2e8d97
1 parent 3367c77
commit f2e8d97
Show file tree

Hide file tree

Showing 5 changed files with 280 additions and 102 deletions.
diff --git a/prow/initupload/options.go b/prow/initupload/options.go
@@ -18,7 +18,6 @@ package initupload
 
 import (
 	"encoding/json"
-	"errors"
 	"flag"
 
 	"k8s.io/test-infra/prow/gcsupload"
@@ -41,16 +40,9 @@ func NewOptions() *Options {
 type Options struct {
 	*gcsupload.Options
 
-	// Log is the log file to which clone records are written
-	Log string `json:"log"`
-}
-
-func (o *Options) Validate() error {
-	if o.Log == "" {
-		return errors.New("the path to the clone records log was not provided")
-	}
-
-	return o.Options.Validate()
+	// Log is the log file to which clone records are written.
+	// If unspecified, no clone records are uploaded.
+	Log string `json:"log,omitempty"`
 }
 
 // ConfigVar exposes the environment variable used

diff --git a/prow/initupload/options_test.go b/prow/initupload/options_test.go
@@ -52,6 +52,16 @@ func TestOptions_Validate(t *testing.T) {
 					},
 				},
 			},
+			expectedErr: false,
+		},
+		{
+			name: "missing path strategy",
+			input: Options{
+				Options: &gcsupload.Options{
+					DryRun:           true,
+					GCSConfiguration: &kube.GCSConfiguration{},
+				},
+			},
 			expectedErr: true,
 		},
 	}

diff --git a/prow/initupload/run.go b/prow/initupload/run.go
@@ -35,15 +35,46 @@ func (o Options) Run() error {
 		return fmt.Errorf("could not resolve job spec: %v", err)
 	}
 
+	started := struct {
+		Timestamp int64 `json:"timestamp"`
+	}{
+		Timestamp: time.Now().Unix(),
+	}
+	startedData, err := json.Marshal(&started)
+	if err != nil {
+		return fmt.Errorf("could not marshal starting data: %v", err)
+	}
+	uploadTargets := map[string]gcs.UploadFunc{
+		"started.json": gcs.DataUpload(bytes.NewReader(startedData)),
+	}
+
+	var failed bool
+	if o.Log != "" {
+		if failed, err = processCloneLog(o.Log, uploadTargets); err != nil {
+			return err
+		}
+	}
+
+	if err := o.Options.Run(spec, uploadTargets); err != nil {
+		return fmt.Errorf("failed to upload to GCS: %v", err)
+	}
+
+	if failed {
+		return errors.New("cloning the appropriate refs failed")
+	}
+
+	return nil
+}
+
+func processCloneLog(logfile string, uploadTargets map[string]gcs.UploadFunc) (bool, error) {
 	var cloneRecords []clone.Record
-	data, err := ioutil.ReadFile(o.Log)
+	data, err := ioutil.ReadFile(logfile)
 	if err != nil {
-		return fmt.Errorf("could not read clone log: %v", err)
+		return true, fmt.Errorf("could not read clone log: %v", err)
 	}
 	if err = json.Unmarshal(data, &cloneRecords); err != nil {
-		return fmt.Errorf("could not unmarshal clone records: %v", err)
+		return true, fmt.Errorf("could not unmarshal clone records: %v", err)
 	}
-
 	// Do not read from cloneLog directly.
 	// Instead create multiple readers from cloneLog so it can be uploaded to
 	// both clone-log.txt and build-log.txt on failure.
@@ -53,25 +84,12 @@ func (o Options) Run() error {
 		cloneLog.WriteString(clone.FormatRecord(record))
 		failed = failed || record.Failed
 	}
-
-	uploadTargets := map[string]gcs.UploadFunc{
-		"clone-log.txt":      gcs.DataUpload(bytes.NewReader(cloneLog.Bytes())),
-		"clone-records.json": gcs.FileUpload(o.Log),
-	}
-
-	started := struct {
-		Timestamp int64 `json:"timestamp"`
-	}{
-		Timestamp: time.Now().Unix(),
-	}
-	startedData, err := json.Marshal(&started)
-	if err != nil {
-		return fmt.Errorf("could not marshal starting data: %v", err)
-	} else {
-		uploadTargets["started.json"] = gcs.DataUpload(bytes.NewReader(startedData))
-	}
+	uploadTargets["clone-log.txt"] = gcs.DataUpload(bytes.NewReader(cloneLog.Bytes()))
+	uploadTargets["clone-records.json"] = gcs.FileUpload(logfile)
 
 	if failed {
+		uploadTargets["build-log.txt"] = gcs.DataUpload(bytes.NewReader(cloneLog.Bytes()))
+
 		finished := struct {
 			Timestamp int64  `json:"timestamp"`
 			Passed    bool   `json:"passed"`
@@ -83,20 +101,9 @@ func (o Options) Run() error {
 		}
 		finishedData, err := json.Marshal(&finished)
 		if err != nil {
-			return fmt.Errorf("could not marshal finishing data: %v", err)
-		} else {
-			uploadTargets["build-log.txt"] = gcs.DataUpload(bytes.NewReader(cloneLog.Bytes()))
-			uploadTargets["finished.json"] = gcs.DataUpload(bytes.NewReader(finishedData))
+			return true, fmt.Errorf("could not marshal finishing data: %v", err)
 		}
+		uploadTargets["finished.json"] = gcs.DataUpload(bytes.NewReader(finishedData))
 	}
-
-	if err := o.Options.Run(spec, uploadTargets); err != nil {
-		return fmt.Errorf("failed to upload to GCS: %v", err)
-	}
-
-	if failed {
-		return errors.New("cloning the appropriate refs failed")
-	}
-
-	return nil
+	return failed, nil
 }
diff --git a/prow/pod-utils/decorate/podspec.go b/prow/pod-utils/decorate/podspec.go
@@ -154,48 +154,58 @@ func decorate(spec *kube.PodSpec, pj *kube.ProwJob, rawEnv map[string]string) er
 		},
 	}
 
-	var sshKeyMode int32 = 0400 // this is octal, so symbolic ref is `u+r`
-	var sshKeysMounts []kube.VolumeMount
 	var sshKeysVolumes []kube.Volume
-	var sshKeyPaths []string
-	for _, secret := range pj.Spec.DecorationConfig.SshKeySecrets {
-		name := fmt.Sprintf("%s-%s", SshKeysMountNamePrefix, secret)
-		keyPath := path.Join(SshKeysMountPathPrefix, secret)
-		sshKeyPaths = append(sshKeyPaths, keyPath)
-		sshKeysMounts = append(sshKeysMounts, kube.VolumeMount{
-			Name:      name,
-			MountPath: keyPath,
-			ReadOnly:  true,
-		})
-		sshKeysVolumes = append(sshKeysVolumes, kube.Volume{
-			Name: name,
-			VolumeSource: kube.VolumeSource{
-				Secret: &kube.SecretSource{
-					SecretName:  secret,
-					DefaultMode: &sshKeyMode,
-				},
-			},
-		})
-	}
-
-	cloneLog := fmt.Sprintf("%s/clone.json", LogMountPath)
+	var cloneLog string
 	var refs []*kube.Refs
 	if pj.Spec.Refs != nil {
 		refs = append(refs, pj.Spec.Refs)
 	}
 	refs = append(refs, pj.Spec.ExtraRefs...)
-	cloneConfigEnv, err := clonerefs.Encode(clonerefs.Options{
-		SrcRoot:      CodeMountPath,
-		Log:          cloneLog,
-		GitUserName:  clonerefs.DefaultGitUserName,
-		GitUserEmail: clonerefs.DefaultGitUserEmail,
-		GitRefs:      refs,
-		KeyFiles:     sshKeyPaths,
-	})
-	if err != nil {
-		return fmt.Errorf("could not encode clone configuration as JSON: %v", err)
-	}
+	if len(refs) > 0 {
+		var sshKeyMode int32 = 0400 // this is octal, so symbolic ref is `u+r`
+		var sshKeysMounts []kube.VolumeMount
+		var sshKeyPaths []string
+		for _, secret := range pj.Spec.DecorationConfig.SshKeySecrets {
+			name := fmt.Sprintf("%s-%s", SshKeysMountNamePrefix, secret)
+			keyPath := path.Join(SshKeysMountPathPrefix, secret)
+			sshKeyPaths = append(sshKeyPaths, keyPath)
+			sshKeysMounts = append(sshKeysMounts, kube.VolumeMount{
+				Name:      name,
+				MountPath: keyPath,
+				ReadOnly:  true,
+			})
+			sshKeysVolumes = append(sshKeysVolumes, kube.Volume{
+				Name: name,
+				VolumeSource: kube.VolumeSource{
+					Secret: &kube.SecretSource{
+						SecretName:  secret,
+						DefaultMode: &sshKeyMode,
+					},
+				},
+			})
+		}
+
+		cloneLog = fmt.Sprintf("%s/clone.json", LogMountPath)
+		cloneConfigEnv, err := clonerefs.Encode(clonerefs.Options{
+			SrcRoot:      CodeMountPath,
+			Log:          cloneLog,
+			GitUserName:  clonerefs.DefaultGitUserName,
+			GitUserEmail: clonerefs.DefaultGitUserEmail,
+			GitRefs:      refs,
+			KeyFiles:     sshKeyPaths,
+		})
+		if err != nil {
+			return fmt.Errorf("could not encode clone configuration as JSON: %v", err)
+		}
 
+		spec.InitContainers = append(spec.InitContainers, kube.Container{
+			Name:         "clonerefs",
+			Image:        pj.Spec.DecorationConfig.UtilityImages.CloneRefs,
+			Command:      []string{"/clonerefs"},
+			Env:          kubeEnv(map[string]string{clonerefs.JSONConfigEnvVar: cloneConfigEnv}),
+			VolumeMounts: append([]kube.VolumeMount{logMount, codeMount}, sshKeysMounts...),
+		})
+	}
 	gcsOptions := gcsupload.Options{
 		// TODO: pass the artifact dir here too once we figure that out
 		GCSConfiguration:   pj.Spec.DecorationConfig.GCSConfiguration,
@@ -212,15 +222,8 @@ func decorate(spec *kube.PodSpec, pj *kube.ProwJob, rawEnv map[string]string) er
 
 	entrypointLocation := fmt.Sprintf("%s/entrypoint", ToolsMountPath)
 
-	spec.InitContainers = []kube.Container{
-		{
-			Name:         "clonerefs",
-			Image:        pj.Spec.DecorationConfig.UtilityImages.CloneRefs,
-			Command:      []string{"/clonerefs"},
-			Env:          kubeEnv(map[string]string{clonerefs.JSONConfigEnvVar: cloneConfigEnv}),
-			VolumeMounts: append([]kube.VolumeMount{logMount, codeMount}, sshKeysMounts...),
-		},
-		{
+	spec.InitContainers = append(spec.InitContainers,
+		kube.Container{
 			Name:    "initupload",
 			Image:   pj.Spec.DecorationConfig.UtilityImages.InitUpload,
 			Command: []string{"/initupload"},
@@ -230,14 +233,14 @@ func decorate(spec *kube.PodSpec, pj *kube.ProwJob, rawEnv map[string]string) er
 			}),
 			VolumeMounts: []kube.VolumeMount{logMount, gcsCredentialsMount},
 		},
-		{
+		kube.Container{
 			Name:         "place-tools",
 			Image:        pj.Spec.DecorationConfig.UtilityImages.Entrypoint,
 			Command:      []string{"/bin/cp"},
 			Args:         []string{"/entrypoint", entrypointLocation},
 			VolumeMounts: []kube.VolumeMount{toolsMount},
 		},
-	}
+	)
 
 	wrapperOptions := wrapper.Options{
 		ProcessLog: fmt.Sprintf("%s/process-log.txt", LogMountPath),
@@ -257,16 +260,8 @@ func decorate(spec *kube.PodSpec, pj *kube.ProwJob, rawEnv map[string]string) er
 	allEnv[entrypoint.JSONConfigEnvVar] = entrypointConfigEnv
 
 	spec.Containers[0].Command = []string{entrypointLocation}
-	var checkoutRefs *kube.Refs
-	if pj.Spec.Type == kube.PeriodicJob {
-		if len(pj.Spec.ExtraRefs) > 0 {
-			checkoutRefs = pj.Spec.ExtraRefs[0]
-		}
-	} else {
-		checkoutRefs = pj.Spec.Refs
-	}
-	if checkoutRefs != nil {
-		spec.Containers[0].WorkingDir = clone.PathForRefs(CodeMountPath, checkoutRefs)
+	if len(refs) > 0 {
+		spec.Containers[0].WorkingDir = clone.PathForRefs(CodeMountPath, refs[0])
 	}
 	spec.Containers[0].Args = []string{}
 	spec.Containers[0].Env = append(spec.Containers[0].Env, kubeEnv(allEnv)...)