Skip to content

Commit 60bf224

Browse files
ankitm123ankitm123
committed
feat: show error message when pipeline syntax is wrong
Signed-off-by: ankitm123 <[email protected]>
1 parent bc4b229 commit 60bf224

File tree

6 files changed

+77
-49
lines changed

6 files changed

+77
-49
lines changed

‎pkg/jobutil/jobutil.go

+26-10
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func createRefs(pr *scm.PullRequest, baseSHA string, prRefFmt string) v1alpha1.R
9595
// NewPresubmit converts a config.Presubmit into a builder.PipelineOptions.
9696
// The builder.Refs are configured correctly per the pr, baseSHA.
9797
// The eventGUID becomes a gitprovider.EventGUID label.
98-
func NewPresubmit(logger *logrus.Entry, pr *scm.PullRequest, baseSHA string, job job.Presubmit, eventGUID string, prRefFmt string) v1alpha1.LighthouseJob {
98+
func NewPresubmit(logger *logrus.Entry, pr *scm.PullRequest, baseSHA string, job job.Presubmit, eventGUID string, prRefFmt string) (v1alpha1.LighthouseJob, error) {
9999
refs := createRefs(pr, baseSHA, prRefFmt)
100100
labels := make(map[string]string)
101101
for k, v := range job.Labels {
@@ -106,12 +106,19 @@ func NewPresubmit(logger *logrus.Entry, pr *scm.PullRequest, baseSHA string, job
106106
annotations[k] = v
107107
}
108108
labels[scmprovider.EventGUID] = eventGUID
109-
return NewLighthouseJob(PresubmitSpec(logger, job, refs), labels, annotations)
109+
preSubmitSpec, err := PresubmitSpec(logger, job, refs)
110+
if err != nil {
111+
return v1alpha1.LighthouseJob{}, err
112+
}
113+
return NewLighthouseJob(preSubmitSpec, labels, annotations), nil
110114
}
111115

112116
// PresubmitSpec initializes a PipelineOptionsSpec for a given presubmit job.
113-
func PresubmitSpec(logger *logrus.Entry, p job.Presubmit, refs v1alpha1.Refs) v1alpha1.LighthouseJobSpec {
114-
pjs := specFromJobBase(logger, p.Base)
117+
func PresubmitSpec(logger *logrus.Entry, p job.Presubmit, refs v1alpha1.Refs) (v1alpha1.LighthouseJobSpec, error) {
118+
pjs, err := specFromJobBase(logger, p.Base)
119+
if err != nil {
120+
return pjs, err
121+
}
115122
pjs.Type = job.PresubmitJob
116123
pjs.Context = p.Context
117124
pjs.RerunCommand = p.RerunCommand
@@ -123,12 +130,12 @@ func PresubmitSpec(logger *logrus.Entry, p job.Presubmit, refs v1alpha1.Refs) v1
123130
}
124131
}
125132

126-
return pjs
133+
return pjs, nil
127134
}
128135

129136
// PostsubmitSpec initializes a PipelineOptionsSpec for a given postsubmit job.
130137
func PostsubmitSpec(logger *logrus.Entry, p job.Postsubmit, refs v1alpha1.Refs) v1alpha1.LighthouseJobSpec {
131-
pjs := specFromJobBase(logger, p.Base)
138+
pjs, _ := specFromJobBase(logger, p.Base)
132139
pjs.Type = job.PostsubmitJob
133140
pjs.Context = p.Context
134141
pjs.Refs = completePrimaryRefs(refs, p.Base)
@@ -144,29 +151,38 @@ func PostsubmitSpec(logger *logrus.Entry, p job.Postsubmit, refs v1alpha1.Refs)
144151

145152
// PeriodicSpec initializes a PipelineOptionsSpec for a given periodic job.
146153
func PeriodicSpec(logger *logrus.Entry, p job.Periodic) v1alpha1.LighthouseJobSpec {
147-
pjs := specFromJobBase(logger, p.Base)
154+
pjs, _ := specFromJobBase(logger, p.Base)
148155
pjs.Type = job.PeriodicJob
149156

150157
return pjs
151158
}
152159

153160
// BatchSpec initializes a PipelineOptionsSpec for a given batch job and ref spec.
154161
func BatchSpec(logger *logrus.Entry, p job.Presubmit, refs v1alpha1.Refs) v1alpha1.LighthouseJobSpec {
155-
pjs := specFromJobBase(logger, p.Base)
162+
pjs, _ := specFromJobBase(logger, p.Base)
156163
pjs.Type = job.BatchJob
157164
pjs.Context = p.Context
158165
pjs.Refs = completePrimaryRefs(refs, p.Base)
159166

160167
return pjs
161168
}
162169

163-
func specFromJobBase(logger *logrus.Entry, jb job.Base) v1alpha1.LighthouseJobSpec {
170+
func specFromJobBase(logger *logrus.Entry, jb job.Base) (v1alpha1.LighthouseJobSpec, error) {
164171
// if we have not yet loaded the PipelineRunSpec then lets do it now
165172
if jb.PipelineRunSpec == nil {
166173
logger = logger.WithField("JobName", jb.Name)
167174
err := jb.LoadPipeline(logger)
168175
if err != nil {
169176
logger.WithError(err).Warn("failed to lazy load the PipelineRunSpec")
177+
return v1alpha1.LighthouseJobSpec{
178+
Agent: jb.Agent,
179+
Job: jb.Name,
180+
Namespace: *jb.Namespace,
181+
MaxConcurrency: jb.MaxConcurrency,
182+
PodSpec: jb.Spec,
183+
PipelineRunSpec: jb.PipelineRunSpec,
184+
PipelineRunParams: jb.PipelineRunParams,
185+
}, err
170186
}
171187
}
172188
var namespace string
@@ -181,7 +197,7 @@ func specFromJobBase(logger *logrus.Entry, jb job.Base) v1alpha1.LighthouseJobSp
181197
PodSpec: jb.Spec,
182198
PipelineRunSpec: jb.PipelineRunSpec,
183199
PipelineRunParams: jb.PipelineRunParams,
184-
}
200+
}, nil
185201
}
186202

187203
func completePrimaryRefs(refs v1alpha1.Refs, jb job.Base) *v1alpha1.Refs {

‎pkg/jobutil/jobutil_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ func TestPresubmitSpec(t *testing.T) {
204204
}
205205

206206
for _, tc := range tests {
207-
actual := PresubmitSpec(logger, tc.p, tc.refs)
207+
actual, _ := PresubmitSpec(logger, tc.p, tc.refs)
208208
if expected := tc.expected; !reflect.DeepEqual(actual, expected) {
209209
t.Errorf("%s: actual %#v != expected %#v", tc.name, actual, expected)
210210
}
@@ -592,7 +592,7 @@ func TestSpecFromJobBase(t *testing.T) {
592592

593593
for _, tc := range testCases {
594594
t.Run(tc.name, func(t *testing.T) {
595-
pj := specFromJobBase(logger, tc.jobBase)
595+
pj, _ := specFromJobBase(logger, tc.jobBase)
596596
if err := tc.verify(pj); err != nil {
597597
t.Fatalf("Verification failed: %v", err)
598598
}

‎pkg/keeper/keeper.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1201,7 +1201,7 @@ func (c *DefaultController) trigger(sp subpool, presubmits map[int][]job.Presubm
12011201
triggeredContexts.Insert(ps.Context)
12021202
var spec v1alpha1.LighthouseJobSpec
12031203
if len(prs) == 1 {
1204-
spec = jobutil.PresubmitSpec(c.logger, ps, refs)
1204+
spec, _ = jobutil.PresubmitSpec(c.logger, ps, refs)
12051205
} else {
12061206
spec = jobutil.BatchSpec(c.logger, ps, refs)
12071207
}

‎pkg/plugins/override/override.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ Only the following contexts were expected:
206206
return spc.CreateComment(org, repo, number, e.IsPR, plugins.FormatResponseRaw(e.Body, e.Link, spc.QuoteAuthorForComment(user), resp))
207207
}
208208

209-
pj := jobutil.NewPresubmit(log, pr, baseSHA, *pre, e.GUID, spc.PRRefFmt())
209+
pj, _ := jobutil.NewPresubmit(log, pr, baseSHA, *pre, e.GUID, spc.PRRefFmt())
210210
now := metav1.Now()
211211
pj.Status = v1alpha1.LighthouseJobStatus{
212212
State: v1alpha1.SuccessState,

‎pkg/plugins/trigger/trigger.go

+46-34
Original file line numberDiff line numberDiff line change
@@ -41,41 +41,39 @@ const (
4141
customerTriggerCommandEnvVar = "LH_CUSTOM_TRIGGER_COMMAND"
4242
)
4343

44-
var (
45-
plugin = plugins.Plugin{
46-
Description: `The trigger plugin starts tests in reaction to commands and pull request events. It is responsible for ensuring that test jobs are only run on trusted PRs. A PR is considered trusted if the author is a member of the 'trusted organization' for the repository or if such a member has left an '/ok-to-test' command on the PR.
44+
var plugin = plugins.Plugin{
45+
Description: `The trigger plugin starts tests in reaction to commands and pull request events. It is responsible for ensuring that test jobs are only run on trusted PRs. A PR is considered trusted if the author is a member of the 'trusted organization' for the repository or if such a member has left an '/ok-to-test' command on the PR.
4746
<br>Trigger starts jobs automatically when a new trusted PR is created or when an untrusted PR becomes trusted, but it can also be used to start jobs manually via the '/test' command.
4847
<br>The '/retest' command can be used to rerun jobs that have reported failure.`,
49-
ConfigHelpProvider: configHelp,
50-
PullRequestHandler: handlePullRequest,
51-
PushEventHandler: handlePush,
52-
Commands: []plugins.Command{{
53-
Name: "ok-to-test",
54-
Description: "Marks a PR as 'trusted' and starts tests.",
55-
WhoCanUse: "Members of the trusted organization for the repo.",
56-
Action: plugins.
57-
Invoke(handleGenericCommentEvent).
58-
When(plugins.Action(scm.ActionCreate), plugins.IsPR(), plugins.IssueState("open")),
59-
}, {
60-
Name: "test",
61-
Arg: &plugins.CommandArg{
62-
Pattern: `[-\w]+(?:,[-\w]+)*`,
63-
},
64-
Description: "Manually starts a/all test job(s).",
65-
Featured: true,
66-
Action: plugins.
67-
Invoke(handleGenericCommentEvent).
68-
When(plugins.Action(scm.ActionCreate), plugins.IsPR(), plugins.IssueState("open")),
69-
}, {
70-
Name: "retest",
71-
Description: "Rerun test jobs that have failed.",
72-
Featured: true,
73-
Action: plugins.
74-
Invoke(handleGenericCommentEvent).
75-
When(plugins.Action(scm.ActionCreate), plugins.IsPR(), plugins.IssueState("open")),
76-
}},
77-
}
78-
)
48+
ConfigHelpProvider: configHelp,
49+
PullRequestHandler: handlePullRequest,
50+
PushEventHandler: handlePush,
51+
Commands: []plugins.Command{{
52+
Name: "ok-to-test",
53+
Description: "Marks a PR as 'trusted' and starts tests.",
54+
WhoCanUse: "Members of the trusted organization for the repo.",
55+
Action: plugins.
56+
Invoke(handleGenericCommentEvent).
57+
When(plugins.Action(scm.ActionCreate), plugins.IsPR(), plugins.IssueState("open")),
58+
}, {
59+
Name: "test",
60+
Arg: &plugins.CommandArg{
61+
Pattern: `[-\w]+(?:,[-\w]+)*`,
62+
},
63+
Description: "Manually starts a/all test job(s).",
64+
Featured: true,
65+
Action: plugins.
66+
Invoke(handleGenericCommentEvent).
67+
When(plugins.Action(scm.ActionCreate), plugins.IsPR(), plugins.IssueState("open")),
68+
}, {
69+
Name: "retest",
70+
Description: "Rerun test jobs that have failed.",
71+
Featured: true,
72+
Action: plugins.
73+
Invoke(handleGenericCommentEvent).
74+
When(plugins.Action(scm.ActionCreate), plugins.IsPR(), plugins.IssueState("open")),
75+
}},
76+
}
7977

8078
func init() {
8179
customTriggerCommand := os.Getenv(customerTriggerCommandEnvVar)
@@ -283,7 +281,10 @@ func runRequested(c Client, pr *scm.PullRequest, requestedJobs []job.Presubmit,
283281
var errors []error
284282
for _, job := range requestedJobs {
285283
c.Logger.Infof("Starting %s build.", job.Name)
286-
pj := jobutil.NewPresubmit(c.Logger, pr, baseSHA, job, eventGUID, c.SCMProviderClient.PRRefFmt())
284+
pj, err := jobutil.NewPresubmit(c.Logger, pr, baseSHA, job, eventGUID, c.SCMProviderClient.PRRefFmt())
285+
if err != nil {
286+
errors = append(errors, err)
287+
}
287288
c.Logger.WithFields(jobutil.LighthouseJobFields(&pj)).Info("Creating a new LighthouseJob.")
288289
if _, err := c.LauncherClient.Launch(&pj); err != nil {
289290
c.Logger.WithError(err).Error("Failed to create LighthouseJob.")
@@ -293,6 +294,17 @@ func runRequested(c Client, pr *scm.PullRequest, requestedJobs []job.Presubmit,
293294
}
294295
}
295296
}
297+
298+
if len(errors) > 0 {
299+
org, repo, _ := orgRepoAuthor(*pr)
300+
errorString := ""
301+
for k := range errors {
302+
errorString += "*" + errors[k].Error() + "\n"
303+
}
304+
message := "failed to trigger Pull Request pipeline\n" + errorString
305+
c.SCMProviderClient.CreateComment(org, repo, pr.Number, true, message)
306+
}
307+
296308
return errorutil.NewAggregate(errors...)
297309
}
298310

‎pkg/webhook/webhook.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ func (o *WebhooksController) createHookServer() (*Server, error) {
520520
Metrics: promMetrics,
521521
ServerURL: serverURL,
522522
InRepoCache: cache,
523-
//TokenGenerator: secretAgent.GetTokenGenerator(o.webhookSecretFile),
523+
// TokenGenerator: secretAgent.GetTokenGenerator(o.webhookSecretFile),
524524
}
525525
return server, nil
526526
}

0 commit comments

Comments
 (0)