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.

Sign up for GitHub

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

Use descriptive container names #4670

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Use descriptive container names #4670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7700e0e
use step name in container name
pat-s Jan 5, 2025
a0d8c70
extract pipeline_name from metadata to agent and add to container names
pat-s Jan 5, 2025
2db8752
switch order of elements in name
pat-s Jan 5, 2025
5a0c941
set max length for names
pat-s Jan 5, 2025
1820517
pipelineName -> workflowName
pat-s Jan 6, 2025
849665d
Merge branch 'main' into feat/descriptive-pod-name
pat-s Jan 7, 2025
5bcb00f
use 5 last chars of uuid
pat-s Jan 7, 2025
5b6bf0e
use google/uuid for random 5 char string
pat-s Jan 7, 2025
5873d01
tests
pat-s Jan 7, 2025
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
6 changes: 6 additions & 0 deletions agent/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@

repoName := extractRepositoryName(workflow.Config) // hack
pipelineNumber := extractPipelineNumber(workflow.Config) // hack
workflowName := extractWorkflowName(workflow.Config) // hack

Check warning on line 75 in agent/runner.go

View check run for this annotation

Codecov / codecov/patch

agent/runner.go#L75 

Added line #L75 was not covered by tests

r.counter.Add(
workflow.ID,
Expand Down Expand Up @@ -149,6 +150,7 @@
"workflow_id": workflow.ID,
"repo": repoName,
"pipeline_number": pipelineNumber,
"workflowName": workflowName,

Check warning on line 153 in agent/runner.go

View check run for this annotation

Codecov / codecov/patch

agent/runner.go#L153 

Added line #L153 was not covered by tests
}),
).Run(runnerCtx)

Expand Down Expand Up @@ -197,3 +199,7 @@
func extractPipelineNumber(config *backend.Config) string {
return config.Stages[0].Steps[0].Environment["CI_PIPELINE_NUMBER"]
}

