Skip to content

Commit 9452f0f

Browse files
committed
Merge branch 'ajwalker/script-to-script-step' into 'main'
Implement user script to step See merge request https://gitlab.com/gitlab-org/gitlab-runner/-/merge_requests/6069 Merged-by: Arran Walker <ajwalker@gitlab.com> Approved-by: Cameron Swords <cswords@gitlab.com> Approved-by: Roshni Sarangadharan <rsarangadharan@gitlab.com> Reviewed-by: Cameron Swords <cswords@gitlab.com> Reviewed-by: GitLab Duo <gitlab-duo@gitlab.com>
2 parents 0983f71 + 78236de commit 9452f0f

24 files changed

Lines changed: 437 additions & 212 deletions

File tree

.gitlab/ci/test.gitlab-ci.yml

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,27 @@ integration test:
7777
-- -ldflags "$(make print_test_ldflags)" -timeout 25m
7878
parallel: 4
7979

80+
integration test (docker, steps):
81+
extends:
82+
- integration test
83+
variables:
84+
RUNNER_TEST_FEATURE_FLAGS: "FF_SCRIPT_TO_STEP_MIGRATION"
85+
script:
86+
- docker import out/helper-images/prebuilt-alpine-latest-x86_64.tar.xz registry.gitlab.com/gitlab-org/gitlab-runner/gitlab-runner-helper:x86_64-latest
87+
- go install gitlab.com/gitlab-org/fleeting/fleeting-plugin-static/cmd/fleeting-plugin-static@latest
88+
- make splitic
89+
- >
90+
.tmp/bin/splitic test \
91+
-junit-report .splitic/junit_${CI_NODE_INDEX}_docker_steps.xml \
92+
-cover-report .splitic/cover_${CI_NODE_INDEX}_docker_steps.profile -cover -coverpkg gitlab.com/gitlab-org/gitlab-runner/... \
93+
-env-passthrough ./scripts/envs/allowlist_common.env -env-passthrough ./scripts/envs/allowlist_unix.env \
94+
-tags integration \
95+
./executors/docker \
96+
-- -ldflags "$(make print_test_ldflags)" -timeout 1h
97+
parallel: 1
98+
when: manual
99+
allow_failure: true
100+
80101
integration test with race:
81102
extends:
82103
- integration test

commands/multi.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import (
3737
"gitlab.com/gitlab-org/gitlab-runner/log"
3838
"gitlab.com/gitlab-org/gitlab-runner/network"
3939
"gitlab.com/gitlab-org/gitlab-runner/session"
40-
"gitlab.com/gitlab-org/gitlab-runner/steps"
4140
)
4241

