Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate build ID from ProwJob ID for Jenkins jobs #6853

Merged
merged 2 commits into from
Feb 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bad API for the manual trigger but it's not super important to tackle right now.

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