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

feat: show error message when pipeline syntax is wrong #1392

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
36 changes: 26 additions & 10 deletions pkg/jobutil/jobutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func createRefs(pr *scm.PullRequest, baseSHA string, prRefFmt string) v1alpha1.R
// NewPresubmit converts a config.Presubmit into a builder.PipelineOptions.
// The builder.Refs are configured correctly per the pr, baseSHA.
// The eventGUID becomes a gitprovider.EventGUID label.
func NewPresubmit(logger *logrus.Entry, pr *scm.PullRequest, baseSHA string, job job.Presubmit, eventGUID string, prRefFmt string) v1alpha1.LighthouseJob {
func NewPresubmit(logger *logrus.Entry, pr *scm.PullRequest, baseSHA string, job job.Presubmit, eventGUID string, prRefFmt string) (v1alpha1.LighthouseJob, error) {
refs := createRefs(pr, baseSHA, prRefFmt)
labels := make(map[string]string)
for k, v := range job.Labels {
Expand All @@ -107,12 +107,19 @@ func NewPresubmit(logger *logrus.Entry, pr *scm.PullRequest, baseSHA string, job
annotations[k] = v
}
labels[scmprovider.EventGUID] = eventGUID
return NewLighthouseJob(PresubmitSpec(logger, job, refs), labels, annotations)
preSubmitSpec, err := PresubmitSpec(logger, job, refs)
if err != nil {
return v1alpha1.LighthouseJob{}, err
}
return NewLighthouseJob(preSubmitSpec, labels, annotations), nil
}

// PresubmitSpec initializes a PipelineOptionsSpec for a given presubmit job.
func PresubmitSpec(logger *logrus.Entry, p job.Presubmit, refs v1alpha1.Refs) v1alpha1.LighthouseJobSpec {
pjs := specFromJobBase(logger, p.Base)
func PresubmitSpec(logger *logrus.Entry, p job.Presubmit, refs v1alpha1.Refs) (v1alpha1.LighthouseJobSpec, error) {
pjs, err := specFromJobBase(logger, p.Base)
if err != nil {
return pjs, err
}
pjs.Type = job.PresubmitJob
pjs.Context = p.Context
pjs.RerunCommand = p.RerunCommand
Expand All @@ -124,12 +131,12 @@ func PresubmitSpec(logger *logrus.Entry, p job.Presubmit, refs v1alpha1.Refs) v1
}
}

return pjs
return pjs, nil
}

// PostsubmitSpec initializes a PipelineOptionsSpec for a given postsubmit job.
func PostsubmitSpec(logger *logrus.Entry, p job.Postsubmit, refs v1alpha1.Refs) v1alpha1.LighthouseJobSpec {
pjs := specFromJobBase(logger, p.Base)
pjs, _ := specFromJobBase(logger, p.Base)
pjs.Type = job.PostsubmitJob
pjs.Context = p.Context
pjs.Refs = completePrimaryRefs(refs, p.Base)
Expand All @@ -145,29 +152,38 @@ func PostsubmitSpec(logger *logrus.Entry, p job.Postsubmit, refs v1alpha1.Refs)

// PeriodicSpec initializes a PipelineOptionsSpec for a given periodic job.
func PeriodicSpec(logger *logrus.Entry, p job.Periodic) v1alpha1.LighthouseJobSpec {
pjs := specFromJobBase(logger, p.Base)
pjs, _ := specFromJobBase(logger, p.Base)
pjs.Type = job.PeriodicJob

return pjs
}

// BatchSpec initializes a PipelineOptionsSpec for a given batch job and ref spec.
func BatchSpec(logger *logrus.Entry, p job.Presubmit, refs v1alpha1.Refs) v1alpha1.LighthouseJobSpec {
pjs := specFromJobBase(logger, p.Base)
pjs, _ := specFromJobBase(logger, p.Base)
pjs.Type = job.BatchJob
pjs.Context = p.Context
pjs.Refs = completePrimaryRefs(refs, p.Base)

return pjs
}

func specFromJobBase(logger *logrus.Entry, jb job.Base) v1alpha1.LighthouseJobSpec {
func specFromJobBase(logger *logrus.Entry, jb job.Base) (v1alpha1.LighthouseJobSpec, error) {
// if we have not yet loaded the PipelineRunSpec then lets do it now
if jb.PipelineRunSpec == nil {
logger = logger.WithField("JobName", jb.Name)
err := jb.LoadPipeline(logger)
if err != nil {
logger.WithError(err).Warn("failed to lazy load the PipelineRunSpec")
return v1alpha1.LighthouseJobSpec{
Agent: jb.Agent,
Job: jb.Name,
Namespace: *jb.Namespace,
MaxConcurrency: jb.MaxConcurrency,
PodSpec: jb.Spec,
PipelineRunSpec: jb.PipelineRunSpec,
PipelineRunParams: jb.PipelineRunParams,
}, err
}
}
var namespace string
Expand All @@ -182,7 +198,7 @@ func specFromJobBase(logger *logrus.Entry, jb job.Base) v1alpha1.LighthouseJobSp
PodSpec: jb.Spec,
PipelineRunSpec: jb.PipelineRunSpec,
PipelineRunParams: jb.PipelineRunParams,
}
}, nil
}

func completePrimaryRefs(refs v1alpha1.Refs, jb job.Base) *v1alpha1.Refs {
Expand Down
4 changes: 2 additions & 2 deletions pkg/jobutil/jobutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func TestPresubmitSpec(t *testing.T) {
}

for _, tc := range tests {
actual := PresubmitSpec(logger, tc.p, tc.refs)
actual, _ := PresubmitSpec(logger, tc.p, tc.refs)
if expected := tc.expected; !reflect.DeepEqual(actual, expected) {
t.Errorf("%s: actual %#v != expected %#v", tc.name, actual, expected)
}
Expand Down Expand Up @@ -639,7 +639,7 @@ func TestSpecFromJobBase(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
pj := specFromJobBase(logger, tc.jobBase)
pj, _ := specFromJobBase(logger, tc.jobBase)
if err := tc.verify(pj); err != nil {
t.Fatalf("Verification failed: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,7 @@ func (c *DefaultController) trigger(sp subpool, presubmits map[int][]job.Presubm
triggeredContexts.Insert(ps.Context)
var spec v1alpha1.LighthouseJobSpec
if len(prs) == 1 {
spec = jobutil.PresubmitSpec(c.logger, ps, refs)
spec, _ = jobutil.PresubmitSpec(c.logger, ps, refs)
} else {
spec = jobutil.BatchSpec(c.logger, ps, refs)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/plugins/override/override.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ Only the following contexts were expected:
return spc.CreateComment(org, repo, number, e.IsPR, plugins.FormatResponseRaw(e.Body, e.Link, spc.QuoteAuthorForComment(user), resp))
}

pj := jobutil.NewPresubmit(log, pr, baseSHA, *pre, e.GUID, spc.PRRefFmt())
pj, _ := jobutil.NewPresubmit(log, pr, baseSHA, *pre, e.GUID, spc.PRRefFmt())
now := metav1.Now()
pj.Status = v1alpha1.LighthouseJobStatus{
State: v1alpha1.SuccessState,
Expand Down
16 changes: 15 additions & 1 deletion pkg/plugins/trigger/trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,10 @@ func runRequested(c Client, pr *scm.PullRequest, requestedJobs []job.Presubmit,
var errors []error
for _, job := range requestedJobs {
c.Logger.Infof("Starting %s build.", job.Name)
pj := jobutil.NewPresubmit(c.Logger, pr, baseSHA, job, eventGUID, c.SCMProviderClient.PRRefFmt())
pj, err := jobutil.NewPresubmit(c.Logger, pr, baseSHA, job, eventGUID, c.SCMProviderClient.PRRefFmt())
if err != nil {
errors = append(errors, err)
}
c.Logger.WithFields(jobutil.LighthouseJobFields(&pj)).Info("Creating a new LighthouseJob.")
if _, err := c.LauncherClient.Launch(&pj); err != nil {
c.Logger.WithError(err).Error("Failed to create LighthouseJob.")
Expand All @@ -291,6 +294,17 @@ func runRequested(c Client, pr *scm.PullRequest, requestedJobs []job.Presubmit,
}
}
}

if len(errors) > 0 {
org, repo, _ := orgRepoAuthor(*pr)
errorString := ""
for k := range errors {
errorString += "*" + errors[k].Error() + "\n"
}
message := "failed to trigger Pull Request pipeline\n" + errorString
c.SCMProviderClient.CreateComment(org, repo, pr.Number, true, message)
}

return errorutil.NewAggregate(errors...)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ func (o *WebhooksController) createHookServer() (*Server, error) {
Metrics: promMetrics,
ServerURL: serverURL,
InRepoCache: cache,
//TokenGenerator: secretAgent.GetTokenGenerator(o.webhookSecretFile),
// TokenGenerator: secretAgent.GetTokenGenerator(o.webhookSecretFile),
}
return server, nil
}
Expand Down