4342
const (
@@ -971,7 +970,6 @@ func (mr *RunCommand) processBuildOnRunner(
971970
}
972971
build.Session = buildSession
973972
build.ArtifactUploader = mr.network.UploadRawArtifacts
974-
build.ExecuteStepFn = steps.Execute
975973

976974
trace.SetDebugModeEnabled(build.IsDebugModeEnabled())
977975

commands/steps/steps.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"google.golang.org/grpc"
1919

2020
"gitlab.com/gitlab-org/gitlab-runner/common"
21+
"gitlab.com/gitlab-org/gitlab-runner/functions/script_legacy"
2122
"gitlab.com/gitlab-org/step-runner/pkg/api"
2223
"gitlab.com/gitlab-org/step-runner/pkg/api/proxy"
2324
"gitlab.com/gitlab-org/step-runner/pkg/di"
@@ -75,7 +76,10 @@ func Serve(ctx context.Context, sockPath string, ioStreams IOStreams, cmdAndArgs
7576
}
7677
defer listener.Close()
7778

78-
service, err := di.NewContainer().StepRunnerService()
79+
service, err := di.NewContainer(
80+
di.WithStepFunc("script_legacy", script_legacy.Spec(), script_legacy.Run),
81+
).StepRunnerService()
82+
7983
if err != nil {
8084
return fmt.Errorf("initializing step-runner: %w", err)
8185
}

common/build.go

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ import (
3030
"gitlab.com/gitlab-org/gitlab-runner/session"
3131
"gitlab.com/gitlab-org/gitlab-runner/session/proxy"
3232
"gitlab.com/gitlab-org/gitlab-runner/session/terminal"
33+
"gitlab.com/gitlab-org/gitlab-runner/steps"
34+
"gitlab.com/gitlab-org/step-runner/pkg/api/client"
35+
"gitlab.com/gitlab-org/step-runner/schema/v1"
3336
)
3437

3538
type BuildRuntimeState string
@@ -159,8 +162,6 @@ type Build struct {
159162
Referees []referees.Referee
160163
ArtifactUploader func(config JobCredentials, bodyProvider ContentProvider, options ArtifactsOptions) (UploadState, string)
161164

162-
ExecuteStepFn func(ctx context.Context, connector Connector, build *Build, trace JobTrace) error
163-
164165
urlHelper urlHelper
165166

166167
OnBuildStageStartFn OnBuildStageFn
@@ -407,7 +408,7 @@ func (b *Build) StartBuild(
407408
return nil
408409
}
409410

410-
func (b *Build) executeStepStage(ctx context.Context, connector Connector, buildStage BuildStage, trace JobTrace) error {
411+
func (b *Build) executeStepStage(ctx context.Context, connector steps.Connector, buildStage BuildStage, req []schema.Step) error {
411412
if ctx.Err() != nil {
412413
return ctx.Err()
413414
}
@@ -430,14 +431,53 @@ func (b *Build) executeStepStage(ctx context.Context, connector Connector, build
430431
)
431432
b.logger.Println(msg)
432433

433-
return b.ExecuteStepFn(ctx, connector, b, trace)
434+
// todo: step-runner should eventually:
435+
// - format its own logs to the Runner log spec
436+
// - provides its own timestamps and mask its own secrets
437+
// for now though, we wrap its logs providing this, and treat everything as stdout
438+
stdout := b.logger.Stream(buildlogger.StreamWorkLevel, buildlogger.Stdout)
439+
440+
info := steps.JobInfo{
441+
ID: b.ID,
442+
ProjectDir: b.FullProjectDir(),
443+
Variables: b.GetAllVariables(),
444+
}
445+
446+
err := steps.Execute(ctx, connector, info, req, stdout)
447+
if err != nil {
448+
berr := &BuildError{Inner: err}
449+
450+
var cserr *steps.ClientStatusError
451+
if errors.As(err, &cserr) {
452+
switch cserr.Status.State {
453+
case client.StateUnspecified:
454+
berr.FailureReason = UnknownFailure
455+
case client.StateFailure:
456+
berr.FailureReason = ScriptFailure
457+
}
458+
}
459+
460+
return berr
461+
}
462+
463+
return err
434464
},
435465
}
436466

437467
return section.Execute(&b.logger)
438468
}
439469

470+
//nolint:gocognit
440471
func (b *Build) executeStage(ctx context.Context, buildStage BuildStage, executor Executor) error {
472+
if b.UseNativeSteps() {
473+
connector, ok := executor.(steps.Connector)
474+
if ok {
475+
if handled, steps := stepDispatch(b, executor, buildStage); handled {
476+
return b.executeStepStage(ctx, connector, buildStage, steps)
477+
}
478+
}
479+
}
480+
441481
if ctx.Err() != nil {
442482
return ctx.Err()
443483
}
@@ -585,13 +625,11 @@ func (b *Build) executeScript(ctx context.Context, trace JobTrace, executor Exec
585625
// execute user provided scripts
586626
//nolint:nestif
587627
if err == nil {
588-
if b.UseNativeSteps() && b.ExecuteStepFn != nil {
589-
connector, ok := executor.(Connector)
590-
if ok {
591-
err = b.executeStepStage(ctx, connector, BuildStage("step_"+spec.StepNameRun), trace)
592-
} else {
628+
if b.UseNativeSteps() && len(b.Job.Run) > 0 {
629+
if _, ok := executor.(steps.Connector); !ok {
593630
return ExecutorStepRunnerConnectNotSupported
594631
}
632+
err = b.executeStage(ctx, stepRunBuildStage, executor)
595633
} else {
596634
err = b.executeUserScripts(ctx, trace, executor)
597635
}

common/build_settings.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
package common
22

33
import (
4+
"flag"
45
"fmt"
6+
"os"
57
"slices"
68
"strconv"
79
"strings"
10+
"unicode"
811

912
"gitlab.com/gitlab-org/gitlab-runner/common/spec"
1013
"gitlab.com/gitlab-org/gitlab-runner/helpers/featureflags"
@@ -218,13 +221,31 @@ func validate[T any](variables spec.Variables, name string, value *T, def T) err
218221
return nil
219222
}
220223

224+
//nolint:gocognit
221225
func populateFeatureFlags(b *Build, variables spec.Variables) []error {
222226
var errs []error
223227

228+
// test mode only: in tests, we provide a mechanism for providing
229+
// feature flags via RUNNER_TEST_FEATURE_FLAGS, if the flag is present,
230+
// we treat it as a toggle to the default flag value.
231+
var testFlags []string
232+
if flag.Lookup("test.v") != nil {
233+
testFlags = strings.FieldsFunc(os.Getenv("RUNNER_TEST_FEATURE_FLAGS"), func(r rune) bool {
234+
return r == ',' || unicode.IsSpace(r)
235+
})
236+
}
237+
224238
b.buildSettings.FeatureFlags = make(map[string]bool)
225239
for _, ff := range featureflags.GetAll() {
226240
b.buildSettings.FeatureFlags[ff.Name] = ff.DefaultValue
227241

242+
if len(testFlags) > 0 {
243+
if slices.Contains(testFlags, ff.Name) {
244+
b.buildSettings.FeatureFlags[ff.Name] = !ff.DefaultValue
245+
continue
246+
}
247+
}
248+
228249
// runner setting takes precedence if defined
229250
if b.Runner != nil && b.Runner.FeatureFlags != nil {
230251
val, ok := b.Runner.FeatureFlags[ff.Name]

common/build_step_dispatch.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
package common
2+
3+
import (
4+
"gitlab.com/gitlab-org/gitlab-runner/common/spec"
5+
"gitlab.com/gitlab-org/gitlab-runner/helpers/featureflags"
6+
"gitlab.com/gitlab-org/step-runner/schema/v1"
7+
)
8+
9+
const stepRunBuildStage = BuildStage("step_" + spec.StepNameRun)
10+
11+
// stepDispatch converts a build stage to a list of steps to run.
12+
//
13+
// Depending on the configuration, this can also include stages that we're
14+
// in the process of migrating (from scripts) to a step.
15+
//
16+
//nolint:gocognit
17+
func stepDispatch(build *Build, executor Executor, stage BuildStage) (bool, []schema.Step) {
18+
switch stage {
19+
case BuildStagePrepare, BuildStageGetSources, BuildStageClearWorktree, BuildStageRestoreCache, BuildStageDownloadArtifacts, BuildStageArchiveOnSuccessCache, BuildStageArchiveOnFailureCache, BuildStageUploadOnFailureArtifacts, BuildStageUploadOnSuccessArtifacts, BuildStageCleanup:
20+
// don't handle non-user script stages
21+
return false, nil
22+
23+
case stepRunBuildStage:
24+
return true, build.Job.Run
25+
26+
case BuildStageAfterScript:
27+
// don't handle after_script (yet)
28+
return false, nil
29+
30+
default: // user script
31+
if !build.IsFeatureFlagOn(featureflags.UseScriptToStepMigration) {
32+
return false, nil
33+
}
34+
35+
shell := executor.Shell()
36+
if shell == nil {
37+
return false, nil
38+
}
39+
40+
var script []string
41+
42+
if shell.PreBuildScript != "" {
43+
script = append(script, shell.PreBuildScript)
44+
}
45+
46+
for _, step := range build.Steps {
47+
if StepToBuildStage(step) == stage {
48+
script = append(script, step.Script...)
49+
if step.Name == "release" {
50+
for i, s := range step.Script {
51+
script[i] = build.GetAllVariables().ExpandValue(s)
52+
}
53+
}
54+
break
55+
}
56+
}
57+
58+
if shell.PostBuildScript != "" {
59+
script = append(script, shell.PostBuildScript)
60+
}
61+
62+
// if no script, no-op
63+
if len(script) == 0 {
64+
return true, nil // handled, but nothing to do
65+
}
66+
67+
return true, []schema.Step{
68+
{
69+
Name: func(s string) *string { return &s }("user_script"),
70+
Step: "builtin://script_legacy",
71+
Inputs: schema.StepInputs{
72+
"script": script,
73+
"debug_trace": build.IsDebugTraceEnabled(),
74+
"posix_escape": true,
75+
"check_for_errors": build.IsFeatureFlagOn(featureflags.EnableBashExitCodeCheck),
76+
"trace_sections": build.IsFeatureFlagOn(featureflags.ScriptSections),
77+
},
78+
},
79+
}
80+
}
81+
}

common/buildtest/masking.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ func RunBuildWithMasking(t *testing.T, config *common.RunnerConfig, setup BuildS
2828
"Job failed: exit status 1",
2929
"Job failed: run exit (exit code: 1)",
3030
"Job failed: command terminated with exit code 1",
31+
"Job failed: step \"user_script\": exec: executing script: exit status 1",
3132
}
3233

3334
build := &common.Build{

common/executor.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"errors"
66
"fmt"
7-
"io"
87

98
"github.com/sirupsen/logrus"
109

@@ -79,10 +78,6 @@ type Executor interface {
7978

8079
var ExecutorStepRunnerConnectNotSupported = fmt.Errorf("executor does not support step-runner connect")
8180

82-
type Connector interface {
83-
Connect(ctx context.Context) (io.ReadWriteCloser, error)
84-
}
85-
8681
type ManagedExecutorProvider interface {
8782
// Init initializes the executor provider.
8883
//

0 commit comments

Comments
 (0)