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

fix(skip): surface error when failing to determine whether a container should execute #524

Merged
merged 1 commit into from
Oct 17, 2023
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
7 changes: 6 additions & 1 deletion executor/linux/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,12 @@ func (c *client) ExecBuild(ctx context.Context) error {
// check if the step should be skipped
//
// https://pkg.go.dev/github.com/go-vela/worker/internal/step#Skip
if step.Skip(_step, c.build, c.repo) {
skip, err := step.Skip(_step, c.build, c.repo)
if err != nil {
return fmt.Errorf("unable to plan step: %w", c.err)
}

if skip {
continue
}

Expand Down
9 changes: 7 additions & 2 deletions executor/linux/stage.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,18 @@ func (c *client) ExecStage(ctx context.Context, s *pipeline.Stage, m *sync.Map)
// check if the step should be skipped
//
// https://pkg.go.dev/github.com/go-vela/worker/internal/step#Skip
if step.Skip(_step, c.build, c.repo) {
skip, err := step.Skip(_step, c.build, c.repo)
if err != nil {
return fmt.Errorf("unable to plan step: %w", c.err)
}

if skip {
continue
}

logger.Debugf("planning %s step", _step.Name)
// plan the step
err := c.PlanStep(ctx, _step)
err = c.PlanStep(ctx, _step)
if err != nil {
return fmt.Errorf("unable to plan step %s: %w", _step.Name, err)
}
Expand Down
7 changes: 6 additions & 1 deletion executor/local/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,12 @@ func (c *client) ExecBuild(ctx context.Context) error {
// check if the step should be skipped
//
// https://pkg.go.dev/github.com/go-vela/worker/internal/step#Skip
if step.Skip(_step, c.build, c.repo) {
skip, err := step.Skip(_step, c.build, c.repo)
if err != nil {
return fmt.Errorf("unable to plan step: %w", c.err)
}

if skip {
logrus.Infof("Skipping step %s due to ruleset", _step.Name)
continue
}
Expand Down
9 changes: 7 additions & 2 deletions executor/local/stage.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,18 @@ func (c *client) ExecStage(ctx context.Context, s *pipeline.Stage, m *sync.Map)
// check if the step should be skipped
//
// https://pkg.go.dev/github.com/go-vela/worker/internal/step#Skip
if step.Skip(_step, c.build, c.repo) {
skip, err := step.Skip(_step, c.build, c.repo)
if err != nil {
return fmt.Errorf("unable to plan step: %w", c.err)
}

if skip {
logrus.Infof("Skipping step %s due to ruleset", _step.Name)
continue
}

// plan the step
err := c.PlanStep(ctx, _step)
err = c.PlanStep(ctx, _step)
if err != nil {
return fmt.Errorf("unable to plan step %s: %w", _step.Name, err)
}
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ require (
github.com/docker/go-units v0.5.0
github.com/gin-gonic/gin v1.9.1
github.com/go-vela/sdk-go v0.21.0
github.com/go-vela/server v0.21.0
github.com/go-vela/types v0.21.0
github.com/go-vela/server v0.21.1-0.20231017135815-bb35e76f6056
github.com/go-vela/types v0.21.1-0.20231012142227-0c0b890487af
github.com/golang-jwt/jwt/v5 v5.0.0
github.com/google/go-cmp v0.5.9
github.com/joho/godotenv v1.5.1
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,10 @@ github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEe
github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572/go.mod h1:9Pwr4B2jHnOSGXyyzV8ROjYa2ojvAY6HCGYYfMoC3Ls=
github.com/go-vela/sdk-go v0.21.0 h1:Hedak1Yk9rGn3ZBOvLvxLrMcyvBf3+RB6/wMgHNxyxw=
github.com/go-vela/sdk-go v0.21.0/go.mod h1:fNMQxSqBCXQH6bK3Ej0aCj/iugEDZNEIWW3Xj/m22AQ=
github.com/go-vela/server v0.21.0 h1:tBSUMp1rni2i1wOzP2uxh7o6uTkzjrYfRe0fKJBOcNA=
github.com/go-vela/server v0.21.0/go.mod h1:3pp/hg5NUZ6VbbDC6JO97H7Ry7vv/qKA8GpWsaUpZ1M=
github.com/go-vela/types v0.21.0 h1:yZrVUw4jKO0JHaUBkOIZZdniDGyDOpTMbKriemdm1jg=
github.com/go-vela/types v0.21.0/go.mod h1:Jn8K28uj7mACc55fkFgaIzL0q45iXydOFGEeoSeHUtQ=
github.com/go-vela/server v0.21.1-0.20231017135815-bb35e76f6056 h1:TqLmvWRU3sqflw7kRUxDRw4H5g9JupLIp0JAGI8biG8=
github.com/go-vela/server v0.21.1-0.20231017135815-bb35e76f6056/go.mod h1:SFAAje/TsPxW+9iDo38CotLX0ralvPRLxbaS9fffT+A=
github.com/go-vela/types v0.21.1-0.20231012142227-0c0b890487af h1:qiP6pXFDyPDDP+hy8zY+nhmoWv9aoQrrnNmfAAT6yCA=
github.com/go-vela/types v0.21.1-0.20231012142227-0c0b890487af/go.mod h1:Jn8K28uj7mACc55fkFgaIzL0q45iXydOFGEeoSeHUtQ=
github.com/goccy/go-json v0.10.2 h1:CrxCmQqYDkv1z7lO7Wbh2HN93uovUHgrECaO5ZrCXAU=
github.com/goccy/go-json v0.10.2/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I=
github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
Expand Down
8 changes: 5 additions & 3 deletions internal/step/skip.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import (
// Skip creates the ruledata from the build and repository
// information and returns true if the data does not match
// the ruleset for the given container.
func Skip(c *pipeline.Container, b *library.Build, r *library.Repo) bool {
func Skip(c *pipeline.Container, b *library.Build, r *library.Repo) (bool, error) {
// check if the container provided is empty
if c == nil {
return true
return true, nil
}

event := b.GetEvent()
Expand Down Expand Up @@ -64,5 +64,7 @@ func Skip(c *pipeline.Container, b *library.Build, r *library.Repo) bool {
// return the inverse of container execute
//
// https://pkg.go.dev/github.com/go-vela/types/pipeline#Container.Execute
return !c.Execute(ruledata)
exec, err := c.Execute(ruledata)

return !exec, err
}
5 changes: 4 additions & 1 deletion internal/step/skip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,10 @@ func TestStep_Skip(t *testing.T) {
// run test
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
got := Skip(test.container, test.build, test.repo)
got, err := Skip(test.container, test.build, test.repo)
if err != nil {
t.Errorf("Skip returned error: %s", err)
}

if got != test.want {
t.Errorf("Skip is %v, want %v", got, test.want)
Expand Down