refactor(pkg/jenkins): change the state update logic

pkg/jenkins/controller.go

@@ -365,7 +365,7 @@ func (c *Controller) syncPendingJob(pj prowapi.ProwJob, reports chan<- prowapi.P
 		case jb.IsRunning():
 			// Build still going.
 			c.incrementNumPendingJobs(pj.Spec.Job)
-			if pj.Status.Description == "Jenkins job running." {
+			if pj.Status.Description == "Jenkins job running." && pj.Status.URL != "" {
 				return nil
 			}
 			pj.Status.Description = "Jenkins job running."
@@ -429,7 +429,7 @@ func (c *Controller) syncTriggeredJob(pj prowapi.ProwJob, reports chan<- prowapi
 	// Record last known state so we can patch
 	prevPJ := pj.DeepCopy()
 
-	if _, jbExists := jbs[pj.ObjectMeta.Name]; !jbExists {
+	if jb, jbExists := jbs[pj.ObjectMeta.Name]; !jbExists {
 		// Do not start more jobs than specified.
 		if !c.canExecuteConcurrently(&pj) {
 			return nil
@@ -446,20 +446,31 @@ func (c *Controller) syncTriggeredJob(pj prowapi.ProwJob, reports chan<- prowapi
 			pj.Status.URL = c.cfg().StatusErrorLink
 			pj.Status.Description = "Error starting Jenkins job."
 		} else {
-			now := metav1.NewTime(c.clock.Now())
-			pj.Status.PendingTime = &now
-			pj.Status.State = prowapi.PendingState
 			pj.Status.Description = "Jenkins job enqueued."
 		}
-	} else {
+	} else if jb.IsEnqueued() {
+		// Still in queue.
+		pj.Status.Description = "Jenkins job enqueued."
+	} else if jb.IsRunning() {
 		// If a Jenkins build already exists for this job, advance the ProwJob to Pending and
 		// it should be handled by syncPendingJob in the next sync.
 		if pj.Status.PendingTime == nil {
 			now := metav1.NewTime(c.clock.Now())
 			pj.Status.PendingTime = &now
 		}
 		pj.Status.State = prowapi.PendingState
-		pj.Status.Description = "Jenkins job enqueued."
+		pj.Status.Description = "Jenkins job running."
+
+		// Construct the status URL that will be used in reports.
+		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 {
+			c.log.WithFields(pjutil.ProwJobFields(&pj)).Errorf("error executing URL template: %v", err)
+		} else {
+			pj.Status.URL = b.String()
+		}
 	}
 	// Report to GitHub.
 	reports <- pj

pkg/jenkins/controller_test.go

@@ -189,7 +189,6 @@ func (f *fghc) EditCommentWithContext(_ context.Context, org, repo string, ID in
 
 func TestSyncTriggeredJobs(t *testing.T) {
 	fakeClock := clocktesting.NewFakeClock(time.Now().Truncate(1 * time.Second))
-	pendingTime := metav1.NewTime(fakeClock.Now())
 
 	var testcases = []struct {
 		name           string
@@ -223,9 +222,9 @@ func TestSyncTriggeredJobs(t *testing.T) {
 			},
 			expectedBuild:       true,
 			expectedReport:      true,
-			expectedState:       prowapi.PendingState,
+			expectedState:       prowapi.TriggeredState,
 			expectedEnqueued:    true,
-			expectedPendingTime: &pendingTime,
+			expectedPendingTime: nil,
 		},
 		{
 			name: "start new job, error",
@@ -292,9 +291,9 @@ func TestSyncTriggeredJobs(t *testing.T) {
 			maxConcurrency:      21,
 			expectedBuild:       true,
 			expectedReport:      true,
-			expectedState:       prowapi.PendingState,
+			expectedState:       prowapi.TriggeredState,
 			expectedEnqueued:    true,
-			expectedPendingTime: &pendingTime,
+			expectedPendingTime: nil,
 		},
 	}
 	for _, tc := range testcases {
@@ -398,14 +397,14 @@ func TestSyncPendingJobs(t *testing.T) {
 					Job: "test-job",
 				},
 				Status: prowapi.ProwJobStatus{
-					State:       prowapi.PendingState,
+					State:       prowapi.TriggeredState,
 					Description: "Jenkins job enqueued.",
 				},
 			},
 			builds: map[string]Build{
 				"foofoo": {enqueued: true, Number: 10},
 			},
-			expectedState:    prowapi.PendingState,
+			expectedState:    prowapi.TriggeredState,
 			expectedEnqueued: true,
 		},
 		{
@@ -420,7 +419,7 @@ func TestSyncPendingJobs(t *testing.T) {
 				},
 				Status: prowapi.ProwJobStatus{
 					State:       prowapi.PendingState,
-					Description: "Jenkins job enqueued.",
+					Description: "Jenkins job running.",
 				},
 			},
 			builds: map[string]Build{
@@ -635,7 +634,7 @@ func TestBatch(t *testing.T) {
 	fakeProwJobClient := fake.NewSimpleClientset(&pj)
 	jc := &fjc{
 		builds: map[string]Build{
-			"known_name": { /* Running */ },
+			"known_name": {enqueued: true},
 		},
 	}
 	totServ := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -661,7 +660,7 @@ func TestBatch(t *testing.T) {
 	if err != nil {
 		t.Fatalf("failed to get prowjob from client: %v", err)
 	}
-	if afterFirstSync.Status.State != prowapi.PendingState {
+	if afterFirstSync.Status.State != prowapi.TriggeredState {
 		t.Fatalf("Wrong state: %v", afterFirstSync.Status.State)
 	}
 	if afterFirstSync.Status.Description != "Jenkins job enqueued." {

pkg/jenkins/jenkins.go

@@ -123,7 +123,7 @@ type JobInfo struct {
 
 // IsRunning means the job started but has not finished.
 func (jb *Build) IsRunning() bool {
-	return jb.Result == nil
+	return jb.Result == nil && !jb.enqueued
 }
 
 // IsSuccess means the job passed