Separate build ID from ProwJob ID for Jenkins jobs

It is the intent in the `initupload` and `sidecar` container utilities
to use the full information provided by Prow to the job in `$JOB_SPEC`
to determine where in GCS artifacts from the job should be pushed. Right
now, this is possible and useful for jobs triggered using `plank` but
not so for those triggered using `jenkins-operator`. The largest issue
is that for Jenkins builds, the `$BUILD_ID` parameter was being used
as an identifier to allow Prow to correlate builds with ProwJobs,
whereas in `plank` pods the parameter was a monotonically increasing
identifier vended by `tot` used for e.g. GCS. This patch introduces the
`$PROW_JOB_ID` variable and field to the job spec to separate these two
concerns and allow for them to be fulfilled together. We also thread
through links so that user-facing pages use the `$BUILD_ID` identifier
and not the `$BUILD_NUMBER` identifier that Jenkins produces. This also
handily solves issues with loss of state or Jenkins inconsistencies by
allowing the source of truth for job build state to live in `tot`.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

experiment/manual-trigger/manual-trigger.go

@@ -178,9 +178,9 @@ func main() {
 		MaxConcurrency: 1,
 	}
 
-	if err = jc.BuildFromSpec(&spec, o.jobName); err != nil {
+	if err = jc.BuildFromSpec(&spec, "0", o.jobName); err != nil {
 		log.Println("Submitting the following to Jenkins:")
-		env, _ := pjutil.EnvForSpec(pjutil.NewJobSpec(spec, "0"))
+		env, _ := pjutil.EnvForSpec(pjutil.NewJobSpec(spec, "0", o.jobName))
 		for k, v := range env {
 			log.Printf("  %s=%s\n", k, v)
 		}

prow/README.md

@@ -251,6 +251,7 @@ Variable | Periodic | Postsubmit | Batch | Presubmit | Description | Example
 `JOB_TYPE` | ✓ | ✓ | ✓ | ✓ | Type of job. | `presubmit`
 `JOB_SPEC` | ✓ | ✓ | ✓ | ✓ | JSON-encoded job specification. | see below
 `BUILD_ID` | ✓ | ✓ | ✓ | ✓ | Unique build number for each run. | `12345`
+`PROW_JOB_ID` | ✓ | ✓ | ✓ | ✓ | Unique identifier for the owning Prow Job. | `1ce07fa2-0831-11e8-b07e-0a58ac101036`
 `BUILD_NUMBER` | ✓ | ✓ | ✓ | ✓ | Unique build number for each run. | `12345`
 `REPO_OWNER` | | ✓ | ✓ | ✓ | GitHub org that triggered the job. | `kubernetes`
 `REPO_NAME` | | ✓ | ✓ | ✓ | GitHub repo that triggered the job. | `test-infra`
@@ -270,22 +271,22 @@ job types:
 
 Periodic Job:
 ```json
-{"type":"periodic","job":"job-name","buildid":"0","refs":{}}
+{"type":"periodic","job":"job-name","buildid":"0","prowjobid":"uuid","refs":{}}
 ```
 
 Postsubmit Job:
 ```json
-{"type":"postsubmit","job":"job-name","buildid":"0","refs":{"org":"org-name","repo":"repo-name","base_ref":"base-ref","base_sha":"base-sha"}}
+{"type":"postsubmit","job":"job-name","buildid":"0","prowjobid":"uuid","refs":{"org":"org-name","repo":"repo-name","base_ref":"base-ref","base_sha":"base-sha"}}
 ```
 
 Presubmit Job:
 ```json
-{"type":"presubmit","job":"job-name","buildid":"0","refs":{"org":"org-name","repo":"repo-name","base_ref":"base-ref","base_sha":"base-sha","pulls":[{"number":1,"author":"author-name","sha":"pull-sha"}]}}
+{"type":"presubmit","job":"job-name","buildid":"0","prowjobid":"uuid","refs":{"org":"org-name","repo":"repo-name","base_ref":"base-ref","base_sha":"base-sha","pulls":[{"number":1,"author":"author-name","sha":"pull-sha"}]}}
 ```
 
 Batch Job:
 ```json
-{"type":"batch","job":"job-name","buildid":"0","refs":{"org":"org-name","repo":"repo-name","base_ref":"base-ref","base_sha":"base-sha","pulls":[{"number":1,"author":"author-name","sha":"pull-sha"},{"number":2,"author":"other-author-name","sha":"second-pull-sha"}]}}
+{"type":"batch","job":"job-name","buildid":"0","prowjobid":"uuid","refs":{"org":"org-name","repo":"repo-name","base_ref":"base-ref","base_sha":"base-sha","pulls":[{"number":1,"author":"author-name","sha":"pull-sha"},{"number":2,"author":"other-author-name","sha":"second-pull-sha"}]}}
 ```
 
 ## Bots home

prow/cmd/jenkins-operator/main.go

@@ -45,6 +45,7 @@ import (
 var (
 	configPath = flag.String("config-path", "/etc/config/config", "Path to config.yaml.")
 	selector   = flag.String("label-selector", kube.EmptySelector, "Label selector to be applied in prowjobs. See https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors for constructing a label selector.")
+	totURL     = flag.String("tot-url", "", "Tot URL")
 
 	jenkinsURL             = flag.String("jenkins-url", "http://jenkins-proxy", "Jenkins URL")
 	jenkinsUserName        = flag.String("jenkins-user", "jenkins-trigger", "Jenkins username")
@@ -134,7 +135,10 @@ func main() {
 		ghc = github.NewClient(oauthSecret, *githubEndpoint)
 	}
 
-	c := jenkins.NewController(kc, jc, ghc, logger, configAgent, *selector)
+	c, err := jenkins.NewController(kc, jc, ghc, logger, configAgent, *totURL, *selector)
+	if err != nil {
+		logger.WithError(err).Fatal("Failed to instantiate Jenkins controller.")
+	}
 
 	// Push metrics to the configured prometheus pushgateway endpoint.
 	pushGateway := configAgent.Config().PushGateway

prow/jenkins/BUILD.bazel

@@ -22,6 +22,7 @@ go_library(
         "//prow/kube:go_default_library",
         "//prow/pjutil:go_default_library",
         "//prow/report:go_default_library",
+        "//vendor/github.com/bwmarrin/snowflake:go_default_library",
         "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
         "//vendor/github.com/sirupsen/logrus:go_default_library",
     ],

prow/jenkins/controller.go

@@ -22,6 +22,7 @@ import (
 	"strconv"
 	"sync"
 
+	"github.com/bwmarrin/snowflake"
 	"github.com/sirupsen/logrus"
 
 	"k8s.io/test-infra/prow/config"
@@ -42,7 +43,7 @@ type kubeClient interface {
 }
 
 type jenkinsClient interface {
-	Build(*kube.ProwJob) error
+	Build(*kube.ProwJob, string) error
 	ListBuilds(jobs []string) (map[string]JenkinsBuild, error)
 	Abort(job string, build *JenkinsBuild) error
 }
@@ -65,11 +66,13 @@ type syncFn func(kube.ProwJob, chan<- kube.ProwJob, map[string]JenkinsBuild) err
 
 // Controller manages ProwJobs.
 type Controller struct {
-	kc  kubeClient
-	jc  jenkinsClient
-	ghc githubClient
-	log *logrus.Entry
-	ca  configAgent
+	kc     kubeClient
+	jc     jenkinsClient
+	ghc    githubClient
+	log    *logrus.Entry
+	ca     configAgent
+	node   *snowflake.Node
+	totURL string
 	// selector that will be applied on prowjobs.
 	selector string
 
@@ -84,7 +87,11 @@ type Controller struct {
 }
 
 // NewController creates a new Controller from the provided clients.
-func NewController(kc *kube.Client, jc *Client, ghc *github.Client, logger *logrus.Entry, ca *config.Agent, selector string) *Controller {
+func NewController(kc *kube.Client, jc *Client, ghc *github.Client, logger *logrus.Entry, ca *config.Agent, totURL, selector string) (*Controller, error) {
+	n, err := snowflake.NewNode(1)
+	if err != nil {
+		return nil, err
+	}
 	if logger == nil {
 		logger = logrus.NewEntry(logrus.StandardLogger())
 	}
@@ -95,8 +102,10 @@ func NewController(kc *kube.Client, jc *Client, ghc *github.Client, logger *logr
 		log:         logger,
 		ca:          ca,
 		selector:    selector,
+		node:        n,
+		totURL:      totURL,
 		pendingJobs: make(map[string]int),
-	}
+	}, nil
 }
 
 func (c *Controller) config() config.Controller {
@@ -379,8 +388,9 @@ func (c *Controller) syncPendingJob(pj kube.ProwJob, reports chan<- kube.ProwJob
 			pj.Status.Description = "Jenkins job aborted."
 		}
 		// Construct the status URL that will be used in reports.
-		pj.Status.PodName = fmt.Sprintf("%s-%d", pj.Spec.Job, jb.Number)
-		pj.Status.BuildID = strconv.Itoa(jb.Number)
+		pj.Status.PodName = pj.ObjectMeta.Name
+		pj.Status.BuildID = jb.BuildID()
+		pj.Status.JenkinsBuildID = strconv.Itoa(jb.Number)
 		var b bytes.Buffer
 		if err := c.config().JobURLTemplate.Execute(&b, &pj); err != nil {
 			return fmt.Errorf("error executing URL template: %v", err)
@@ -407,8 +417,12 @@ func (c *Controller) syncTriggeredJob(pj kube.ProwJob, reports chan<- kube.ProwJ
 		if !c.canExecuteConcurrently(&pj) {
 			return nil
 		}
+		buildID, err := c.getBuildID(pj.Spec.Job)
+		if err != nil {
+			return fmt.Errorf("error getting build ID: %v", err)
+		}
 		// Start the Jenkins job.
-		if err := c.jc.Build(&pj); err != nil {
+		if err := c.jc.Build(&pj, buildID); err != nil {
 			c.log.WithError(err).WithFields(pjutil.ProwJobFields(&pj)).Warn("Cannot start Jenkins build")
 			pj.SetComplete()
 			pj.Status.State = kube.ErrorState
@@ -436,6 +450,13 @@ func (c *Controller) syncTriggeredJob(pj kube.ProwJob, reports chan<- kube.ProwJ
 	return err
 }
 
+func (c *Controller) getBuildID(name string) (string, error) {
+	if c.totURL == "" {
+		return c.node.Generate().String(), nil
+	}
+	return pjutil.GetBuildID(name, c.totURL)
+}
+
 // RunAfterSuccessCanRun returns whether a child job (specified as run_after_success in the
 // prow config) can run once its parent job succeeds. The only case we will not run a child job
 // is when it is a presubmit job and has a run_if_changed regural expression specified which does

prow/jenkins/controller_test.go

@@ -19,6 +19,8 @@ package jenkins
 import (
 	"errors"
 	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"reflect"
 	"sync"
 	"testing"
@@ -132,7 +134,7 @@ type fjc struct {
 	builds map[string]JenkinsBuild
 }
 
-func (f *fjc) Build(pj *kube.ProwJob) error {
+func (f *fjc) Build(pj *kube.ProwJob, buildId string) error {
 	f.Lock()
 	defer f.Unlock()
 	if f.err != nil {
@@ -290,6 +292,10 @@ func TestSyncTriggeredJobs(t *testing.T) {
 		},
 	}
 	for _, tc := range testcases {
+		totServ := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			fmt.Fprint(w, "42")
+		}))
+		defer totServ.Close()
 		t.Logf("scenario %q", tc.name)
 		fjc := &fjc{
 			err: tc.err,
@@ -303,6 +309,7 @@ func TestSyncTriggeredJobs(t *testing.T) {
 			jc:          fjc,
 			log:         logrus.NewEntry(logrus.StandardLogger()),
 			ca:          newFakeConfigAgent(t, tc.maxConcurrency, nil),
+			totURL:      totServ.URL,
 			lock:        sync.RWMutex{},
 			pendingJobs: make(map[string]int),
 		}
@@ -402,7 +409,7 @@ func TestSyncPendingJobs(t *testing.T) {
 			builds: map[string]JenkinsBuild{
 				"boing": {enqueued: false, Number: 10},
 			},
-			expectedURL:      "test-job-10/pending",
+			expectedURL:      "boing/pending",
 			expectedState:    kube.PendingState,
 			expectedEnqueued: false,
 			expectedReport:   true,
@@ -423,7 +430,7 @@ func TestSyncPendingJobs(t *testing.T) {
 			builds: map[string]JenkinsBuild{
 				"firstoutthetrenches": {enqueued: false, Number: 10},
 			},
-			expectedURL:    "test-job-10/pending",
+			expectedURL:    "firstoutthetrenches/pending",
 			expectedState:  kube.PendingState,
 			expectedReport: true,
 		},
@@ -473,7 +480,7 @@ func TestSyncPendingJobs(t *testing.T) {
 			builds: map[string]JenkinsBuild{
 				"winwin": {Result: pState(Succeess), Number: 11},
 			},
-			expectedURL:      "test-job-11/success",
+			expectedURL:      "winwin/success",
 			expectedState:    kube.SuccessState,
 			expectedComplete: true,
 			expectedReport:   true,
@@ -494,14 +501,18 @@ func TestSyncPendingJobs(t *testing.T) {
 			builds: map[string]JenkinsBuild{
 				"whatapity": {Result: pState(Failure), Number: 12},
 			},
-			expectedURL:      "test-job-12/failure",
+			expectedURL:      "whatapity/failure",
 			expectedState:    kube.FailureState,
 			expectedComplete: true,
 			expectedReport:   true,
 		},
 	}
 	for _, tc := range testcases {
 		t.Logf("scenario %q", tc.name)
+		totServ := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			fmt.Fprint(w, "42")
+		}))
+		defer totServ.Close()
 		fjc := &fjc{
 			err: tc.err,
 		}
@@ -514,6 +525,7 @@ func TestSyncPendingJobs(t *testing.T) {
 			jc:          fjc,
 			log:         logrus.NewEntry(logrus.StandardLogger()),
 			ca:          newFakeConfigAgent(t, 0, nil),
+			totURL:      totServ.URL,
 			lock:        sync.RWMutex{},
 			pendingJobs: make(map[string]int),
 		}
@@ -596,11 +608,16 @@ func TestBatch(t *testing.T) {
 			"known_name": { /* Running */ },
 		},
 	}
+	totServ := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprint(w, "42")
+	}))
+	defer totServ.Close()
 	c := Controller{
 		kc:          fc,
 		jc:          jc,
 		log:         logrus.NewEntry(logrus.StandardLogger()),
 		ca:          newFakeConfigAgent(t, 0, nil),
+		totURL:      totServ.URL,
 		pendingJobs: make(map[string]int),
 		lock:        sync.RWMutex{},
 	}
@@ -621,7 +638,7 @@ func TestBatch(t *testing.T) {
 	if fc.prowjobs[0].Status.Description != "Jenkins job running." {
 		t.Fatalf("Expected description %q, got %q.", "Jenkins job running.", fc.prowjobs[0].Status.Description)
 	}
-	if fc.prowjobs[0].Status.PodName != "pr-some-job-42" {
+	if fc.prowjobs[0].Status.PodName != "known_name" {
 		t.Fatalf("Wrong PodName: %s", fc.prowjobs[0].Status.PodName)
 	}
 	jc.builds["known_name"] = JenkinsBuild{Result: pState(Succeess)}
@@ -837,6 +854,10 @@ func TestMaxConcurrencyWithNewlyTriggeredJobs(t *testing.T) {
 	}
 
 	for _, test := range tests {
+		totServ := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			fmt.Fprint(w, "42")
+		}))
+		defer totServ.Close()
 		jobs := make(chan kube.ProwJob, len(test.pjs))
 		for _, pj := range test.pjs {
 			jobs <- pj
@@ -852,6 +873,7 @@ func TestMaxConcurrencyWithNewlyTriggeredJobs(t *testing.T) {
 			jc:          fjc,
 			log:         logrus.NewEntry(logrus.StandardLogger()),
 			ca:          newFakeConfigAgent(t, 0, nil),
+			totURL:      totServ.URL,
 			pendingJobs: test.pendingJobs,
 		}