Skip to content

Commit

Permalink
Actually omit prowjobspec parts when empty
Browse files Browse the repository at this point in the history 
Fix the prowjob API to actually omit fields when empty.
Also fix some mutations in the plank controller that
will bite us back once we switch to informers w/ caches.
  • Loading branch information
@0xmichalis
0xmichalis committed Mar 20, 2018
1 parent 8a2cb8f commit e4a2701
Show file tree
Hide file tree
Showing 13 changed files with 98 additions and 71 deletions.
2 changes: 1 addition & 1 deletion experiment/manual-trigger/manual-trigger.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func main() {
spec := kube.ProwJobSpec{
Type: kube.PresubmitJob,
Job: o.jobName,
Refs: kube.Refs{
Refs: &kube.Refs{
Org: o.org,
Repo: o.repo,
BaseRef: pr.Base.Ref,
Expand Down
18 changes: 10 additions & 8 deletions prow/cmd/deck/jobs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,6 @@ func (ja *JobAgent) update() error {
buildID := j.Status.BuildID
nj := Job{
Type: string(j.Spec.Type),
Repo: fmt.Sprintf("%s/%s", j.Spec.Refs.Org, j.Spec.Refs.Repo),
Refs: j.Spec.Refs.String(),
BaseRef: j.Spec.Refs.BaseRef,
BaseSHA: j.Spec.Refs.BaseSHA,
Job: j.Spec.Job,
Context: j.Spec.Context,
Agent: j.Spec.Agent,
Expand All @@ -232,10 +228,16 @@ func (ja *JobAgent) update() error {
duration -= duration % time.Second // strip fractional seconds
nj.Duration = duration.String()
}
if len(j.Spec.Refs.Pulls) == 1 {
nj.Number = j.Spec.Refs.Pulls[0].Number
nj.Author = j.Spec.Refs.Pulls[0].Author
nj.PullSHA = j.Spec.Refs.Pulls[0].SHA
if j.Spec.Refs != nil {
nj.Repo = fmt.Sprintf("%s/%s", j.Spec.Refs.Org, j.Spec.Refs.Repo)
nj.Refs = j.Spec.Refs.String()
nj.BaseRef = j.Spec.Refs.BaseRef
nj.BaseSHA = j.Spec.Refs.BaseSHA
if len(j.Spec.Refs.Pulls) == 1 {
nj.Number = j.Spec.Refs.Pulls[0].Number
nj.Author = j.Spec.Refs.Pulls[0].Author
nj.PullSHA = j.Spec.Refs.Pulls[0].SHA
}
}
njs = append(njs, nj)
if nj.PodName != "" {
Expand Down
2 changes: 1 addition & 1 deletion prow/cmd/deck/jobs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func TestProwJobs(t *testing.T) {
Spec: kube.ProwJobSpec{
Agent: kube.KubernetesAgent,
Job: "job",
Refs: kube.Refs{
Refs: &kube.Refs{
Org: "kubernetes",
Repo: "test-infra",
},
Expand Down
2 changes: 1 addition & 1 deletion prow/cmd/splice/main_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func fakeProwJob(context string, jobType kube.ProwJobType, completed bool, state
},
Spec: kube.ProwJobSpec{
Context: context,
Refs: refs,
Refs: &refs,
Type: jobType,
},
}
Expand Down
4 changes: 2 additions & 2 deletions prow/config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@ func TestURLTemplate(t *testing.T) {
Spec: kube.ProwJobSpec{
Type: tc.jobType,
Job: tc.job,
Refs: kube.Refs{
Refs: &kube.Refs{
Pulls: []kube.Pull{{}},
Org: tc.org,
Repo: tc.repo,
Expand Down Expand Up @@ -807,7 +807,7 @@ func TestReportTemplate(t *testing.T) {
var b bytes.Buffer
if err := c.Plank.ReportTemplate.Execute(&b, &kube.ProwJob{
Spec: kube.ProwJobSpec{
Refs: kube.Refs{
Refs: &kube.Refs{
Org: tc.org,
Repo: tc.repo,
Pulls: []kube.Pull{
Expand Down
10 changes: 5 additions & 5 deletions prow/jenkins/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func TestSyncTriggeredJobs(t *testing.T) {
pj: kube.ProwJob{
Spec: kube.ProwJobSpec{
Type: kube.PresubmitJob,
Refs: kube.Refs{
Refs: &kube.Refs{
Pulls: []kube.Pull{{
Number: 1,
SHA: "fake-sha",
Expand Down Expand Up @@ -443,7 +443,7 @@ func TestSyncPendingJobs(t *testing.T) {
Spec: kube.ProwJobSpec{
Type: kube.PresubmitJob,
Job: "test-job",
Refs: kube.Refs{
Refs: &kube.Refs{
Pulls: []kube.Pull{{
Number: 1,
SHA: "fake-sha",
Expand Down Expand Up @@ -675,7 +675,7 @@ func TestRunAfterSuccessCanRun(t *testing.T) {
Spec: kube.ProwJobSpec{
Job: "test-e2e",
Type: kube.PresubmitJob,
Refs: kube.Refs{
Refs: &kube.Refs{
Org: "kubernetes",
Repo: "kubernetes",
Pulls: []kube.Pull{
Expand All @@ -697,7 +697,7 @@ func TestRunAfterSuccessCanRun(t *testing.T) {
Spec: kube.ProwJobSpec{
Job: "test-bazel-build",
Type: kube.PresubmitJob,
Refs: kube.Refs{
Refs: &kube.Refs{
Org: "kubernetes",
Repo: "kubernetes",
Pulls: []kube.Pull{
Expand All @@ -724,7 +724,7 @@ func TestRunAfterSuccessCanRun(t *testing.T) {
Spec: kube.ProwJobSpec{
Job: "test-bazel-build",
Type: kube.PresubmitJob,
Refs: kube.Refs{
Refs: &kube.Refs{
Org: "kubernetes",
Repo: "kubernetes",
Pulls: []kube.Pull{
Expand Down
4 changes: 2 additions & 2 deletions prow/kube/prowjob.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,14 @@ type ProwJobSpec struct {
Agent ProwJobAgent `json:"agent,omitempty"`
Cluster string `json:"cluster,omitempty"`
Job string `json:"job,omitempty"`
Refs Refs `json:"refs,omitempty"`
Refs *Refs `json:"refs,omitempty"`

Report bool `json:"report,omitempty"`
Context string `json:"context,omitempty"`
RerunCommand string `json:"rerun_command,omitempty"`
MaxConcurrency int `json:"max_concurrency,omitempty"`

PodSpec v1.PodSpec `json:"pod_spec,omitempty"`
PodSpec *v1.PodSpec `json:"pod_spec,omitempty"`

RunAfterSuccess []ProwJobSpec `json:"run_after_success,omitempty"`
}
Expand Down
6 changes: 5 additions & 1 deletion prow/pjutil/jobspec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,16 @@ type JobSpec struct {
}

func NewJobSpec(spec kube.ProwJobSpec, buildId, prowJobId string) JobSpec {
refs := kube.Refs{}
if spec.Refs != nil {
refs = *spec.Refs
}
return JobSpec{
Type: spec.Type,
Job: spec.Job,
BuildId: buildId,
ProwJobId: prowJobId,
Refs: spec.Refs,
Refs: refs,
agent: spec.Agent,
}
}
Expand Down
19 changes: 11 additions & 8 deletions prow/pjutil/pjutil.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func PresubmitSpec(p config.Presubmit, refs kube.Refs) kube.ProwJobSpec {
pjs := kube.ProwJobSpec{
Type: kube.PresubmitJob,
Job: p.Name,
Refs: refs,
Refs: &refs,

Report: !p.SkipReport,
Context: p.Context,
Expand All @@ -61,7 +61,7 @@ func PresubmitSpec(p config.Presubmit, refs kube.Refs) kube.ProwJobSpec {
}
pjs.Agent = kube.ProwJobAgent(p.Agent)
if pjs.Agent == kube.KubernetesAgent {
pjs.PodSpec = *p.Spec
pjs.PodSpec = p.Spec
pjs.Cluster = p.Cluster
if pjs.Cluster == "" {
pjs.Cluster = kube.DefaultClusterAlias
Expand All @@ -78,12 +78,12 @@ func PostsubmitSpec(p config.Postsubmit, refs kube.Refs) kube.ProwJobSpec {
pjs := kube.ProwJobSpec{
Type: kube.PostsubmitJob,
Job: p.Name,
Refs: refs,
Refs: &refs,
MaxConcurrency: p.MaxConcurrency,
}
pjs.Agent = kube.ProwJobAgent(p.Agent)
if pjs.Agent == kube.KubernetesAgent {
pjs.PodSpec = *p.Spec
pjs.PodSpec = p.Spec
pjs.Cluster = p.Cluster
if pjs.Cluster == "" {
pjs.Cluster = kube.DefaultClusterAlias
Expand All @@ -103,7 +103,7 @@ func PeriodicSpec(p config.Periodic) kube.ProwJobSpec {
}
pjs.Agent = kube.ProwJobAgent(p.Agent)
if pjs.Agent == kube.KubernetesAgent {
pjs.PodSpec = *p.Spec
pjs.PodSpec = p.Spec
pjs.Cluster = p.Cluster
if pjs.Cluster == "" {
pjs.Cluster = kube.DefaultClusterAlias
Expand All @@ -120,12 +120,12 @@ func BatchSpec(p config.Presubmit, refs kube.Refs) kube.ProwJobSpec {
pjs := kube.ProwJobSpec{
Type: kube.BatchJob,
Job: p.Name,
Refs: refs,
Refs: &refs,
Context: p.Context, // The Submit Queue's getCompleteBatches needs this.
}
pjs.Agent = kube.ProwJobAgent(p.Agent)
if pjs.Agent == kube.KubernetesAgent {
pjs.PodSpec = *p.Spec
pjs.PodSpec = p.Spec
pjs.Cluster = p.Cluster
if pjs.Cluster == "" {
pjs.Cluster = kube.DefaultClusterAlias
Expand All @@ -139,6 +139,9 @@ func BatchSpec(p config.Presubmit, refs kube.Refs) kube.ProwJobSpec {

// ProwJobToPod converts a ProwJob to a Pod that will run the tests.
func ProwJobToPod(pj kube.ProwJob, buildID string) (*v1.Pod, error) {
if pj.Spec.PodSpec == nil {
return nil, fmt.Errorf("prowjob %q lacks a pod spec?", pj.Name)
}
env, err := EnvForSpec(NewJobSpec(pj.Spec, buildID, pj.Name))
if err != nil {
return nil, err
Expand Down Expand Up @@ -249,7 +252,7 @@ func ProwJobFields(pj *kube.ProwJob) logrus.Fields {
if len(pj.ObjectMeta.Labels[github.EventGUID]) > 0 {
fields[github.EventGUID] = pj.ObjectMeta.Labels[github.EventGUID]
}
if len(pj.Spec.Refs.Pulls) == 1 {
if pj.Spec.Refs != nil && len(pj.Spec.Refs.Pulls) == 1 {
fields[github.PrLogField] = pj.Spec.Refs.Pulls[0].Number
fields[github.RepoLogField] = pj.Spec.Refs.Repo
fields[github.OrgLogField] = pj.Spec.Refs.Org
Expand Down
8 changes: 4 additions & 4 deletions prow/pjutil/pjutil_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestProwJobToPod(t *testing.T) {
Type: kube.PresubmitJob,
Job: "job-name",
Agent: kube.KubernetesAgent,
Refs: kube.Refs{
Refs: &kube.Refs{
Org: "org-name",
Repo: "repo-name",
BaseRef: "base-ref",
Expand All @@ -54,7 +54,7 @@ func TestProwJobToPod(t *testing.T) {
SHA: "pull-sha",
}},
},
PodSpec: v1.PodSpec{
PodSpec: &v1.PodSpec{
Containers: []v1.Container{
{
Image: "tester",
Expand Down Expand Up @@ -277,7 +277,7 @@ func TestGetLatestProwJobs(t *testing.T) {
Type: kube.PresubmitJob,
Agent: kube.JenkinsAgent,
Job: "test_pull_request_origin_extended_networking_minimal",
Refs: kube.Refs{
Refs: &kube.Refs{
Org: "openshift",
Repo: "origin",
BaseRef: "master",
Expand Down Expand Up @@ -311,7 +311,7 @@ func TestGetLatestProwJobs(t *testing.T) {
Type: kube.PresubmitJob,
Agent: kube.JenkinsAgent,
Job: "test_pull_request_origin_extended_networking_minimal",
Refs: kube.Refs{
Refs: &kube.Refs{
Org: "openshift",
Repo: "origin",
BaseRef: "master",
Expand Down
Loading

0 comments on commit e4a2701

Please sign in to comment.