Skip to content

Commit

Permalink
fix(skip): surface error when failing to determine whether a containe…
Browse files Browse the repository at this point in the history
…r should execute (#524)
  • Loading branch information
ecrupper authored Oct 17, 2023
1 parent 26628ba commit bcdf8e4
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 16 deletions.
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

0 comments on commit bcdf8e4

Please sign in to comment.