Skip to content

Commit

Permalink
Merge pull request #6853 from stevekuznetsov/skuznets/jenkins-pj-id
Browse files Browse the repository at this point in the history
Separate build ID from ProwJob ID for Jenkins jobs
  • Loading branch information
k8s-ci-robot authored Feb 16, 2018
2 parents 730e957 + 1b9425a commit 351c416
Show file tree
Hide file tree
Showing 14 changed files with 335 additions and 114 deletions.
4 changes: 2 additions & 2 deletions experiment/manual-trigger/manual-trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
15 changes: 11 additions & 4 deletions prow/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ Note: versions specified in these announcements may not include bug fixes made
in more recent versions so it is recommended that the most recent versions are
used when updating deployments.

- *February 15, 2018* `jenkins-operator` can now accept the `--tot-url` flag
and will use the connection to `tot` to vend build identifiers as `plank`
does, giving control over where in GCS artifacts land to Prow and away from
Jenkins. Furthermore, the `$BUILD_ID` variable in Jenkins jobs will now
correctly point to the build identifier vended by `tot` and a new variable,
`$PROW_JOB_ID`, points to the identifier used to link ProwJobs to Jenkins builds.
- *February 1, 2018* The `config_updater` section in `plugins.yaml`
now uses a `maps` object instead of `config_file`, `plugin_file` strings.
Please switch over before July.
Expand Down Expand Up @@ -251,6 +257,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`
Expand All @@ -270,22 +277,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
Expand Down
6 changes: 5 additions & 1 deletion prow/cmd/jenkins-operator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,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")
Expand Down Expand Up @@ -136,7 +137,10 @@ func main() {
ghc = github.NewClient(oauthSecret, *githubEndpoint)
}

c := jenkins.NewController(kc, jc, ghc, nil, configAgent, *selector)
c, err := jenkins.NewController(kc, jc, ghc, nil, configAgent, *totURL, *selector)
if err != nil {
logrus.WithError(err).Fatal("Failed to instantiate Jenkins controller.")
}

// Push metrics to the configured prometheus pushgateway endpoint.
pushGateway := configAgent.Config().PushGateway
Expand Down
1 change: 1 addition & 0 deletions prow/jenkins/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down
43 changes: 32 additions & 11 deletions prow/jenkins/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"strconv"
"sync"

"github.com/bwmarrin/snowflake"
"github.com/sirupsen/logrus"

"k8s.io/test-infra/prow/config"
Expand All @@ -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
}
Expand All @@ -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

Expand All @@ -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())
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
34 changes: 28 additions & 6 deletions prow/jenkins/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package jenkins
import (
"errors"
"fmt"
"net/http"
"net/http/httptest"
"reflect"
"sync"
"testing"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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),
}
Expand Down Expand Up @@ -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,
Expand All @@ -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,
},
Expand Down Expand Up @@ -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,
Expand All @@ -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,
}
Expand All @@ -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),
}
Expand Down Expand Up @@ -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{},
}
Expand All @@ -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)}
Expand Down Expand Up @@ -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
Expand All @@ -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,
}

Expand Down
Loading

0 comments on commit 351c416

Please sign in to comment.