fix(pkg/jenkins): treat the zero number build as enqueued state (#26)

* Revert "Fix Jenkins operator reporting invalid URLs with build number 0 (#23)"

This reverts commit c4199af.

* fix(pkg/jenkins): use enqueued flag for builds with zero number in
`GetBuilds` method

Author: wuhuizuo

pkg/jenkins/controller.go

@@ -365,12 +365,6 @@ func (c *Controller) syncPendingJob(pj prowapi.ProwJob, reports chan<- prowapi.P
 		case jb.IsRunning():
 			// Build still going.
 			c.incrementNumPendingJobs(pj.Spec.Job)
-			// Do not update the commit status when build number is 0.
-			// Build number 0 indicates the Jenkins job hasn't been properly assigned a build number yet.
-			if jb.Number == 0 {
-				c.log.WithFields(pjutil.ProwJobFields(&pj)).Debug("Skipping status update for build with number 0")
-				return nil
-			}
 			if pj.Status.Description == "Jenkins job running." && pj.Status.URL != "" {
 				return nil
 			}
@@ -458,12 +452,6 @@ func (c *Controller) syncTriggeredJob(pj prowapi.ProwJob, reports chan<- prowapi
 		// Still in queue.
 		pj.Status.Description = "Jenkins job enqueued."
 	} else if jb.IsRunning() {
-		// Do not update the commit status when build number is 0.
-		// Build number 0 indicates the Jenkins job hasn't been properly assigned a build number yet.
-		if jb.Number == 0 {
-			c.log.WithFields(pjutil.ProwJobFields(&pj)).Debug("Skipping status update for build with number 0")
-			return nil
-		}
 		// 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 {

pkg/jenkins/controller_test.go

@@ -295,28 +295,6 @@ func TestSyncTriggeredJobs(t *testing.T) {
 			expectedEnqueued:    true,
 			expectedPendingTime: nil,
 		},
-		{
-			name: "running job with build number 0, should not report",
-			pj: prowapi.ProwJob{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "zerobuild",
-					Namespace: "prowjobs",
-				},
-				Spec: prowapi.ProwJobSpec{
-					Type: prowapi.PostsubmitJob,
-				},
-				Status: prowapi.ProwJobStatus{
-					State: prowapi.TriggeredState,
-				},
-			},
-			builds: map[string]Build{
-				"zerobuild": {enqueued: false, Number: 0},
-			},
-			expectedBuild:       false,
-			expectedReport:      false,
-			expectedState:       prowapi.TriggeredState,
-			expectedPendingTime: nil,
-		},
 	}
 	for _, tc := range testcases {
 		totServ := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -548,26 +526,6 @@ func TestSyncPendingJobs(t *testing.T) {
 			expectedComplete: true,
 			expectedReport:   true,
 		},
-		{
-			name: "building with build number 0, should not report",
-			pj: prowapi.ProwJob{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "zerobuild",
-					Namespace: "prowjobs",
-				},
-				Spec: prowapi.ProwJobSpec{
-					Job: "test-job",
-				},
-				Status: prowapi.ProwJobStatus{
-					State: prowapi.PendingState,
-				},
-			},
-			builds: map[string]Build{
-				"zerobuild": {enqueued: false, Number: 0},
-			},
-			expectedState:  prowapi.PendingState,
-			expectedReport: false,
-		},
 	}
 	for _, tc := range testcases {
 		t.Logf("scenario %q", tc.name)

pkg/jenkins/jenkins.go

@@ -22,6 +22,7 @@ import (
 	"errors"
 	"fmt"
 	stdio "io"
+	"maps"
 	"net/http"
 	"net/url"
 	"strings"
@@ -670,9 +671,7 @@ func (c *Client) ListBuilds(jobs []BuildQueryParams) (map[string]Build, error) {
 	}
 
 	for builds := range buildChan {
-		for id, build := range builds {
-			jenkinsBuilds[id] = build
-		}
+		maps.Copy(jenkinsBuilds, builds)
 	}
 
 	return jenkinsBuilds, nil
@@ -744,6 +743,9 @@ func (c *Client) GetBuilds(job string) (map[string]Build, error) {
 		if prowJobID == "" {
 			continue
 		}
+		if jb.Number == 0 {
+			jb.enqueued = true
+		}
 		jenkinsBuilds[prowJobID] = jb
 	}
 	return jenkinsBuilds, nil