func extractWorkflowName(config *backend.Config) string {
return config.Stages[0].Steps[0].Environment["CI_WORKFLOW_NAME"]

Check warning on line 204 in agent/runner.go

View check run for this annotation

Codecov / codecov/patch

agent/runner.go#L203-L204 

Added lines #L203 - L204 were not covered by tests
}
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ require (
github.com/golang-jwt/jwt/v5 v5.2.1
github.com/google/go-github/v68 v68.0.0
github.com/google/tink/go v1.7.0
github.com/google/uuid v1.6.0
github.com/gorilla/securecookie v1.1.2
github.com/hashicorp/go-hclog v1.6.3
github.com/hashicorp/go-plugin v1.6.2
Expand Down Expand Up @@ -131,7 +132,6 @@ require (
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/go-querystring v1.1.0 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
github.com/hashicorp/go-retryablehttp v0.7.7 // indirect
github.com/hashicorp/go-version v1.7.0 // indirect
Expand Down
4 changes: 2 additions & 2 deletions pipeline/backend/docker/convert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ func (e *docker) toConfig(step *types.Step, options BackendOptions) *container.C
return config
}

func toContainerName(step *types.Step) string {
return "wp_" + step.UUID
func toContainerName(step *types.Step, workflowName string) string {
return "wp_" + workflowName + "-" + step.Name + "-" + step.UUID
}

// returns a container host configuration.
Expand Down
8 changes: 4 additions & 4 deletions pipeline/backend/docker/convert_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func TestSplitVolumeParts(t *testing.T) {
var (
testCmdStep = &backend.Step{
Name: "hello",
UUID: "f51821af-4cb8-435e-a3c2-3a684185d828",
UUID: "f5182",
Type: backend.StepTypeCommands,
Commands: []string{"echo \"hello world\"", "ls"},
Image: "alpine",
Expand All @@ -106,7 +106,7 @@ var (

testPluginStep = &backend.Step{
Name: "lint",
UUID: "d841ee40-e66e-4275-bb3f-55bf89744b21",
UUID: "d841e",
Type: backend.StepTypePlugin,
Image: "mstruebing/editorconfig-checker",
Environment: make(map[string]string),
Expand All @@ -125,8 +125,8 @@ var (
)

func TestToContainerName(t *testing.T) {
assert.EqualValues(t, "wp_f51821af-4cb8-435e-a3c2-3a684185d828", toContainerName(testCmdStep))
assert.EqualValues(t, "wp_d841ee40-e66e-4275-bb3f-55bf89744b21", toContainerName(testPluginStep))
assert.EqualValues(t, "wp_workflowNameTest-hello-f5182", toContainerName(testCmdStep, "workflowNameTest"))
assert.EqualValues(t, "wp_workflowNameTest-lint-d841e", toContainerName(testPluginStep, "workflowNameTest"))
}

func TestStepToConfig(t *testing.T) {
Expand Down
26 changes: 13 additions & 13 deletions pipeline/backend/docker/docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@
}, nil
}

func (e *docker) SetupWorkflow(ctx context.Context, conf *backend.Config, taskUUID string) error {
log.Trace().Str("taskUUID", taskUUID).Msg("create workflow environment")
func (e *docker) SetupWorkflow(ctx context.Context, conf *backend.Config, taskUUID, workflowName string) error {
log.Trace().Str("taskUUID", taskUUID).Str("workflowName", workflowName).Msg("create workflow environment")

Check warning on line 145 in pipeline/backend/docker/docker.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/docker/docker.go#L144-L145 

Added lines #L144 - L145 were not covered by tests

_, err := e.client.VolumeCreate(ctx, volume.CreateOptions{
Name: conf.Volume.Name,
Expand All @@ -163,17 +163,17 @@
return err
}

func (e *docker) StartStep(ctx context.Context, step *backend.Step, taskUUID string) error {
func (e *docker) StartStep(ctx context.Context, step *backend.Step, taskUUID, workflowName string) error {

Check warning on line 166 in pipeline/backend/docker/docker.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/docker/docker.go#L166 

Added line #L166 was not covered by tests
options, err := parseBackendOptions(step)
if err != nil {
log.Error().Err(err).Msg("could not parse backend options")
}

log.Trace().Str("taskUUID", taskUUID).Msgf("start step %s", step.Name)
log.Trace().Str("taskUUID", taskUUID).Msgf("start step %s-%s", workflowName, step.Name)

Check warning on line 172 in pipeline/backend/docker/docker.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/docker/docker.go#L172 

Added line #L172 was not covered by tests

config := e.toConfig(step, options)
hostConfig := toHostConfig(step, &e.config)
containerName := toContainerName(step)
containerName := toContainerName(step, workflowName)

Check warning on line 176 in pipeline/backend/docker/docker.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/docker/docker.go#L176 

Added line #L176 was not covered by tests

// create pull options with encoded authorization credentials.
pullOpts := image.PullOptions{}
Expand Down Expand Up @@ -246,10 +246,10 @@
return e.client.ContainerStart(ctx, containerName, container.StartOptions{})
}

func (e *docker) WaitStep(ctx context.Context, step *backend.Step, taskUUID string) (*backend.State, error) {
func (e *docker) WaitStep(ctx context.Context, step *backend.Step, taskUUID, workflowName string) (*backend.State, error) {

Check warning on line 249 in pipeline/backend/docker/docker.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/docker/docker.go#L249 

Added line #L249 was not covered by tests
log.Trace().Str("taskUUID", taskUUID).Msgf("wait for step %s", step.Name)

containerName := toContainerName(step)
containerName := toContainerName(step, workflowName)

Check warning on line 252 in pipeline/backend/docker/docker.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/docker/docker.go#L252 

Added line #L252 was not covered by tests

wait, errC := e.client.ContainerWait(ctx, containerName, "")
select {
Expand All @@ -269,10 +269,10 @@
}, nil
}

func (e *docker) TailStep(ctx context.Context, step *backend.Step, taskUUID string) (io.ReadCloser, error) {
func (e *docker) TailStep(ctx context.Context, step *backend.Step, taskUUID, workflowName string) (io.ReadCloser, error) {

Check warning on line 272 in pipeline/backend/docker/docker.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/docker/docker.go#L272 

Added line #L272 was not covered by tests
log.Trace().Str("taskUUID", taskUUID).Msgf("tail logs of step %s", step.Name)

logs, err := e.client.ContainerLogs(ctx, toContainerName(step), container.LogsOptions{
logs, err := e.client.ContainerLogs(ctx, toContainerName(step, workflowName), container.LogsOptions{

Check warning on line 275 in pipeline/backend/docker/docker.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/docker/docker.go#L275 

Added line #L275 was not covered by tests
Follow: true,
ShowStdout: true,
ShowStderr: true,
Expand All @@ -293,10 +293,10 @@
return rc, nil
}

func (e *docker) DestroyStep(ctx context.Context, step *backend.Step, taskUUID string) error {
func (e *docker) DestroyStep(ctx context.Context, step *backend.Step, taskUUID, workflowName string) error {

Check warning on line 296 in pipeline/backend/docker/docker.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/docker/docker.go#L296 

Added line #L296 was not covered by tests
log.Trace().Str("taskUUID", taskUUID).Msgf("stop step %s", step.Name)

containerName := toContainerName(step)
containerName := toContainerName(step, workflowName)

Check warning on line 299 in pipeline/backend/docker/docker.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/docker/docker.go#L299 

Added line #L299 was not covered by tests

if err := e.client.ContainerKill(ctx, containerName, "9"); err != nil && !isErrContainerNotFoundOrNotRunning(err) {
return err
Expand All @@ -309,12 +309,12 @@
return nil
}

func (e *docker) DestroyWorkflow(ctx context.Context, conf *backend.Config, taskUUID string) error {
func (e *docker) DestroyWorkflow(ctx context.Context, conf *backend.Config, taskUUID, workflowName string) error {

Check warning on line 312 in pipeline/backend/docker/docker.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/docker/docker.go#L312 

Added line #L312 was not covered by tests
log.Trace().Str("taskUUID", taskUUID).Msgf("delete workflow environment")

for _, stage := range conf.Stages {
for _, step := range stage.Steps {
containerName := toContainerName(step)
containerName := toContainerName(step, workflowName)

Check warning on line 317 in pipeline/backend/docker/docker.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/docker/docker.go#L317 

Added line #L317 was not covered by tests
if err := e.client.ContainerKill(ctx, containerName, "9"); err != nil && !isErrContainerNotFoundOrNotRunning(err) {
log.Error().Err(err).Msgf("could not kill container '%s'", step.Name)
}
Expand Down
25 changes: 14 additions & 11 deletions pipeline/backend/dummy/dummy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,20 @@
}, nil
}

func (e *dummy) SetupWorkflow(_ context.Context, _ *backend.Config, taskUUID string) error {
func (e *dummy) SetupWorkflow(_ context.Context, _ *backend.Config, taskUUID, workflowName string) error {
if taskUUID == WorkflowSetupFailUUID {
return fmt.Errorf("expected fail to setup workflow")
}
if workflowName == "" {
return fmt.Errorf("expected fail to setup workflow")
}

Check warning on line 87 in pipeline/backend/dummy/dummy.go

View check run for this annotation

Codecov / codecov/patch

pipeline/backend/dummy/dummy.go#L86-L87 

Added lines #L86 - L87 were not covered by tests
log.Trace().Str("taskUUID", taskUUID).Msg("create workflow environment")
e.kv.Store("task_"+taskUUID, "setup")
return nil
}

func (e *dummy) StartStep(_ context.Context, step *backend.Step, taskUUID string) error {
log.Trace().Str("taskUUID", taskUUID).Msgf("start step %s", step.Name)
func (e *dummy) StartStep(_ context.Context, step *backend.Step, taskUUID, workflowName string) error {
log.Trace().Str("taskUUID", taskUUID).Msgf("start step %s-%s", workflowName, step.Name)

// internal state checks
_, exist := e.kv.Load("task_" + taskUUID)
Expand All @@ -114,8 +117,8 @@
return nil
}

func (e *dummy) WaitStep(ctx context.Context, step *backend.Step, taskUUID string) (*backend.State, error) {
log.Trace().Str("taskUUID", taskUUID).Msgf("wait for step %s", step.Name)
func (e *dummy) WaitStep(ctx context.Context, step *backend.Step, taskUUID, workflowName string) (*backend.State, error) {
log.Trace().Str("taskUUID", taskUUID).Msgf("start step %s-%s", workflowName, step.Name)

_, exist := e.kv.Load("task_" + taskUUID)
if !exist {
Expand Down Expand Up @@ -172,8 +175,8 @@
}, nil
}

func (e *dummy) TailStep(_ context.Context, step *backend.Step, taskUUID string) (io.ReadCloser, error) {
log.Trace().Str("taskUUID", taskUUID).Msgf("tail logs of step %s", step.Name)
func (e *dummy) TailStep(_ context.Context, step *backend.Step, taskUUID, workflowName string) (io.ReadCloser, error) {
log.Trace().Str("taskUUID", taskUUID).Msgf("start step %s-%s", workflowName, step.Name)

_, exist := e.kv.Load("task_" + taskUUID)
if !exist {
Expand All @@ -196,8 +199,8 @@
return io.NopCloser(strings.NewReader(dummyExecStepOutput(step))), nil
}

func (e *dummy) DestroyStep(_ context.Context, step *backend.Step, taskUUID string) error {
log.Trace().Str("taskUUID", taskUUID).Msgf("stop step %s", step.Name)
func (e *dummy) DestroyStep(_ context.Context, step *backend.Step, taskUUID, workflowName string) error {
log.Trace().Str("taskUUID", taskUUID).Str("workflowName", workflowName).Msgf("stop step %s", step.Name)

_, exist := e.kv.Load("task_" + taskUUID)
if !exist {
Expand All @@ -217,8 +220,8 @@
return nil
}

func (e *dummy) DestroyWorkflow(_ context.Context, _ *backend.Config, taskUUID string) error {
log.Trace().Str("taskUUID", taskUUID).Msgf("delete workflow environment")
func (e *dummy) DestroyWorkflow(_ context.Context, _ *backend.Config, taskUUID, workflowName string) error {
log.Trace().Str("taskUUID", taskUUID).Str("workflowName", workflowName).Msgf("delete workflow environment")

_, exist := e.kv.Load("task_" + taskUUID)
if !exist {
Expand Down
58 changes: 29 additions & 29 deletions pipeline/backend/dummy/dummy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,22 @@ func TestSmalPipelineDummyRun(t *testing.T) {
_, err := dummyEngine.Load(ctx)
assert.NoError(t, err)

assert.Error(t, dummyEngine.SetupWorkflow(ctx, nil, dummy.WorkflowSetupFailUUID))
assert.Error(t, dummyEngine.SetupWorkflow(ctx, nil, dummy.WorkflowSetupFailUUID, "workflowTestName"))

t.Run("expect fail of step func with non setup workflow", func(t *testing.T) {
step := &types.Step{Name: "step1", UUID: "SID_1"}
nonExistWorkflowID := "WID_NONE"

err := dummyEngine.StartStep(ctx, step, nonExistWorkflowID)
err := dummyEngine.StartStep(ctx, step, nonExistWorkflowID, "workflowTestName")
assert.Error(t, err)

_, err = dummyEngine.TailStep(ctx, step, nonExistWorkflowID)
_, err = dummyEngine.TailStep(ctx, step, nonExistWorkflowID, "workflowTestName")
assert.Error(t, err)

_, err = dummyEngine.WaitStep(ctx, step, nonExistWorkflowID)
_, err = dummyEngine.WaitStep(ctx, step, nonExistWorkflowID, "workflowTestName")
assert.Error(t, err)

err = dummyEngine.DestroyStep(ctx, step, nonExistWorkflowID)
err = dummyEngine.DestroyStep(ctx, step, nonExistWorkflowID, "workflowTestName")
assert.Error(t, err)
})

Expand All @@ -63,11 +63,11 @@ func TestSmalPipelineDummyRun(t *testing.T) {
}
workflowUUID := "WID_1"

assert.NoError(t, dummyEngine.SetupWorkflow(ctx, nil, workflowUUID))
assert.NoError(t, dummyEngine.SetupWorkflow(ctx, nil, workflowUUID, "workflowTestName"))

assert.NoError(t, dummyEngine.StartStep(ctx, step, workflowUUID))
assert.NoError(t, dummyEngine.StartStep(ctx, step, workflowUUID, "workflowTestName"))

reader, err := dummyEngine.TailStep(ctx, step, workflowUUID)
reader, err := dummyEngine.TailStep(ctx, step, workflowUUID, "workflowTestName")
assert.NoError(t, err)
log, err := io.ReadAll(reader)
assert.NoError(t, err)
Expand All @@ -81,14 +81,14 @@ echo nein
------------------
`, string(log))

state, err := dummyEngine.WaitStep(ctx, step, workflowUUID)
state, err := dummyEngine.WaitStep(ctx, step, workflowUUID, "workflowTestName")
assert.NoError(t, err)
assert.NoError(t, state.Error)
assert.EqualValues(t, 0, state.ExitCode)

assert.NoError(t, dummyEngine.DestroyStep(ctx, step, workflowUUID))
assert.NoError(t, dummyEngine.DestroyStep(ctx, step, workflowUUID, "workflowTestName"))

assert.NoError(t, dummyEngine.DestroyWorkflow(ctx, nil, workflowUUID))
assert.NoError(t, dummyEngine.DestroyWorkflow(ctx, nil, workflowUUID, "workflowTestName"))
})

t.Run("step exec error", func(t *testing.T) {
Expand All @@ -100,21 +100,21 @@ echo nein
}
workflowUUID := "WID_1"

assert.NoError(t, dummyEngine.SetupWorkflow(ctx, nil, workflowUUID))
assert.NoError(t, dummyEngine.SetupWorkflow(ctx, nil, workflowUUID, "workflowTestName"))

assert.NoError(t, dummyEngine.StartStep(ctx, step, workflowUUID))
assert.NoError(t, dummyEngine.StartStep(ctx, step, workflowUUID, "workflowTestName"))

_, err := dummyEngine.TailStep(ctx, step, workflowUUID)
_, err := dummyEngine.TailStep(ctx, step, workflowUUID, "workflowTestName")
assert.NoError(t, err)

state, err := dummyEngine.WaitStep(ctx, step, workflowUUID)
state, err := dummyEngine.WaitStep(ctx, step, workflowUUID, "workflowTestName")
assert.NoError(t, err)
assert.NoError(t, state.Error)
assert.EqualValues(t, 1, state.ExitCode)

assert.NoError(t, dummyEngine.DestroyStep(ctx, step, workflowUUID))
assert.NoError(t, dummyEngine.DestroyStep(ctx, step, workflowUUID, "workflowTestName"))

assert.NoError(t, dummyEngine.DestroyWorkflow(ctx, nil, workflowUUID))
assert.NoError(t, dummyEngine.DestroyWorkflow(ctx, nil, workflowUUID, "workflowTestName"))
})

t.Run("step tail error", func(t *testing.T) {
Expand All @@ -125,19 +125,19 @@ echo nein
}
workflowUUID := "WID_1"

assert.NoError(t, dummyEngine.SetupWorkflow(ctx, nil, workflowUUID))
assert.NoError(t, dummyEngine.SetupWorkflow(ctx, nil, workflowUUID, "workflowTestName"))

assert.NoError(t, dummyEngine.StartStep(ctx, step, workflowUUID))
assert.NoError(t, dummyEngine.StartStep(ctx, step, workflowUUID, "workflowTestName"))

_, err := dummyEngine.TailStep(ctx, step, workflowUUID)
_, err := dummyEngine.TailStep(ctx, step, workflowUUID, "workflowTestName")
assert.Error(t, err)

_, err = dummyEngine.WaitStep(ctx, step, workflowUUID)
_, err = dummyEngine.WaitStep(ctx, step, workflowUUID, "workflowTestName")
assert.NoError(t, err)

assert.NoError(t, dummyEngine.DestroyStep(ctx, step, workflowUUID))
assert.NoError(t, dummyEngine.DestroyStep(ctx, step, workflowUUID, "workflowTestName"))

assert.NoError(t, dummyEngine.DestroyWorkflow(ctx, nil, workflowUUID))
assert.NoError(t, dummyEngine.DestroyWorkflow(ctx, nil, workflowUUID, "workflowTestName"))
})

t.Run("step start fail", func(t *testing.T) {
Expand All @@ -149,20 +149,20 @@ echo nein
}
workflowUUID := "WID_1"

assert.NoError(t, dummyEngine.SetupWorkflow(ctx, nil, workflowUUID))
assert.NoError(t, dummyEngine.SetupWorkflow(ctx, nil, workflowUUID, "workflowTestName"))

assert.Error(t, dummyEngine.StartStep(ctx, step, workflowUUID))
assert.Error(t, dummyEngine.StartStep(ctx, step, workflowUUID, "workflowTestName"))

_, err := dummyEngine.TailStep(ctx, step, workflowUUID)
_, err := dummyEngine.TailStep(ctx, step, workflowUUID, "workflowTestName")
assert.Error(t, err)

state, err := dummyEngine.WaitStep(ctx, step, workflowUUID)
state, err := dummyEngine.WaitStep(ctx, step, workflowUUID, "workflowTestName")
assert.Error(t, err)
assert.Error(t, state.Error)
assert.EqualValues(t, 0, state.ExitCode)

assert.Error(t, dummyEngine.DestroyStep(ctx, step, workflowUUID))
assert.Error(t, dummyEngine.DestroyStep(ctx, step, workflowUUID, "workflowTestName"))

assert.NoError(t, dummyEngine.DestroyWorkflow(ctx, nil, workflowUUID))
assert.NoError(t, dummyEngine.DestroyWorkflow(ctx, nil, workflowUUID, "workflowTestName"))
})
}
Loading