From 7700e0e6c80252eb2e0681a6c835730811947e31 Mon Sep 17 00:00:00 2001 From: pat-s Date: Sun, 5 Jan 2025 18:36:10 +0100 Subject: [PATCH 1/8] use step name in container name --- pipeline/backend/docker/convert.go | 2 +- pipeline/backend/docker/convert_test.go | 4 +-- pipeline/backend/kubernetes/pod.go | 2 +- pipeline/backend/kubernetes/pod_test.go | 36 ++++++++++----------- pipeline/backend/kubernetes/secrets_test.go | 2 +- pipeline/backend/kubernetes/service.go | 2 +- pipeline/backend/kubernetes/service_test.go | 12 +++---- pipeline/backend/kubernetes/utils.go | 7 ++-- 8 files changed, 34 insertions(+), 33 deletions(-) diff --git a/pipeline/backend/docker/convert.go b/pipeline/backend/docker/convert.go index 5e20c71496d..27bab2360fa 100644 --- a/pipeline/backend/docker/convert.go +++ b/pipeline/backend/docker/convert.go @@ -70,7 +70,7 @@ func (e *docker) toConfig(step *types.Step, options BackendOptions) *container.C } func toContainerName(step *types.Step) string { - return "wp_" + step.UUID + return "wp_" + step.UUID[:5] + "-" + step.Name } // returns a container host configuration. diff --git a/pipeline/backend/docker/convert_test.go b/pipeline/backend/docker/convert_test.go index f2948a4a742..f344aaffaa4 100644 --- a/pipeline/backend/docker/convert_test.go +++ b/pipeline/backend/docker/convert_test.go @@ -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_f5182-hello", toContainerName(testCmdStep)) + assert.EqualValues(t, "wp_d841e-lint", toContainerName(testPluginStep)) } func TestStepToConfig(t *testing.T) { diff --git a/pipeline/backend/kubernetes/pod.go b/pipeline/backend/kubernetes/pod.go index 42a04356c2c..2a8a9b5d5f4 100644 --- a/pipeline/backend/kubernetes/pod.go +++ b/pipeline/backend/kubernetes/pod.go @@ -77,7 +77,7 @@ func stepToPodName(step *types.Step) (name string, err error) { } func podName(step *types.Step) (string, error) { - return dnsName(podPrefix + step.UUID) + return dnsName(podPrefix + step.UUID[:5] + "-" + step.Name) } func podMeta(step *types.Step, config *config, options BackendOptions, podName string) (meta_v1.ObjectMeta, error) { diff --git a/pipeline/backend/kubernetes/pod_test.go b/pipeline/backend/kubernetes/pod_test.go index 01aa95ecbd9..ddee1a31900 100644 --- a/pipeline/backend/kubernetes/pod_test.go +++ b/pipeline/backend/kubernetes/pod_test.go @@ -26,9 +26,9 @@ import ( ) func TestPodName(t *testing.T) { - name, err := podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me-0"}) + name, err := podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me-0", Name: "go-test"}) assert.NoError(t, err) - assert.Equal(t, "wp-01he8bebctabr3kgk0qj36d2me-0", name) + assert.Equal(t, "wp-01he8-go-test", name) _, err = podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me\\0a"}) assert.ErrorIs(t, err, ErrDNSPatternInvalid) @@ -40,19 +40,19 @@ func TestPodName(t *testing.T) { func TestStepToPodName(t *testing.T) { name, err := stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "clone", Type: types.StepTypeClone}) assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8bebctabr3kg", name) + assert.EqualValues(t, "wp-01he8-clone", name) name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "cache", Type: types.StepTypeCache}) assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8bebctabr3kg", name) + assert.EqualValues(t, "wp-01he8-cache", name) name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "release", Type: types.StepTypePlugin}) assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8bebctabr3kg", name) + assert.EqualValues(t, "wp-01he8-release", name) name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "prepare-env", Type: types.StepTypeCommands}) assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8bebctabr3kg", name) + assert.EqualValues(t, "wp-01he8-prepare-env", name) name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "postgres", Type: types.StepTypeService}) assert.NoError(t, err) - assert.EqualValues(t, "wp-svc-01he8bebctabr3kg-postgres", name) + assert.EqualValues(t, "wp-svc-01he8-postgres", name) } func TestStepLabel(t *testing.T) { @@ -68,7 +68,7 @@ func TestTinyPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-01he8bebctabr3kgk0qj36d2me-0", + "name": "wp-01he8-go-test", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -86,7 +86,7 @@ func TestTinyPod(t *testing.T) { ], "containers": [ { - "name": "wp-01he8bebctabr3kgk0qj36d2me-0", + "name": "wp-01he8-go-test", "image": "gradle:8.4.0-jdk21", "command": [ "/bin/sh", @@ -133,7 +133,7 @@ func TestTinyPod(t *testing.T) { Environment: map[string]string{"CI": "woodpecker"}, }, &config{ Namespace: "woodpecker", - }, "wp-01he8bebctabr3kgk0qj36d2me-0", "linux/amd64", BackendOptions{}) + }, "wp-01he8-go-test", "linux/amd64", BackendOptions{}) assert.NoError(t, err) podJSON, err := json.Marshal(pod) @@ -147,7 +147,7 @@ func TestFullPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-01he8bebctabr3kgk0qj36d2me-0", + "name": "wp-01he8-go-test", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -171,7 +171,7 @@ func TestFullPod(t *testing.T) { ], "containers": [ { - "name": "wp-01he8bebctabr3kgk0qj36d2me-0", + "name": "wp-01he8-go-test", "image": "meltwater/drone-cache", "command": [ "/bin/sh", @@ -256,7 +256,7 @@ func TestFullPod(t *testing.T) { "name": "another-pull-secret" }, { - "name": "wp-01he8bebctabr3kgk0qj36d2me-0" + "name": "wp-01he8-go-test" } ], "tolerations": [ @@ -335,7 +335,7 @@ func TestFullPod(t *testing.T) { PodAnnotationsAllowFromStep: true, PodNodeSelector: map[string]string{"topology.kubernetes.io/region": "eu-central-1"}, SecurityContext: SecurityContextConfig{RunAsNonRoot: false}, - }, "wp-01he8bebctabr3kgk0qj36d2me-0", "linux/amd64", BackendOptions{ + }, "wp-01he8-go-test", "linux/amd64", BackendOptions{ Labels: map[string]string{"part-of": "woodpecker-ci"}, Annotations: map[string]string{"kubernetes.io/limit-ranger": "LimitRanger plugin set: cpu, memory request and limit for container"}, NodeSelector: map[string]string{"storage": "ssd"}, @@ -366,7 +366,7 @@ func TestPodPrivilege(t *testing.T) { }, &config{ Namespace: "woodpecker", SecurityContext: SecurityContextConfig{RunAsNonRoot: globalRunAsRoot}, - }, "wp-01he8bebctabr3kgk0qj36d2me-0", "linux/amd64", BackendOptions{ + }, "wp-01he8-go-test", "linux/amd64", BackendOptions{ SecurityContext: &secCtx, }) } @@ -443,7 +443,7 @@ func TestScratchPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-01he8bebctabr3kgk0qj36d2me-0", + "name": "wp-01he8-go-test", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -453,7 +453,7 @@ func TestScratchPod(t *testing.T) { "spec": { "containers": [ { - "name": "wp-01he8bebctabr3kgk0qj36d2me-0", + "name": "wp-01he8-go-test", "image": "quay.io/curl/curl", "command": [ "/usr/bin/curl", @@ -474,7 +474,7 @@ func TestScratchPod(t *testing.T) { Entrypoint: []string{"/usr/bin/curl", "-v", "google.com"}, }, &config{ Namespace: "woodpecker", - }, "wp-01he8bebctabr3kgk0qj36d2me-0", "linux/amd64", BackendOptions{}) + }, "wp-01he8-go-test", "linux/amd64", BackendOptions{}) assert.NoError(t, err) podJSON, err := json.Marshal(pod) diff --git a/pipeline/backend/kubernetes/secrets_test.go b/pipeline/backend/kubernetes/secrets_test.go index c918fc74174..9151daf9190 100644 --- a/pipeline/backend/kubernetes/secrets_test.go +++ b/pipeline/backend/kubernetes/secrets_test.go @@ -208,7 +208,7 @@ func TestUsernameAndPasswordNeedsSecret(t *testing.T) { func TestRegistrySecret(t *testing.T) { const expected = `{ "metadata": { - "name": "wp-01he8bebctabr3kgk0qj36d2me-0", + "name": "wp-01he8-go-test", "namespace": "woodpecker", "creationTimestamp": null, "labels": { diff --git a/pipeline/backend/kubernetes/service.go b/pipeline/backend/kubernetes/service.go index 428684648bb..de063363e3e 100644 --- a/pipeline/backend/kubernetes/service.go +++ b/pipeline/backend/kubernetes/service.go @@ -63,7 +63,7 @@ func mkService(step *types.Step, config *config) (*v1.Service, error) { } func serviceName(step *types.Step) (string, error) { - return dnsName(servicePrefix + step.UUID + "-" + step.Name) + return dnsName(servicePrefix + step.UUID[:5] + "-" + step.Name) } func servicePort(port types.Port) v1.ServicePort { diff --git a/pipeline/backend/kubernetes/service_test.go b/pipeline/backend/kubernetes/service_test.go index ab02175342d..24e4e44fefe 100644 --- a/pipeline/backend/kubernetes/service_test.go +++ b/pipeline/backend/kubernetes/service_test.go @@ -26,22 +26,22 @@ import ( func TestServiceName(t *testing.T) { name, err := serviceName(&types.Step{Name: "database", UUID: "01he8bebctabr3kgk0qj36d2me"}) assert.NoError(t, err) - assert.Equal(t, "wp-svc-01he8bebctabr3kgk0qj36d2me-database", name) + assert.Equal(t, "wp-svc-01he8-database", name) - name, err = serviceName(&types.Step{Name: "wp-01he8bebctabr3kgk0qj36d2me-0-services-0.woodpecker-runtime.svc.cluster.local", UUID: "01he8bebctabr3kgk0qj36d2me"}) + name, err = serviceName(&types.Step{Name: "wp-01he8-clone-0-services-0.woodpecker-runtime.svc.cluster.local", UUID: "01he8bebctabr3kgk0qj36d2me"}) assert.NoError(t, err) - assert.Equal(t, "wp-svc-01he8bebctabr3kgk0qj36d2me-wp-01he8bebctabr3kgk0qj36d2me-0-services-0.woodpecker-runtime.svc.cluster.local", name) + assert.Equal(t, "wp-svc-01he8-wp-01he8-clone-0-services-0.woodpecker-runtime.svc.cluster.local", name) name, err = serviceName(&types.Step{Name: "awesome_service", UUID: "01he8bebctabr3kgk0qj36d2me"}) assert.NoError(t, err) - assert.Equal(t, "wp-svc-01he8bebctabr3kgk0qj36d2me-awesome-service", name) + assert.Equal(t, "wp-svc-01he8-awesome-service", name) } func TestService(t *testing.T) { expected := ` { "metadata": { - "name": "wp-svc-01he8bebctabr3kgk0qj36d2me-0-bar", + "name": "wp-svc-01he8-bar", "namespace": "foo", "creationTimestamp": null }, @@ -66,7 +66,7 @@ func TestService(t *testing.T) { } ], "selector": { - "service": "wp-svc-01he8bebctabr3kgk0qj36d2me-0-bar" + "service": "wp-svc-01he8-bar" }, "type": "ClusterIP" }, diff --git a/pipeline/backend/kubernetes/utils.go b/pipeline/backend/kubernetes/utils.go index ec601bcccef..46e57970bb7 100644 --- a/pipeline/backend/kubernetes/utils.go +++ b/pipeline/backend/kubernetes/utils.go @@ -16,6 +16,7 @@ package kubernetes import ( "errors" + "fmt" "os" "regexp" "strings" @@ -38,10 +39,10 @@ var ( func dnsName(i string) (string, error) { res := strings.ToLower(strings.ReplaceAll(i, "_", "-")) - if found := dnsPattern.FindStringIndex(res); found == nil { - return "", ErrDNSPatternInvalid + found := dnsPattern.FindStringIndex(res) + if found == nil { + return "", fmt.Errorf("%w: found invalid characters '%v'", ErrDNSPatternInvalid, found) } - return res, nil } From a0d8c70a2c467e8adcd0828e3012432f8e12db2a Mon Sep 17 00:00:00 2001 From: pat-s Date: Sun, 5 Jan 2025 20:57:57 +0100 Subject: [PATCH 2/8] extract pipeline_name from metadata to agent and add to container names --- agent/runner.go | 6 +++ pipeline/backend/docker/convert.go | 4 +- pipeline/backend/docker/convert_test.go | 4 +- pipeline/backend/docker/docker.go | 26 ++++----- pipeline/backend/dummy/dummy.go | 25 +++++---- pipeline/backend/dummy/dummy_test.go | 58 ++++++++++----------- pipeline/backend/kubernetes/kubernetes.go | 30 +++++------ pipeline/backend/kubernetes/pod.go | 38 +++++++------- pipeline/backend/kubernetes/pod_test.go | 56 ++++++++++---------- pipeline/backend/kubernetes/secrets.go | 22 ++++---- pipeline/backend/kubernetes/secrets_test.go | 4 +- pipeline/backend/kubernetes/service.go | 16 +++--- pipeline/backend/kubernetes/service_test.go | 18 +++---- pipeline/backend/local/local.go | 22 ++++---- pipeline/backend/types/backend.go | 12 ++--- pipeline/frontend/yaml/parse_test.go | 8 +-- pipeline/pipeline.go | 20 +++---- 17 files changed, 189 insertions(+), 180 deletions(-) diff --git a/agent/runner.go b/agent/runner.go index f5640982e79..a0bb4c095a3 100644 --- a/agent/runner.go +++ b/agent/runner.go @@ -72,6 +72,7 @@ func (r *Runner) Run(runnerCtx, shutdownCtx context.Context) error { //nolint:co repoName := extractRepositoryName(workflow.Config) // hack pipelineNumber := extractPipelineNumber(workflow.Config) // hack + pipelineName := extractPipelineName(workflow.Config) // hack r.counter.Add( workflow.ID, @@ -149,6 +150,7 @@ func (r *Runner) Run(runnerCtx, shutdownCtx context.Context) error { //nolint:co "workflow_id": workflow.ID, "repo": repoName, "pipeline_number": pipelineNumber, + "pipeline_name": pipelineName, }), ).Run(runnerCtx) @@ -197,3 +199,7 @@ func extractRepositoryName(config *backend.Config) string { func extractPipelineNumber(config *backend.Config) string { return config.Stages[0].Steps[0].Environment["CI_PIPELINE_NUMBER"] } + +func extractPipelineName(config *backend.Config) string { + return config.Stages[0].Steps[0].Environment["CI_WORKFLOW_NAME"] +} diff --git a/pipeline/backend/docker/convert.go b/pipeline/backend/docker/convert.go index 27bab2360fa..a33f40ebd1a 100644 --- a/pipeline/backend/docker/convert.go +++ b/pipeline/backend/docker/convert.go @@ -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[:5] + "-" + step.Name +func toContainerName(step *types.Step, workflowName string) string { + return "wp_" + step.UUID[:5] + "-" + workflowName + "-" + step.Name } // returns a container host configuration. diff --git a/pipeline/backend/docker/convert_test.go b/pipeline/backend/docker/convert_test.go index f344aaffaa4..7a549e50699 100644 --- a/pipeline/backend/docker/convert_test.go +++ b/pipeline/backend/docker/convert_test.go @@ -125,8 +125,8 @@ var ( ) func TestToContainerName(t *testing.T) { - assert.EqualValues(t, "wp_f5182-hello", toContainerName(testCmdStep)) - assert.EqualValues(t, "wp_d841e-lint", toContainerName(testPluginStep)) + assert.EqualValues(t, "wp_f5182-workflowNameTest-hello", toContainerName(testCmdStep, "workflowNameTest")) + assert.EqualValues(t, "wp_d841e-workflowNameTest-lint", toContainerName(testPluginStep, "workflowNameTest")) } func TestStepToConfig(t *testing.T) { diff --git a/pipeline/backend/docker/docker.go b/pipeline/backend/docker/docker.go index eacc59d5a68..6edad15563a 100644 --- a/pipeline/backend/docker/docker.go +++ b/pipeline/backend/docker/docker.go @@ -141,8 +141,8 @@ func (e *docker) Load(ctx context.Context) (*backend.BackendInfo, error) { }, 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") _, err := e.client.VolumeCreate(ctx, volume.CreateOptions{ Name: conf.Volume.Name, @@ -163,17 +163,17 @@ func (e *docker) SetupWorkflow(ctx context.Context, conf *backend.Config, taskUU 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 { 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) config := e.toConfig(step, options) hostConfig := toHostConfig(step, &e.config) - containerName := toContainerName(step) + containerName := toContainerName(step, workflowName) // create pull options with encoded authorization credentials. pullOpts := image.PullOptions{} @@ -246,10 +246,10 @@ func (e *docker) StartStep(ctx context.Context, step *backend.Step, taskUUID str 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) { log.Trace().Str("taskUUID", taskUUID).Msgf("wait for step %s", step.Name) - containerName := toContainerName(step) + containerName := toContainerName(step, workflowName) wait, errC := e.client.ContainerWait(ctx, containerName, "") select { @@ -269,10 +269,10 @@ func (e *docker) WaitStep(ctx context.Context, step *backend.Step, taskUUID stri }, 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) { 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{ Follow: true, ShowStdout: true, ShowStderr: true, @@ -293,10 +293,10 @@ func (e *docker) TailStep(ctx context.Context, step *backend.Step, taskUUID stri 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 { log.Trace().Str("taskUUID", taskUUID).Msgf("stop step %s", step.Name) - containerName := toContainerName(step) + containerName := toContainerName(step, workflowName) if err := e.client.ContainerKill(ctx, containerName, "9"); err != nil && !isErrContainerNotFoundOrNotRunning(err) { return err @@ -309,12 +309,12 @@ func (e *docker) DestroyStep(ctx context.Context, step *backend.Step, taskUUID s 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 { 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) if err := e.client.ContainerKill(ctx, containerName, "9"); err != nil && !isErrContainerNotFoundOrNotRunning(err) { log.Error().Err(err).Msgf("could not kill container '%s'", step.Name) } diff --git a/pipeline/backend/dummy/dummy.go b/pipeline/backend/dummy/dummy.go index 5e56f7fbb64..8871655038a 100644 --- a/pipeline/backend/dummy/dummy.go +++ b/pipeline/backend/dummy/dummy.go @@ -78,17 +78,20 @@ func (e *dummy) Load(_ context.Context) (*backend.BackendInfo, error) { }, 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") + } 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) @@ -114,8 +117,8 @@ func (e *dummy) StartStep(_ context.Context, step *backend.Step, taskUUID string 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 { @@ -172,8 +175,8 @@ func (e *dummy) WaitStep(ctx context.Context, step *backend.Step, taskUUID strin }, 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 { @@ -196,8 +199,8 @@ func (e *dummy) TailStep(_ context.Context, step *backend.Step, taskUUID string) 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 { @@ -217,8 +220,8 @@ func (e *dummy) DestroyStep(_ context.Context, step *backend.Step, taskUUID stri 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 { diff --git a/pipeline/backend/dummy/dummy_test.go b/pipeline/backend/dummy/dummy_test.go index 44e4034aee3..9b934240e81 100644 --- a/pipeline/backend/dummy/dummy_test.go +++ b/pipeline/backend/dummy/dummy_test.go @@ -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) }) @@ -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) @@ -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) { @@ -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) { @@ -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) { @@ -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")) }) } diff --git a/pipeline/backend/kubernetes/kubernetes.go b/pipeline/backend/kubernetes/kubernetes.go index 2ddf17852d8..c0728441545 100644 --- a/pipeline/backend/kubernetes/kubernetes.go +++ b/pipeline/backend/kubernetes/kubernetes.go @@ -188,7 +188,7 @@ func (e *kube) getConfig() *config { } // SetupWorkflow sets up the pipeline environment. -func (e *kube) SetupWorkflow(ctx context.Context, conf *types.Config, taskUUID string) error { +func (e *kube) SetupWorkflow(ctx context.Context, conf *types.Config, taskUUID, workflowName string) error { log.Trace().Str("taskUUID", taskUUID).Msgf("Setting up Kubernetes primitives") _, err := startVolume(ctx, e, conf.Volume.Name) @@ -200,7 +200,7 @@ func (e *kube) SetupWorkflow(ctx context.Context, conf *types.Config, taskUUID s for _, stage := range conf.Stages { for _, step := range stage.Steps { if step.Type == types.StepTypeService { - svc, err := startService(ctx, e, step) + svc, err := startService(ctx, e, step, workflowName) if err != nil { return err } @@ -220,28 +220,28 @@ func (e *kube) SetupWorkflow(ctx context.Context, conf *types.Config, taskUUID s } // StartStep starts the pipeline step. -func (e *kube) StartStep(ctx context.Context, step *types.Step, taskUUID string) error { +func (e *kube) StartStep(ctx context.Context, step *types.Step, taskUUID, workflowName string) error { options, err := parseBackendOptions(step) if err != nil { log.Error().Err(err).Msg("could not parse backend options") } if needsRegistrySecret(step) { - err = startRegistrySecret(ctx, e, step) + err = startRegistrySecret(ctx, e, step, workflowName) if err != nil { return err } } log.Trace().Str("taskUUID", taskUUID).Msgf("starting step: %s", step.Name) - _, err = startPod(ctx, e, step, options) + _, err = startPod(ctx, e, step, options, workflowName) return err } // WaitStep waits for the pipeline step to complete and returns // the completion results. -func (e *kube) WaitStep(ctx context.Context, step *types.Step, taskUUID string) (*types.State, error) { - podName, err := stepToPodName(step) +func (e *kube) WaitStep(ctx context.Context, step *types.Step, taskUUID, workflowName string) (*types.State, error) { + podName, err := stepToPodName(step, workflowName) if err != nil { return nil, err } @@ -316,8 +316,8 @@ func (e *kube) WaitStep(ctx context.Context, step *types.Step, taskUUID string) } // TailStep tails the pipeline step logs. -func (e *kube) TailStep(ctx context.Context, step *types.Step, taskUUID string) (io.ReadCloser, error) { - podName, err := stepToPodName(step) +func (e *kube) TailStep(ctx context.Context, step *types.Step, taskUUID, workflowName string) (io.ReadCloser, error) { + podName, err := stepToPodName(step, workflowName) if err != nil { return nil, err } @@ -388,17 +388,17 @@ func (e *kube) TailStep(ctx context.Context, step *types.Step, taskUUID string) return rc, nil } -func (e *kube) DestroyStep(ctx context.Context, step *types.Step, taskUUID string) error { +func (e *kube) DestroyStep(ctx context.Context, step *types.Step, taskUUID, workflowName string) error { var errs []error log.Trace().Str("taskUUID", taskUUID).Msgf("Stopping step: %s", step.Name) if needsRegistrySecret(step) { - err := stopRegistrySecret(ctx, e, step, defaultDeleteOptions) + err := stopRegistrySecret(ctx, e, step, defaultDeleteOptions, workflowName) if err != nil { errs = append(errs, err) } } - err := stopPod(ctx, e, step, defaultDeleteOptions) + err := stopPod(ctx, e, step, defaultDeleteOptions, workflowName) if err != nil { errs = append(errs, err) } @@ -406,18 +406,18 @@ func (e *kube) DestroyStep(ctx context.Context, step *types.Step, taskUUID strin } // DestroyWorkflow destroys the pipeline environment. -func (e *kube) DestroyWorkflow(ctx context.Context, conf *types.Config, taskUUID string) error { +func (e *kube) DestroyWorkflow(ctx context.Context, conf *types.Config, taskUUID, workflowName string) error { log.Trace().Str("taskUUID", taskUUID).Msg("deleting Kubernetes primitives") for _, stage := range conf.Stages { for _, step := range stage.Steps { - err := stopPod(ctx, e, step, defaultDeleteOptions) + err := stopPod(ctx, e, step, defaultDeleteOptions, workflowName) if err != nil { return err } if step.Type == types.StepTypeService { - err := stopService(ctx, e, step, defaultDeleteOptions) + err := stopService(ctx, e, step, defaultDeleteOptions, workflowName) if err != nil { return err } diff --git a/pipeline/backend/kubernetes/pod.go b/pipeline/backend/kubernetes/pod.go index 2a8a9b5d5f4..11de5efadab 100644 --- a/pipeline/backend/kubernetes/pod.go +++ b/pipeline/backend/kubernetes/pod.go @@ -36,7 +36,7 @@ const ( defaultFSGroup int64 = 1000 ) -func mkPod(step *types.Step, config *config, podName, goos string, options BackendOptions) (*v1.Pod, error) { +func mkPod(step *types.Step, config *config, podName, goos string, options BackendOptions, workflowName string) (*v1.Pod, error) { var err error nsp := newNativeSecretsProcessor(config, options.Secrets) @@ -45,12 +45,12 @@ func mkPod(step *types.Step, config *config, podName, goos string, options Backe return nil, err } - meta, err := podMeta(step, config, options, podName) + meta, err := podMeta(step, config, options, podName, workflowName) if err != nil { return nil, err } - spec, err := podSpec(step, config, options, nsp) + spec, err := podSpec(step, config, options, nsp, workflowName) if err != nil { return nil, err } @@ -69,18 +69,18 @@ func mkPod(step *types.Step, config *config, podName, goos string, options Backe return pod, nil } -func stepToPodName(step *types.Step) (name string, err error) { +func stepToPodName(step *types.Step, workflowName string) (name string, err error) { if step.Type == types.StepTypeService { - return serviceName(step) + return serviceName(step, workflowName) } - return podName(step) + return podName(step, workflowName) } -func podName(step *types.Step) (string, error) { - return dnsName(podPrefix + step.UUID[:5] + "-" + step.Name) +func podName(step *types.Step, workflowName string) (string, error) { + return dnsName(podPrefix + step.UUID[:5] + "-" + workflowName + "-" + step.Name) } -func podMeta(step *types.Step, config *config, options BackendOptions, podName string) (meta_v1.ObjectMeta, error) { +func podMeta(step *types.Step, config *config, options BackendOptions, podName, workflowName string) (meta_v1.ObjectMeta, error) { var err error meta := meta_v1.ObjectMeta{ Name: podName, @@ -88,7 +88,7 @@ func podMeta(step *types.Step, config *config, options BackendOptions, podName s Annotations: podAnnotations(config, options), } - meta.Labels, err = podLabels(step, config, options) + meta.Labels, err = podLabels(step, config, options, workflowName) if err != nil { return meta, err } @@ -96,7 +96,7 @@ func podMeta(step *types.Step, config *config, options BackendOptions, podName s return meta, nil } -func podLabels(step *types.Step, config *config, options BackendOptions) (map[string]string, error) { +func podLabels(step *types.Step, config *config, options BackendOptions, workflowName string) (map[string]string, error) { var err error labels := make(map[string]string) @@ -113,7 +113,7 @@ func podLabels(step *types.Step, config *config, options BackendOptions) (map[st maps.Copy(labels, config.PodLabels) } if step.Type == types.StepTypeService { - labels[ServiceLabel], _ = serviceName(step) + labels[ServiceLabel], _ = serviceName(step, workflowName) } labels[StepLabel], err = stepLabel(step) if err != nil { @@ -146,7 +146,7 @@ func podAnnotations(config *config, options BackendOptions) map[string]string { return annotations } -func podSpec(step *types.Step, config *config, options BackendOptions, nsp nativeSecretsProcessor) (v1.PodSpec, error) { +func podSpec(step *types.Step, config *config, options BackendOptions, nsp nativeSecretsProcessor, workflowName string) (v1.PodSpec, error) { var err error spec := v1.PodSpec{ RestartPolicy: v1.RestartPolicyNever, @@ -176,7 +176,7 @@ func podSpec(step *types.Step, config *config, options BackendOptions, nsp nativ spec.ImagePullSecrets = secretsReferences(config.ImagePullSecretNames) if needsRegistrySecret(step) { log.Trace().Msgf("using an image pull secret from registries") - name, err := registrySecretName(step) + name, err := registrySecretName(step, workflowName) if err != nil { return spec, err } @@ -524,13 +524,13 @@ func mapToEnvVars(m map[string]string) []v1.EnvVar { return ev } -func startPod(ctx context.Context, engine *kube, step *types.Step, options BackendOptions) (*v1.Pod, error) { - podName, err := stepToPodName(step) +func startPod(ctx context.Context, engine *kube, step *types.Step, options BackendOptions, workflowName string) (*v1.Pod, error) { + podName, err := stepToPodName(step, workflowName) if err != nil { return nil, err } engineConfig := engine.getConfig() - pod, err := mkPod(step, engineConfig, podName, engine.goos, options) + pod, err := mkPod(step, engineConfig, podName, engine.goos, options, workflowName) if err != nil { return nil, err } @@ -539,8 +539,8 @@ func startPod(ctx context.Context, engine *kube, step *types.Step, options Backe return engine.client.CoreV1().Pods(engineConfig.Namespace).Create(ctx, pod, meta_v1.CreateOptions{}) } -func stopPod(ctx context.Context, engine *kube, step *types.Step, deleteOpts meta_v1.DeleteOptions) error { - podName, err := stepToPodName(step) +func stopPod(ctx context.Context, engine *kube, step *types.Step, deleteOpts meta_v1.DeleteOptions, workflowName string) error { + podName, err := stepToPodName(step, workflowName) if err != nil { return err } diff --git a/pipeline/backend/kubernetes/pod_test.go b/pipeline/backend/kubernetes/pod_test.go index ddee1a31900..0f468b4d32e 100644 --- a/pipeline/backend/kubernetes/pod_test.go +++ b/pipeline/backend/kubernetes/pod_test.go @@ -26,33 +26,33 @@ import ( ) func TestPodName(t *testing.T) { - name, err := podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me-0", Name: "go-test"}) + name, err := podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me-0", Name: "go-test"}, "workflowNameTest") assert.NoError(t, err) - assert.Equal(t, "wp-01he8-go-test", name) + assert.Equal(t, "wp-01he8-workflownametest-go-test", name) - _, err = podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me\\0a"}) + _, err = podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me\\0a"}, "workflowNameTest") assert.ErrorIs(t, err, ErrDNSPatternInvalid) - _, err = podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me-0-services-0..woodpecker-runtime.svc.cluster.local"}) + _, err = podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me-0-services-0..woodpecker-runtime.svc.cluster.local"}, "workflowNameTest") assert.ErrorIs(t, err, ErrDNSPatternInvalid) } func TestStepToPodName(t *testing.T) { - name, err := stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "clone", Type: types.StepTypeClone}) + name, err := stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "clone", Type: types.StepTypeClone}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8-clone", name) - name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "cache", Type: types.StepTypeCache}) + assert.EqualValues(t, "wp-01he8-workflownametest-clone", name) + name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "cache", Type: types.StepTypeCache}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8-cache", name) - name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "release", Type: types.StepTypePlugin}) + assert.EqualValues(t, "wp-01he8-workflownametest-cache", name) + name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "release", Type: types.StepTypePlugin}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8-release", name) - name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "prepare-env", Type: types.StepTypeCommands}) + assert.EqualValues(t, "wp-01he8-workflownametest-release", name) + name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "prepare-env", Type: types.StepTypeCommands}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8-prepare-env", name) - name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "postgres", Type: types.StepTypeService}) + assert.EqualValues(t, "wp-01he8-workflownametest-prepare-env", name) + name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "postgres", Type: types.StepTypeService}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-svc-01he8-postgres", name) + assert.EqualValues(t, "wp-svc-01he8-workflownametest-postgres", name) } func TestStepLabel(t *testing.T) { @@ -68,7 +68,7 @@ func TestTinyPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-01he8-go-test", + "name": "wp-01he8-workflownametest-go-test", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -86,7 +86,7 @@ func TestTinyPod(t *testing.T) { ], "containers": [ { - "name": "wp-01he8-go-test", + "name": "wp-01he8-workflownametest-go-test", "image": "gradle:8.4.0-jdk21", "command": [ "/bin/sh", @@ -133,7 +133,7 @@ func TestTinyPod(t *testing.T) { Environment: map[string]string{"CI": "woodpecker"}, }, &config{ Namespace: "woodpecker", - }, "wp-01he8-go-test", "linux/amd64", BackendOptions{}) + }, "wp-01he8-workflownametest-go-test", "linux/amd64", BackendOptions{}, "workflowNameTest") assert.NoError(t, err) podJSON, err := json.Marshal(pod) @@ -147,7 +147,7 @@ func TestFullPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-01he8-go-test", + "name": "wp-01he8-workflownametest-go-test", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -171,7 +171,7 @@ func TestFullPod(t *testing.T) { ], "containers": [ { - "name": "wp-01he8-go-test", + "name": "wp-01he8-workflownametest-go-test", "image": "meltwater/drone-cache", "command": [ "/bin/sh", @@ -256,7 +256,7 @@ func TestFullPod(t *testing.T) { "name": "another-pull-secret" }, { - "name": "wp-01he8-go-test" + "name": "wp-01he8-workflownametest-go-test" } ], "tolerations": [ @@ -335,7 +335,7 @@ func TestFullPod(t *testing.T) { PodAnnotationsAllowFromStep: true, PodNodeSelector: map[string]string{"topology.kubernetes.io/region": "eu-central-1"}, SecurityContext: SecurityContextConfig{RunAsNonRoot: false}, - }, "wp-01he8-go-test", "linux/amd64", BackendOptions{ + }, "wp-01he8-workflownametest-go-test", "linux/amd64", BackendOptions{ Labels: map[string]string{"part-of": "woodpecker-ci"}, Annotations: map[string]string{"kubernetes.io/limit-ranger": "LimitRanger plugin set: cpu, memory request and limit for container"}, NodeSelector: map[string]string{"storage": "ssd"}, @@ -347,7 +347,7 @@ func TestFullPod(t *testing.T) { Limits: map[string]string{"memory": "256Mi", "cpu": "2"}, }, SecurityContext: &secCtx, - }) + }, "workflowNameTest") assert.NoError(t, err) podJSON, err := json.Marshal(pod) @@ -366,9 +366,9 @@ func TestPodPrivilege(t *testing.T) { }, &config{ Namespace: "woodpecker", SecurityContext: SecurityContextConfig{RunAsNonRoot: globalRunAsRoot}, - }, "wp-01he8-go-test", "linux/amd64", BackendOptions{ + }, "wp-01he8-workflownametest-go-test", "linux/amd64", BackendOptions{ SecurityContext: &secCtx, - }) + }, "workflowNameTest") } // securty context is requesting user and group 101 (non-root) @@ -443,7 +443,7 @@ func TestScratchPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-01he8-go-test", + "name": "wp-01he8-workflownametest-go-test", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -453,7 +453,7 @@ func TestScratchPod(t *testing.T) { "spec": { "containers": [ { - "name": "wp-01he8-go-test", + "name": "wp-01he8-workflownametest-go-test", "image": "quay.io/curl/curl", "command": [ "/usr/bin/curl", @@ -474,7 +474,7 @@ func TestScratchPod(t *testing.T) { Entrypoint: []string{"/usr/bin/curl", "-v", "google.com"}, }, &config{ Namespace: "woodpecker", - }, "wp-01he8-go-test", "linux/amd64", BackendOptions{}) + }, "wp-01he8-workflownametest-go-test", "linux/amd64", BackendOptions{}, "workflowNameTest") assert.NoError(t, err) podJSON, err := json.Marshal(pod) @@ -593,7 +593,7 @@ func TestSecrets(t *testing.T) { Target: SecretTarget{File: "~/.docker/config.json"}, }, }, - }) + }, "workflowNameTest") assert.NoError(t, err) podJSON, err := json.Marshal(pod) diff --git a/pipeline/backend/kubernetes/secrets.go b/pipeline/backend/kubernetes/secrets.go index 3acba1e078b..d84837f3e01 100644 --- a/pipeline/backend/kubernetes/secrets.go +++ b/pipeline/backend/kubernetes/secrets.go @@ -204,13 +204,13 @@ func needsRegistrySecret(step *types.Step) bool { return step.AuthConfig.Username != "" && step.AuthConfig.Password != "" } -func mkRegistrySecret(step *types.Step, config *config) (*v1.Secret, error) { - name, err := registrySecretName(step) +func mkRegistrySecret(step *types.Step, config *config, workflowName string) (*v1.Secret, error) { + name, err := registrySecretName(step, workflowName) if err != nil { return nil, err } - labels, err := registrySecretLabels(step) + labels, err := registrySecretLabels(step, workflowName) if err != nil { return nil, err } @@ -247,16 +247,16 @@ func mkRegistrySecret(step *types.Step, config *config) (*v1.Secret, error) { }, nil } -func registrySecretName(step *types.Step) (string, error) { - return podName(step) +func registrySecretName(step *types.Step, workflowName string) (string, error) { + return podName(step, workflowName) } -func registrySecretLabels(step *types.Step) (map[string]string, error) { +func registrySecretLabels(step *types.Step, workflowName string) (map[string]string, error) { var err error labels := make(map[string]string) if step.Type == types.StepTypeService { - labels[ServiceLabel], _ = serviceName(step) + labels[ServiceLabel], _ = serviceName(step, workflowName) } labels[StepLabel], err = stepLabel(step) if err != nil { @@ -266,8 +266,8 @@ func registrySecretLabels(step *types.Step) (map[string]string, error) { return labels, nil } -func startRegistrySecret(ctx context.Context, engine *kube, step *types.Step) error { - secret, err := mkRegistrySecret(step, engine.config) +func startRegistrySecret(ctx context.Context, engine *kube, step *types.Step, workflowName string) error { + secret, err := mkRegistrySecret(step, engine.config, workflowName) if err != nil { return err } @@ -279,8 +279,8 @@ func startRegistrySecret(ctx context.Context, engine *kube, step *types.Step) er return nil } -func stopRegistrySecret(ctx context.Context, engine *kube, step *types.Step, deleteOpts meta_v1.DeleteOptions) error { - name, err := registrySecretName(step) +func stopRegistrySecret(ctx context.Context, engine *kube, step *types.Step, deleteOpts meta_v1.DeleteOptions, workflowName string) error { + name, err := registrySecretName(step, workflowName) if err != nil { return err } diff --git a/pipeline/backend/kubernetes/secrets_test.go b/pipeline/backend/kubernetes/secrets_test.go index 9151daf9190..0bb8df75dfd 100644 --- a/pipeline/backend/kubernetes/secrets_test.go +++ b/pipeline/backend/kubernetes/secrets_test.go @@ -208,7 +208,7 @@ func TestUsernameAndPasswordNeedsSecret(t *testing.T) { func TestRegistrySecret(t *testing.T) { const expected = `{ "metadata": { - "name": "wp-01he8-go-test", + "name": "wp-01he8-workflownametest-go-test", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -231,7 +231,7 @@ func TestRegistrySecret(t *testing.T) { }, }, &config{ Namespace: "woodpecker", - }) + }, "workflowNameTest") assert.NoError(t, err) secretJSON, err := json.Marshal(secret) diff --git a/pipeline/backend/kubernetes/service.go b/pipeline/backend/kubernetes/service.go index de063363e3e..c0e907b4d8d 100644 --- a/pipeline/backend/kubernetes/service.go +++ b/pipeline/backend/kubernetes/service.go @@ -33,8 +33,8 @@ const ( servicePrefix = "wp-svc-" ) -func mkService(step *types.Step, config *config) (*v1.Service, error) { - name, err := serviceName(step) +func mkService(step *types.Step, config *config, workflowName string) (*v1.Service, error) { + name, err := serviceName(step, workflowName) if err != nil { return nil, err } @@ -62,8 +62,8 @@ func mkService(step *types.Step, config *config) (*v1.Service, error) { }, nil } -func serviceName(step *types.Step) (string, error) { - return dnsName(servicePrefix + step.UUID[:5] + "-" + step.Name) +func serviceName(step *types.Step, workflowName string) (string, error) { + return dnsName(servicePrefix + step.UUID[:5] + "-" + workflowName + "-" + step.Name) } func servicePort(port types.Port) v1.ServicePort { @@ -77,9 +77,9 @@ func servicePort(port types.Port) v1.ServicePort { } } -func startService(ctx context.Context, engine *kube, step *types.Step) (*v1.Service, error) { +func startService(ctx context.Context, engine *kube, step *types.Step, workflowName string) (*v1.Service, error) { engineConfig := engine.getConfig() - svc, err := mkService(step, engineConfig) + svc, err := mkService(step, engineConfig, workflowName) if err != nil { return nil, err } @@ -88,8 +88,8 @@ func startService(ctx context.Context, engine *kube, step *types.Step) (*v1.Serv return engine.client.CoreV1().Services(engineConfig.Namespace).Create(ctx, svc, meta_v1.CreateOptions{}) } -func stopService(ctx context.Context, engine *kube, step *types.Step, deleteOpts meta_v1.DeleteOptions) error { - svcName, err := serviceName(step) +func stopService(ctx context.Context, engine *kube, step *types.Step, deleteOpts meta_v1.DeleteOptions, workflowName string) error { + svcName, err := serviceName(step, workflowName) if err != nil { return err } diff --git a/pipeline/backend/kubernetes/service_test.go b/pipeline/backend/kubernetes/service_test.go index 24e4e44fefe..b8fee573cda 100644 --- a/pipeline/backend/kubernetes/service_test.go +++ b/pipeline/backend/kubernetes/service_test.go @@ -24,24 +24,24 @@ import ( ) func TestServiceName(t *testing.T) { - name, err := serviceName(&types.Step{Name: "database", UUID: "01he8bebctabr3kgk0qj36d2me"}) + name, err := serviceName(&types.Step{Name: "database", UUID: "01he8bebctabr3kgk0qj36d2me"}, "workflowNameTest") assert.NoError(t, err) - assert.Equal(t, "wp-svc-01he8-database", name) + assert.Equal(t, "wp-svc-01he8-workflownametest-database", name) - name, err = serviceName(&types.Step{Name: "wp-01he8-clone-0-services-0.woodpecker-runtime.svc.cluster.local", UUID: "01he8bebctabr3kgk0qj36d2me"}) + name, err = serviceName(&types.Step{Name: "wp-01he8-workflownametest-clone-0-services-0.woodpecker-runtime.svc.cluster.local", UUID: "01he8bebctabr3kgk0qj36d2me"}, "workflowNameTest") assert.NoError(t, err) - assert.Equal(t, "wp-svc-01he8-wp-01he8-clone-0-services-0.woodpecker-runtime.svc.cluster.local", name) + assert.Equal(t, "wp-svc-01he8-workflownametest-wp-01he8-workflownametest-clone-0-services-0.woodpecker-runtime.svc.cluster.local", name) - name, err = serviceName(&types.Step{Name: "awesome_service", UUID: "01he8bebctabr3kgk0qj36d2me"}) + name, err = serviceName(&types.Step{Name: "awesome_service", UUID: "01he8bebctabr3kgk0qj36d2me"}, "workflowNameTest") assert.NoError(t, err) - assert.Equal(t, "wp-svc-01he8-awesome-service", name) + assert.Equal(t, "wp-svc-01he8-workflownametest-awesome-service", name) } func TestService(t *testing.T) { expected := ` { "metadata": { - "name": "wp-svc-01he8-bar", + "name": "wp-svc-01he8-workflownametest-bar", "namespace": "foo", "creationTimestamp": null }, @@ -66,7 +66,7 @@ func TestService(t *testing.T) { } ], "selector": { - "service": "wp-svc-01he8-bar" + "service": "wp-svc-01he8-workflownametest-bar" }, "type": "ClusterIP" }, @@ -83,7 +83,7 @@ func TestService(t *testing.T) { Name: "bar", UUID: "01he8bebctabr3kgk0qj36d2me-0", Ports: ports, - }, &config{Namespace: "foo"}) + }, &config{Namespace: "foo"}, "workflowNameTest") assert.NoError(t, err) j, err := json.Marshal(s) assert.NoError(t, err) diff --git a/pipeline/backend/local/local.go b/pipeline/backend/local/local.go index ab5eff802d8..b7f85acd8be 100644 --- a/pipeline/backend/local/local.go +++ b/pipeline/backend/local/local.go @@ -90,8 +90,8 @@ func (e *local) Load(ctx context.Context) (*types.BackendInfo, error) { } // SetupWorkflow the pipeline environment. -func (e *local) SetupWorkflow(_ context.Context, _ *types.Config, taskUUID string) error { - log.Trace().Str("taskUUID", taskUUID).Msg("create workflow environment") +func (e *local) SetupWorkflow(_ context.Context, _ *types.Config, taskUUID, workflowName string) error { + log.Trace().Str("taskUUID", taskUUID).Str("workflowName", workflowName).Msg("create workflow environment") baseDir, err := os.MkdirTemp(e.tempDir, "woodpecker-local-*") if err != nil { @@ -119,8 +119,8 @@ func (e *local) SetupWorkflow(_ context.Context, _ *types.Config, taskUUID strin } // StartStep the pipeline step. -func (e *local) StartStep(ctx context.Context, step *types.Step, taskUUID string) error { - log.Trace().Str("taskUUID", taskUUID).Msgf("start step %s", step.Name) +func (e *local) StartStep(ctx context.Context, step *types.Step, taskUUID, workflowName string) error { + log.Trace().Str("taskUUID", taskUUID).Msgf("start step %s-%s", workflowName, step.Name) state, err := e.getState(taskUUID) if err != nil { @@ -204,8 +204,8 @@ func (e *local) execPlugin(ctx context.Context, step *types.Step, state *workflo // WaitStep for the pipeline step to complete and returns // the completion results. -func (e *local) WaitStep(_ context.Context, step *types.Step, taskUUID string) (*types.State, error) { - log.Trace().Str("taskUUID", taskUUID).Msgf("wait for step %s", step.Name) +func (e *local) WaitStep(_ context.Context, step *types.Step, taskUUID, workflowName string) (*types.State, error) { + log.Trace().Str("taskUUID", taskUUID).Msgf("start step %s-%s", workflowName, step.Name) state, err := e.getState(taskUUID) if err != nil { @@ -234,19 +234,19 @@ func (e *local) WaitStep(_ context.Context, step *types.Step, taskUUID string) ( } // TailStep the pipeline step logs. -func (e *local) TailStep(_ context.Context, step *types.Step, taskUUID string) (io.ReadCloser, error) { - log.Trace().Str("taskUUID", taskUUID).Msgf("tail logs of step %s", step.Name) +func (e *local) TailStep(_ context.Context, step *types.Step, taskUUID, workflowName string) (io.ReadCloser, error) { + log.Trace().Str("taskUUID", taskUUID).Str("workflowName", workflowName).Msgf("tail logs of step %s", step.Name) return e.output, nil } -func (e *local) DestroyStep(_ context.Context, _ *types.Step, _ string) error { +func (e *local) DestroyStep(_ context.Context, _ *types.Step, _, _ string) error { // WaitStep already waits for the command to finish, so there is nothing to do here. return nil } // DestroyWorkflow the pipeline environment. -func (e *local) DestroyWorkflow(_ context.Context, _ *types.Config, taskUUID string) error { - log.Trace().Str("taskUUID", taskUUID).Msg("delete workflow environment") +func (e *local) DestroyWorkflow(_ context.Context, _ *types.Config, taskUUID, workflowName string) error { + log.Trace().Str("taskUUID", taskUUID).Str("workflowName", workflowName).Msg("delete workflow environment") state, err := e.getState(taskUUID) if err != nil { diff --git a/pipeline/backend/types/backend.go b/pipeline/backend/types/backend.go index 4687018f86d..c2cbc1f4dc4 100644 --- a/pipeline/backend/types/backend.go +++ b/pipeline/backend/types/backend.go @@ -37,23 +37,23 @@ type Backend interface { Load(ctx context.Context) (*BackendInfo, error) // SetupWorkflow sets up the workflow environment. - SetupWorkflow(ctx context.Context, conf *Config, taskUUID string) error + SetupWorkflow(ctx context.Context, conf *Config, taskUUID, workflowName string) error // StartStep starts the workflow step. - StartStep(ctx context.Context, step *Step, taskUUID string) error + StartStep(ctx context.Context, step *Step, taskUUID, workflowName string) error // WaitStep waits for the workflow step to complete and returns // the completion results. - WaitStep(ctx context.Context, step *Step, taskUUID string) (*State, error) + WaitStep(ctx context.Context, step *Step, taskUUID, workflowName string) (*State, error) // TailStep tails the workflow step logs. - TailStep(ctx context.Context, step *Step, taskUUID string) (io.ReadCloser, error) + TailStep(ctx context.Context, step *Step, taskUUID, workflowName string) (io.ReadCloser, error) // DestroyStep destroys the workflow step. - DestroyStep(ctx context.Context, step *Step, taskUUID string) error + DestroyStep(ctx context.Context, step *Step, taskUUID, workflowName string) error // DestroyWorkflow destroys the workflow environment. - DestroyWorkflow(ctx context.Context, conf *Config, taskUUID string) error + DestroyWorkflow(ctx context.Context, conf *Config, taskUUID, workflowName string) error } // BackendInfo represents the reported information of a loaded backend. diff --git a/pipeline/frontend/yaml/parse_test.go b/pipeline/frontend/yaml/parse_test.go index dc9eca5ef40..549c393f30b 100644 --- a/pipeline/frontend/yaml/parse_test.go +++ b/pipeline/frontend/yaml/parse_test.go @@ -147,7 +147,7 @@ pipeline: commands: meh! ` - workflow1, err := ParseString(sampleYamlPipeline) + workflowNameTest, err := ParseString(sampleYamlPipeline) if !assert.NoError(t, err) { return } @@ -157,9 +157,9 @@ pipeline: return } - assert.EqualValues(t, workflow1, workflow2) - assert.Len(t, workflow1.Steps.ContainerList, 1) - assert.EqualValues(t, "say hello", workflow1.Steps.ContainerList[0].Name) + assert.EqualValues(t, workflowNameTest, workflow2) + assert.Len(t, workflowNameTest.Steps.ContainerList, 1) + assert.EqualValues(t, "say hello", workflowNameTest.Steps.ContainerList[0].Name) } var sampleYaml = ` diff --git a/pipeline/pipeline.go b/pipeline/pipeline.go index c7429dbf3a0..5e4d1be910b 100644 --- a/pipeline/pipeline.go +++ b/pipeline/pipeline.go @@ -109,13 +109,13 @@ func (r *Runtime) Run(runnerCtx context.Context) error { if ctx.Err() != nil { ctx = GetShutdownCtx() } - if err := r.engine.DestroyWorkflow(ctx, r.spec, r.taskUUID); err != nil { + if err := r.engine.DestroyWorkflow(ctx, r.spec, r.taskUUID, r.Description["pipeline_name"]); err != nil { logger.Error().Err(err).Msg("could not destroy engine") } }() r.started = time.Now().Unix() - if err := r.engine.SetupWorkflow(runnerCtx, r.spec, r.taskUUID); err != nil { + if err := r.engine.SetupWorkflow(runnerCtx, r.spec, r.taskUUID, r.Description["pipeline_name"]); err != nil { return err } @@ -123,7 +123,7 @@ func (r *Runtime) Run(runnerCtx context.Context) error { select { case <-r.ctx.Done(): return ErrCancel - case err := <-r.execAll(stage.Steps): + case err := <-r.execAll(stage.Steps, r.Description["pipeline_name"]): if err != nil { r.err = err } @@ -163,7 +163,7 @@ func (r *Runtime) traceStep(processState *backend.State, err error, step *backen } // Executes a set of parallel steps. -func (r *Runtime) execAll(steps []*backend.Step) <-chan error { +func (r *Runtime) execAll(steps []*backend.Step, workflowName string) <-chan error { var g errgroup.Group done := make(chan error) logger := r.MakeLogger() @@ -206,7 +206,7 @@ func (r *Runtime) execAll(steps []*backend.Step) <-chan error { Str("step", step.Name). Msg("executing") - processState, err := r.exec(step) + processState, err := r.exec(step, workflowName) logger.Debug(). Str("step", step.Name). @@ -229,14 +229,14 @@ func (r *Runtime) execAll(steps []*backend.Step) <-chan error { } // Executes the step and returns the state and error. -func (r *Runtime) exec(step *backend.Step) (*backend.State, error) { - if err := r.engine.StartStep(r.ctx, step, r.taskUUID); err != nil { +func (r *Runtime) exec(step *backend.Step, workflowName string) (*backend.State, error) { + if err := r.engine.StartStep(r.ctx, step, r.taskUUID, workflowName); err != nil { return nil, err } var wg sync.WaitGroup if r.logger != nil { - rc, err := r.engine.TailStep(r.ctx, step, r.taskUUID) + rc, err := r.engine.TailStep(r.ctx, step, r.taskUUID, workflowName) if err != nil { return nil, err } @@ -261,7 +261,7 @@ func (r *Runtime) exec(step *backend.Step) (*backend.State, error) { // We wait until all data was logged. (Needed for some backends like local as WaitStep kills the log stream) wg.Wait() - waitState, err := r.engine.WaitStep(r.ctx, step, r.taskUUID) + waitState, err := r.engine.WaitStep(r.ctx, step, r.taskUUID, workflowName) if err != nil { if errors.Is(err, context.Canceled) { return waitState, ErrCancel @@ -269,7 +269,7 @@ func (r *Runtime) exec(step *backend.Step) (*backend.State, error) { return nil, err } - if err := r.engine.DestroyStep(r.ctx, step, r.taskUUID); err != nil { + if err := r.engine.DestroyStep(r.ctx, step, r.taskUUID, workflowName); err != nil { return nil, err } From 2db87523727b087c340e427335d3d5b1429c5ae9 Mon Sep 17 00:00:00 2001 From: pat-s Date: Sun, 5 Jan 2025 21:50:54 +0100 Subject: [PATCH 3/8] switch order of elements in name --- pipeline/backend/docker/convert.go | 2 +- pipeline/backend/docker/convert_test.go | 4 +- pipeline/backend/kubernetes/pod.go | 2 +- pipeline/backend/kubernetes/pod_test.go | 44 ++++++++++----------- pipeline/backend/kubernetes/secrets_test.go | 2 +- pipeline/backend/kubernetes/service.go | 2 +- pipeline/backend/kubernetes/service_test.go | 12 +++--- pipeline/backend/kubernetes/utils.go | 21 +++++----- pipeline/backend/kubernetes/utils_test.go | 16 ++++---- pipeline/backend/kubernetes/volume_test.go | 4 +- 10 files changed, 55 insertions(+), 54 deletions(-) diff --git a/pipeline/backend/docker/convert.go b/pipeline/backend/docker/convert.go index a33f40ebd1a..003186a3656 100644 --- a/pipeline/backend/docker/convert.go +++ b/pipeline/backend/docker/convert.go @@ -70,7 +70,7 @@ func (e *docker) toConfig(step *types.Step, options BackendOptions) *container.C } func toContainerName(step *types.Step, workflowName string) string { - return "wp_" + step.UUID[:5] + "-" + workflowName + "-" + step.Name + return "wp_" + workflowName + "-" + step.Name + "-" + step.UUID[:5] } // returns a container host configuration. diff --git a/pipeline/backend/docker/convert_test.go b/pipeline/backend/docker/convert_test.go index 7a549e50699..180aa1aa82b 100644 --- a/pipeline/backend/docker/convert_test.go +++ b/pipeline/backend/docker/convert_test.go @@ -125,8 +125,8 @@ var ( ) func TestToContainerName(t *testing.T) { - assert.EqualValues(t, "wp_f5182-workflowNameTest-hello", toContainerName(testCmdStep, "workflowNameTest")) - assert.EqualValues(t, "wp_d841e-workflowNameTest-lint", toContainerName(testPluginStep, "workflowNameTest")) + 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) { diff --git a/pipeline/backend/kubernetes/pod.go b/pipeline/backend/kubernetes/pod.go index 11de5efadab..506dec18a30 100644 --- a/pipeline/backend/kubernetes/pod.go +++ b/pipeline/backend/kubernetes/pod.go @@ -77,7 +77,7 @@ func stepToPodName(step *types.Step, workflowName string) (name string, err erro } func podName(step *types.Step, workflowName string) (string, error) { - return dnsName(podPrefix + step.UUID[:5] + "-" + workflowName + "-" + step.Name) + return dnsName(podPrefix + workflowName + "-" + step.Name + "-" + step.UUID[:5]) } func podMeta(step *types.Step, config *config, options BackendOptions, podName, workflowName string) (meta_v1.ObjectMeta, error) { diff --git a/pipeline/backend/kubernetes/pod_test.go b/pipeline/backend/kubernetes/pod_test.go index 0f468b4d32e..60961cf2c0a 100644 --- a/pipeline/backend/kubernetes/pod_test.go +++ b/pipeline/backend/kubernetes/pod_test.go @@ -28,31 +28,31 @@ import ( func TestPodName(t *testing.T) { name, err := podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me-0", Name: "go-test"}, "workflowNameTest") assert.NoError(t, err) - assert.Equal(t, "wp-01he8-workflownametest-go-test", name) + assert.Equal(t, "wp-workflownametest-go-test-01he8", name) - _, err = podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me\\0a"}, "workflowNameTest") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + _, err = podName(&types.Step{UUID: "01\\he8bebctabr3kgk0qj36d2me\\0a"}, "workflowNameTest") + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") - _, err = podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me-0-services-0..woodpecker-runtime.svc.cluster.local"}, "workflowNameTest") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + _, err = podName(&types.Step{UUID: "01..he8bebctabr3kgk0qj36d2me-0-services-0..woodpecker-runtime.svc.cluster.local"}, "workflowNameTest") + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") } func TestStepToPodName(t *testing.T) { name, err := stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "clone", Type: types.StepTypeClone}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8-workflownametest-clone", name) + assert.EqualValues(t, "wp-workflownametest-clone-01he8", name) name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "cache", Type: types.StepTypeCache}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8-workflownametest-cache", name) + assert.EqualValues(t, "wp-workflownametest-cache-01he8", name) name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "release", Type: types.StepTypePlugin}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8-workflownametest-release", name) + assert.EqualValues(t, "wp-workflownametest-release-01he8", name) name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "prepare-env", Type: types.StepTypeCommands}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-01he8-workflownametest-prepare-env", name) + assert.EqualValues(t, "wp-workflownametest-prepare-env-01he8", name) name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "postgres", Type: types.StepTypeService}, "workflowNameTest") assert.NoError(t, err) - assert.EqualValues(t, "wp-svc-01he8-workflownametest-postgres", name) + assert.EqualValues(t, "wp-svc-workflownametest-postgres-01he8", name) } func TestStepLabel(t *testing.T) { @@ -61,14 +61,14 @@ func TestStepLabel(t *testing.T) { assert.EqualValues(t, "build-image", name) _, err = stepLabel(&types.Step{Name: ".build.image"}) - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") } func TestTinyPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-01he8-workflownametest-go-test", + "name": "wp-workflownametest-go-test-01he8", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -86,7 +86,7 @@ func TestTinyPod(t *testing.T) { ], "containers": [ { - "name": "wp-01he8-workflownametest-go-test", + "name": "wp-workflownametest-go-test-01he8", "image": "gradle:8.4.0-jdk21", "command": [ "/bin/sh", @@ -133,7 +133,7 @@ func TestTinyPod(t *testing.T) { Environment: map[string]string{"CI": "woodpecker"}, }, &config{ Namespace: "woodpecker", - }, "wp-01he8-workflownametest-go-test", "linux/amd64", BackendOptions{}, "workflowNameTest") + }, "wp-workflownametest-go-test-01he8", "linux/amd64", BackendOptions{}, "workflowNameTest") assert.NoError(t, err) podJSON, err := json.Marshal(pod) @@ -147,7 +147,7 @@ func TestFullPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-01he8-workflownametest-go-test", + "name": "wp-workflownametest-go-test-01he8", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -171,7 +171,7 @@ func TestFullPod(t *testing.T) { ], "containers": [ { - "name": "wp-01he8-workflownametest-go-test", + "name": "wp-workflownametest-go-test-01he8", "image": "meltwater/drone-cache", "command": [ "/bin/sh", @@ -256,7 +256,7 @@ func TestFullPod(t *testing.T) { "name": "another-pull-secret" }, { - "name": "wp-01he8-workflownametest-go-test" + "name": "wp-workflownametest-go-test-01he8" } ], "tolerations": [ @@ -335,7 +335,7 @@ func TestFullPod(t *testing.T) { PodAnnotationsAllowFromStep: true, PodNodeSelector: map[string]string{"topology.kubernetes.io/region": "eu-central-1"}, SecurityContext: SecurityContextConfig{RunAsNonRoot: false}, - }, "wp-01he8-workflownametest-go-test", "linux/amd64", BackendOptions{ + }, "wp-workflownametest-go-test-01he8", "linux/amd64", BackendOptions{ Labels: map[string]string{"part-of": "woodpecker-ci"}, Annotations: map[string]string{"kubernetes.io/limit-ranger": "LimitRanger plugin set: cpu, memory request and limit for container"}, NodeSelector: map[string]string{"storage": "ssd"}, @@ -366,7 +366,7 @@ func TestPodPrivilege(t *testing.T) { }, &config{ Namespace: "woodpecker", SecurityContext: SecurityContextConfig{RunAsNonRoot: globalRunAsRoot}, - }, "wp-01he8-workflownametest-go-test", "linux/amd64", BackendOptions{ + }, "wp-workflownametest-go-test-01he8", "linux/amd64", BackendOptions{ SecurityContext: &secCtx, }, "workflowNameTest") } @@ -443,7 +443,7 @@ func TestScratchPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-01he8-workflownametest-go-test", + "name": "wp-workflownametest-go-test-01he8", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -453,7 +453,7 @@ func TestScratchPod(t *testing.T) { "spec": { "containers": [ { - "name": "wp-01he8-workflownametest-go-test", + "name": "wp-workflownametest-go-test-01he8", "image": "quay.io/curl/curl", "command": [ "/usr/bin/curl", @@ -474,7 +474,7 @@ func TestScratchPod(t *testing.T) { Entrypoint: []string{"/usr/bin/curl", "-v", "google.com"}, }, &config{ Namespace: "woodpecker", - }, "wp-01he8-workflownametest-go-test", "linux/amd64", BackendOptions{}, "workflowNameTest") + }, "wp-workflownametest-go-test-01he8", "linux/amd64", BackendOptions{}, "workflowNameTest") assert.NoError(t, err) podJSON, err := json.Marshal(pod) diff --git a/pipeline/backend/kubernetes/secrets_test.go b/pipeline/backend/kubernetes/secrets_test.go index 0bb8df75dfd..4300b4d9139 100644 --- a/pipeline/backend/kubernetes/secrets_test.go +++ b/pipeline/backend/kubernetes/secrets_test.go @@ -208,7 +208,7 @@ func TestUsernameAndPasswordNeedsSecret(t *testing.T) { func TestRegistrySecret(t *testing.T) { const expected = `{ "metadata": { - "name": "wp-01he8-workflownametest-go-test", + "name": "wp-workflownametest-go-test-01he8", "namespace": "woodpecker", "creationTimestamp": null, "labels": { diff --git a/pipeline/backend/kubernetes/service.go b/pipeline/backend/kubernetes/service.go index c0e907b4d8d..45c49472a8d 100644 --- a/pipeline/backend/kubernetes/service.go +++ b/pipeline/backend/kubernetes/service.go @@ -63,7 +63,7 @@ func mkService(step *types.Step, config *config, workflowName string) (*v1.Servi } func serviceName(step *types.Step, workflowName string) (string, error) { - return dnsName(servicePrefix + step.UUID[:5] + "-" + workflowName + "-" + step.Name) + return dnsName(servicePrefix + workflowName + "-" + step.Name + "-" + step.UUID[:5]) } func servicePort(port types.Port) v1.ServicePort { diff --git a/pipeline/backend/kubernetes/service_test.go b/pipeline/backend/kubernetes/service_test.go index b8fee573cda..338d35cfbc0 100644 --- a/pipeline/backend/kubernetes/service_test.go +++ b/pipeline/backend/kubernetes/service_test.go @@ -26,22 +26,22 @@ import ( func TestServiceName(t *testing.T) { name, err := serviceName(&types.Step{Name: "database", UUID: "01he8bebctabr3kgk0qj36d2me"}, "workflowNameTest") assert.NoError(t, err) - assert.Equal(t, "wp-svc-01he8-workflownametest-database", name) + assert.Equal(t, "wp-svc-workflownametest-database-01he8", name) - name, err = serviceName(&types.Step{Name: "wp-01he8-workflownametest-clone-0-services-0.woodpecker-runtime.svc.cluster.local", UUID: "01he8bebctabr3kgk0qj36d2me"}, "workflowNameTest") + name, err = serviceName(&types.Step{Name: "wp-workflownametest-clone-01he8-0-services-0.woodpecker-runtime.svc.cluster.local", UUID: "01he8bebctabr3kgk0qj36d2me"}, "workflowNameTest") assert.NoError(t, err) - assert.Equal(t, "wp-svc-01he8-workflownametest-wp-01he8-workflownametest-clone-0-services-0.woodpecker-runtime.svc.cluster.local", name) + assert.Equal(t, "wp-svc-workflownametest-wp-workflownametest-clone-01he8-0-services-0.woodpecker-runtime.svc.cluster.local-01he8", name) name, err = serviceName(&types.Step{Name: "awesome_service", UUID: "01he8bebctabr3kgk0qj36d2me"}, "workflowNameTest") assert.NoError(t, err) - assert.Equal(t, "wp-svc-01he8-workflownametest-awesome-service", name) + assert.Equal(t, "wp-svc-workflownametest-awesome-service-01he8", name) } func TestService(t *testing.T) { expected := ` { "metadata": { - "name": "wp-svc-01he8-workflownametest-bar", + "name": "wp-svc-workflownametest-bar-01he8", "namespace": "foo", "creationTimestamp": null }, @@ -66,7 +66,7 @@ func TestService(t *testing.T) { } ], "selector": { - "service": "wp-svc-01he8-workflownametest-bar" + "service": "wp-svc-workflownametest-bar-01he8" }, "type": "ClusterIP" }, diff --git a/pipeline/backend/kubernetes/utils.go b/pipeline/backend/kubernetes/utils.go index 46e57970bb7..b68bc4731fd 100644 --- a/pipeline/backend/kubernetes/utils.go +++ b/pipeline/backend/kubernetes/utils.go @@ -15,7 +15,6 @@ package kubernetes import ( - "errors" "fmt" "os" "regexp" @@ -28,20 +27,22 @@ import ( ) var ( - dnsPattern = regexp.MustCompile(`^[a-z0-9]` + // must start with - `([-a-z0-9]*[a-z0-9])?` + // inside can als contain - - `(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`, // allow the same pattern as before with dots in between but only one dot - ) - dnsDisallowedCharacters = regexp.MustCompile(`[^-^.a-z0-9]+`) - ErrDNSPatternInvalid = errors.New("name is not a valid kubernetes DNS name") + dnsPattern = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`) + dnsDisallowedCharacters = regexp.MustCompile(`[^-a-z0-9.]+`) ) func dnsName(i string) (string, error) { res := strings.ToLower(strings.ReplaceAll(i, "_", "-")) - found := dnsPattern.FindStringIndex(res) - if found == nil { - return "", fmt.Errorf("%w: found invalid characters '%v'", ErrDNSPatternInvalid, found) + // Check for invalid characters (dnsDisallowedCharacters) + invalidChars := dnsDisallowedCharacters.FindAllString(res, -1) + if len(invalidChars) > 0 { + return "", fmt.Errorf("name is not a valid kubernetes DNS name: found invalid characters '%v'", strings.Join(invalidChars, "")) + } + + // Check if the entire string matches the dnsPattern + if !dnsPattern.MatchString(res) { + return "", fmt.Errorf("name is not a valid kubernetes DNS name") } return res, nil } diff --git a/pipeline/backend/kubernetes/utils_test.go b/pipeline/backend/kubernetes/utils_test.go index 43c5b8a1a5a..7836ebb1fed 100644 --- a/pipeline/backend/kubernetes/utils_test.go +++ b/pipeline/backend/kubernetes/utils_test.go @@ -34,25 +34,25 @@ func TestDNSName(t *testing.T) { assert.Equal(t, "wp-01he8bebctabr3kgk0qj36d2me-0-services-0.woodpecker-runtime.svc.cluster.local", name) _, err = dnsName(".0-a") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") _, err = dnsName("ABC..DEF") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") _, err = dnsName("0.-a") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") _, err = dnsName("test-") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") _, err = dnsName("-test") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") _, err = dnsName("0-a.") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") _, err = dnsName("abc\\def") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") } func TestToDnsName(t *testing.T) { @@ -69,5 +69,5 @@ func TestToDnsName(t *testing.T) { assert.Equal(t, "build--deploy", name) _, err = toDNSName("-build-and-deploy") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") } diff --git a/pipeline/backend/kubernetes/volume_test.go b/pipeline/backend/kubernetes/volume_test.go index 07ec7c30e93..fa0315b5adc 100644 --- a/pipeline/backend/kubernetes/volume_test.go +++ b/pipeline/backend/kubernetes/volume_test.go @@ -27,10 +27,10 @@ func TestPvcName(t *testing.T) { assert.Equal(t, "woodpecker-cache", name) _, err = volumeName("woodpecker\\cache") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") _, err = volumeName("-woodpecker.cache:/woodpecker/src/cache") - assert.ErrorIs(t, err, ErrDNSPatternInvalid) + assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") } func TestPvcMount(t *testing.T) { From 5a0c941d34c905e6a8a93cd25f21800e9ea9feb6 Mon Sep 17 00:00:00 2001 From: pat-s Date: Sun, 5 Jan 2025 21:56:28 +0100 Subject: [PATCH 4/8] set max length for names --- pipeline/backend/kubernetes/utils.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pipeline/backend/kubernetes/utils.go b/pipeline/backend/kubernetes/utils.go index b68bc4731fd..cb5165f7580 100644 --- a/pipeline/backend/kubernetes/utils.go +++ b/pipeline/backend/kubernetes/utils.go @@ -34,16 +34,21 @@ var ( func dnsName(i string) (string, error) { res := strings.ToLower(strings.ReplaceAll(i, "_", "-")) - // Check for invalid characters (dnsDisallowedCharacters) invalidChars := dnsDisallowedCharacters.FindAllString(res, -1) if len(invalidChars) > 0 { return "", fmt.Errorf("name is not a valid kubernetes DNS name: found invalid characters '%v'", strings.Join(invalidChars, "")) } - // Check if the entire string matches the dnsPattern if !dnsPattern.MatchString(res) { return "", fmt.Errorf("name is not a valid kubernetes DNS name") } + + // k8s pod name limit + const maxPodNameLength = 253 + if len(res) > maxPodNameLength { + res = res[:maxPodNameLength] + } + return res, nil } From 182051790e4fe53f02d7a4a378493ff315217e20 Mon Sep 17 00:00:00 2001 From: pat-s Date: Mon, 6 Jan 2025 09:44:47 +0100 Subject: [PATCH 5/8] pipelineName -> workflowName --- agent/runner.go | 6 +++--- pipeline/pipeline.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/agent/runner.go b/agent/runner.go index a0bb4c095a3..411c7d10cdf 100644 --- a/agent/runner.go +++ b/agent/runner.go @@ -72,7 +72,7 @@ func (r *Runner) Run(runnerCtx, shutdownCtx context.Context) error { //nolint:co repoName := extractRepositoryName(workflow.Config) // hack pipelineNumber := extractPipelineNumber(workflow.Config) // hack - pipelineName := extractPipelineName(workflow.Config) // hack + workflowName := extractWorkflowName(workflow.Config) // hack r.counter.Add( workflow.ID, @@ -150,7 +150,7 @@ func (r *Runner) Run(runnerCtx, shutdownCtx context.Context) error { //nolint:co "workflow_id": workflow.ID, "repo": repoName, "pipeline_number": pipelineNumber, - "pipeline_name": pipelineName, + "workflowName": workflowName, }), ).Run(runnerCtx) @@ -200,6 +200,6 @@ func extractPipelineNumber(config *backend.Config) string { return config.Stages[0].Steps[0].Environment["CI_PIPELINE_NUMBER"] } -func extractPipelineName(config *backend.Config) string { +func extractWorkflowName(config *backend.Config) string { return config.Stages[0].Steps[0].Environment["CI_WORKFLOW_NAME"] } diff --git a/pipeline/pipeline.go b/pipeline/pipeline.go index 5e4d1be910b..3bbc2ea7332 100644 --- a/pipeline/pipeline.go +++ b/pipeline/pipeline.go @@ -109,13 +109,13 @@ func (r *Runtime) Run(runnerCtx context.Context) error { if ctx.Err() != nil { ctx = GetShutdownCtx() } - if err := r.engine.DestroyWorkflow(ctx, r.spec, r.taskUUID, r.Description["pipeline_name"]); err != nil { + if err := r.engine.DestroyWorkflow(ctx, r.spec, r.taskUUID, r.Description["workflowName"]); err != nil { logger.Error().Err(err).Msg("could not destroy engine") } }() r.started = time.Now().Unix() - if err := r.engine.SetupWorkflow(runnerCtx, r.spec, r.taskUUID, r.Description["pipeline_name"]); err != nil { + if err := r.engine.SetupWorkflow(runnerCtx, r.spec, r.taskUUID, r.Description["workflowName"]); err != nil { return err } @@ -123,7 +123,7 @@ func (r *Runtime) Run(runnerCtx context.Context) error { select { case <-r.ctx.Done(): return ErrCancel - case err := <-r.execAll(stage.Steps, r.Description["pipeline_name"]): + case err := <-r.execAll(stage.Steps, r.Description["workflowName"]): if err != nil { r.err = err } From 5bcb00ffc3ebc09dd09275fd64ef9aeab552baed Mon Sep 17 00:00:00 2001 From: pat-s Date: Tue, 7 Jan 2025 09:06:07 +0100 Subject: [PATCH 6/8] use 5 last chars of uuid --- pipeline/backend/docker/convert.go | 2 +- pipeline/backend/kubernetes/pod.go | 2 +- pipeline/backend/kubernetes/service.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pipeline/backend/docker/convert.go b/pipeline/backend/docker/convert.go index 003186a3656..b1d0fd8cae9 100644 --- a/pipeline/backend/docker/convert.go +++ b/pipeline/backend/docker/convert.go @@ -70,7 +70,7 @@ func (e *docker) toConfig(step *types.Step, options BackendOptions) *container.C } func toContainerName(step *types.Step, workflowName string) string { - return "wp_" + workflowName + "-" + step.Name + "-" + step.UUID[:5] + return "wp_" + workflowName + "-" + step.Name + "-" + step.UUID[len(step.UUID)-5:] } // returns a container host configuration. diff --git a/pipeline/backend/kubernetes/pod.go b/pipeline/backend/kubernetes/pod.go index 506dec18a30..93f5f58bffe 100644 --- a/pipeline/backend/kubernetes/pod.go +++ b/pipeline/backend/kubernetes/pod.go @@ -77,7 +77,7 @@ func stepToPodName(step *types.Step, workflowName string) (name string, err erro } func podName(step *types.Step, workflowName string) (string, error) { - return dnsName(podPrefix + workflowName + "-" + step.Name + "-" + step.UUID[:5]) + return dnsName(podPrefix + workflowName + "-" + step.Name + "-" + step.UUID[len(step.UUID)-5:]) } func podMeta(step *types.Step, config *config, options BackendOptions, podName, workflowName string) (meta_v1.ObjectMeta, error) { diff --git a/pipeline/backend/kubernetes/service.go b/pipeline/backend/kubernetes/service.go index 45c49472a8d..1f5aad35ca1 100644 --- a/pipeline/backend/kubernetes/service.go +++ b/pipeline/backend/kubernetes/service.go @@ -63,7 +63,7 @@ func mkService(step *types.Step, config *config, workflowName string) (*v1.Servi } func serviceName(step *types.Step, workflowName string) (string, error) { - return dnsName(servicePrefix + workflowName + "-" + step.Name + "-" + step.UUID[:5]) + return dnsName(servicePrefix + workflowName + "-" + step.Name + "-" + step.UUID[len(step.UUID)-5:]) } func servicePort(port types.Port) v1.ServicePort { From 5b6bf0e00060d3579f52211f65340ef44b31e1b2 Mon Sep 17 00:00:00 2001 From: pat-s Date: Tue, 7 Jan 2025 09:22:11 +0100 Subject: [PATCH 7/8] use google/uuid for random 5 char string --- go.mod | 2 +- pipeline/backend/docker/convert.go | 2 +- pipeline/backend/kubernetes/pod.go | 2 +- pipeline/backend/kubernetes/service.go | 2 +- pipeline/pipeline.go | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 8e2077ae48c..04d03a80130 100644 --- a/go.mod +++ b/go.mod @@ -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 @@ -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 diff --git a/pipeline/backend/docker/convert.go b/pipeline/backend/docker/convert.go index b1d0fd8cae9..a1b11189be8 100644 --- a/pipeline/backend/docker/convert.go +++ b/pipeline/backend/docker/convert.go @@ -70,7 +70,7 @@ func (e *docker) toConfig(step *types.Step, options BackendOptions) *container.C } func toContainerName(step *types.Step, workflowName string) string { - return "wp_" + workflowName + "-" + step.Name + "-" + step.UUID[len(step.UUID)-5:] + return "wp_" + workflowName + "-" + step.Name + "-" + step.UUID } // returns a container host configuration. diff --git a/pipeline/backend/kubernetes/pod.go b/pipeline/backend/kubernetes/pod.go index 93f5f58bffe..63c33d92ff3 100644 --- a/pipeline/backend/kubernetes/pod.go +++ b/pipeline/backend/kubernetes/pod.go @@ -77,7 +77,7 @@ func stepToPodName(step *types.Step, workflowName string) (name string, err erro } func podName(step *types.Step, workflowName string) (string, error) { - return dnsName(podPrefix + workflowName + "-" + step.Name + "-" + step.UUID[len(step.UUID)-5:]) + return dnsName(podPrefix + workflowName + "-" + step.Name + "-" + step.UUID) } func podMeta(step *types.Step, config *config, options BackendOptions, podName, workflowName string) (meta_v1.ObjectMeta, error) { diff --git a/pipeline/backend/kubernetes/service.go b/pipeline/backend/kubernetes/service.go index 1f5aad35ca1..668206a6a30 100644 --- a/pipeline/backend/kubernetes/service.go +++ b/pipeline/backend/kubernetes/service.go @@ -63,7 +63,7 @@ func mkService(step *types.Step, config *config, workflowName string) (*v1.Servi } func serviceName(step *types.Step, workflowName string) (string, error) { - return dnsName(servicePrefix + workflowName + "-" + step.Name + "-" + step.UUID[len(step.UUID)-5:]) + return dnsName(servicePrefix + workflowName + "-" + step.Name + "-" + step.UUID) } func servicePort(port types.Port) v1.ServicePort { diff --git a/pipeline/pipeline.go b/pipeline/pipeline.go index 3bbc2ea7332..e5861dac01a 100644 --- a/pipeline/pipeline.go +++ b/pipeline/pipeline.go @@ -21,7 +21,7 @@ import ( "sync" "time" - "github.com/oklog/ulid/v2" + "github.com/google/uuid" "github.com/rs/zerolog" "github.com/rs/zerolog/log" "golang.org/x/sync/errgroup" @@ -73,7 +73,7 @@ func New(spec *backend.Config, opts ...Option) *Runtime { r.Description = map[string]string{} r.spec = spec r.ctx = context.Background() - r.taskUUID = ulid.Make().String() + r.taskUUID = uuid.New().String()[:5] for _, opts := range opts { opts(r) } From 5873d01aa139e0f001efccca3c5b0b435d3bbbc5 Mon Sep 17 00:00:00 2001 From: pat-s Date: Tue, 7 Jan 2025 11:34:30 +0100 Subject: [PATCH 8/8] tests --- pipeline/backend/docker/convert_test.go | 4 +-- pipeline/backend/kubernetes/pod_test.go | 38 ++++++++++----------- pipeline/backend/kubernetes/secrets_test.go | 4 +-- pipeline/backend/kubernetes/service_test.go | 12 +++---- pipeline/backend/kubernetes/utils_test.go | 8 ++--- 5 files changed, 33 insertions(+), 33 deletions(-) diff --git a/pipeline/backend/docker/convert_test.go b/pipeline/backend/docker/convert_test.go index 180aa1aa82b..6118f349080 100644 --- a/pipeline/backend/docker/convert_test.go +++ b/pipeline/backend/docker/convert_test.go @@ -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", @@ -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), diff --git a/pipeline/backend/kubernetes/pod_test.go b/pipeline/backend/kubernetes/pod_test.go index 60961cf2c0a..e6860397970 100644 --- a/pipeline/backend/kubernetes/pod_test.go +++ b/pipeline/backend/kubernetes/pod_test.go @@ -26,9 +26,9 @@ import ( ) func TestPodName(t *testing.T) { - name, err := podName(&types.Step{UUID: "01he8bebctabr3kgk0qj36d2me-0", Name: "go-test"}, "workflowNameTest") + name, err := podName(&types.Step{UUID: "01he8-0", Name: "go-test"}, "workflowNameTest") assert.NoError(t, err) - assert.Equal(t, "wp-workflownametest-go-test-01he8", name) + assert.Equal(t, "wp-workflownametest-go-test-01he8-0", name) _, err = podName(&types.Step{UUID: "01\\he8bebctabr3kgk0qj36d2me\\0a"}, "workflowNameTest") assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name") @@ -38,19 +38,19 @@ func TestPodName(t *testing.T) { } func TestStepToPodName(t *testing.T) { - name, err := stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "clone", Type: types.StepTypeClone}, "workflowNameTest") + name, err := stepToPodName(&types.Step{UUID: "01he8", Name: "clone", Type: types.StepTypeClone}, "workflowNameTest") assert.NoError(t, err) assert.EqualValues(t, "wp-workflownametest-clone-01he8", name) - name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "cache", Type: types.StepTypeCache}, "workflowNameTest") + name, err = stepToPodName(&types.Step{UUID: "01he8", Name: "cache", Type: types.StepTypeCache}, "workflowNameTest") assert.NoError(t, err) assert.EqualValues(t, "wp-workflownametest-cache-01he8", name) - name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "release", Type: types.StepTypePlugin}, "workflowNameTest") + name, err = stepToPodName(&types.Step{UUID: "01he8", Name: "release", Type: types.StepTypePlugin}, "workflowNameTest") assert.NoError(t, err) assert.EqualValues(t, "wp-workflownametest-release-01he8", name) - name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "prepare-env", Type: types.StepTypeCommands}, "workflowNameTest") + name, err = stepToPodName(&types.Step{UUID: "01he8", Name: "prepare-env", Type: types.StepTypeCommands}, "workflowNameTest") assert.NoError(t, err) assert.EqualValues(t, "wp-workflownametest-prepare-env-01he8", name) - name, err = stepToPodName(&types.Step{UUID: "01he8bebctabr3kg", Name: "postgres", Type: types.StepTypeService}, "workflowNameTest") + name, err = stepToPodName(&types.Step{UUID: "01he8", Name: "postgres", Type: types.StepTypeService}, "workflowNameTest") assert.NoError(t, err) assert.EqualValues(t, "wp-svc-workflownametest-postgres-01he8", name) } @@ -68,7 +68,7 @@ func TestTinyPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-workflownametest-go-test-01he8", + "name": "wp-workflownametest-go-test-01he8-0", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -86,7 +86,7 @@ func TestTinyPod(t *testing.T) { ], "containers": [ { - "name": "wp-workflownametest-go-test-01he8", + "name": "wp-workflownametest-go-test-01he8-0", "image": "gradle:8.4.0-jdk21", "command": [ "/bin/sh", @@ -133,7 +133,7 @@ func TestTinyPod(t *testing.T) { Environment: map[string]string{"CI": "woodpecker"}, }, &config{ Namespace: "woodpecker", - }, "wp-workflownametest-go-test-01he8", "linux/amd64", BackendOptions{}, "workflowNameTest") + }, "wp-workflownametest-go-test-01he8-0", "linux/amd64", BackendOptions{}, "workflowNameTest") assert.NoError(t, err) podJSON, err := json.Marshal(pod) @@ -147,7 +147,7 @@ func TestFullPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-workflownametest-go-test-01he8", + "name": "wp-workflownametest-go-test-01he8-0", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -171,7 +171,7 @@ func TestFullPod(t *testing.T) { ], "containers": [ { - "name": "wp-workflownametest-go-test-01he8", + "name": "wp-workflownametest-go-test-01he8-0", "image": "meltwater/drone-cache", "command": [ "/bin/sh", @@ -256,7 +256,7 @@ func TestFullPod(t *testing.T) { "name": "another-pull-secret" }, { - "name": "wp-workflownametest-go-test-01he8" + "name": "wp-workflownametest-go-test-01he8-0" } ], "tolerations": [ @@ -310,7 +310,7 @@ func TestFullPod(t *testing.T) { }, } pod, err := mkPod(&types.Step{ - UUID: "01he8bebctabr3kgk0qj36d2me-0", + UUID: "01he8-0", Name: "go-test", Image: "meltwater/drone-cache", WorkingDir: "/woodpecker/src", @@ -335,7 +335,7 @@ func TestFullPod(t *testing.T) { PodAnnotationsAllowFromStep: true, PodNodeSelector: map[string]string{"topology.kubernetes.io/region": "eu-central-1"}, SecurityContext: SecurityContextConfig{RunAsNonRoot: false}, - }, "wp-workflownametest-go-test-01he8", "linux/amd64", BackendOptions{ + }, "wp-workflownametest-go-test-01he8-0", "linux/amd64", BackendOptions{ Labels: map[string]string{"part-of": "woodpecker-ci"}, Annotations: map[string]string{"kubernetes.io/limit-ranger": "LimitRanger plugin set: cpu, memory request and limit for container"}, NodeSelector: map[string]string{"storage": "ssd"}, @@ -366,7 +366,7 @@ func TestPodPrivilege(t *testing.T) { }, &config{ Namespace: "woodpecker", SecurityContext: SecurityContextConfig{RunAsNonRoot: globalRunAsRoot}, - }, "wp-workflownametest-go-test-01he8", "linux/amd64", BackendOptions{ + }, "wp-workflownametest-go-test-01he8-0", "linux/amd64", BackendOptions{ SecurityContext: &secCtx, }, "workflowNameTest") } @@ -443,7 +443,7 @@ func TestScratchPod(t *testing.T) { const expected = ` { "metadata": { - "name": "wp-workflownametest-go-test-01he8", + "name": "wp-workflownametest-go-test-01he8-0", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -453,7 +453,7 @@ func TestScratchPod(t *testing.T) { "spec": { "containers": [ { - "name": "wp-workflownametest-go-test-01he8", + "name": "wp-workflownametest-go-test-01he8-0", "image": "quay.io/curl/curl", "command": [ "/usr/bin/curl", @@ -474,7 +474,7 @@ func TestScratchPod(t *testing.T) { Entrypoint: []string{"/usr/bin/curl", "-v", "google.com"}, }, &config{ Namespace: "woodpecker", - }, "wp-workflownametest-go-test-01he8", "linux/amd64", BackendOptions{}, "workflowNameTest") + }, "wp-workflownametest-go-test-01he8-0", "linux/amd64", BackendOptions{}, "workflowNameTest") assert.NoError(t, err) podJSON, err := json.Marshal(pod) diff --git a/pipeline/backend/kubernetes/secrets_test.go b/pipeline/backend/kubernetes/secrets_test.go index 4300b4d9139..8e3c9398d16 100644 --- a/pipeline/backend/kubernetes/secrets_test.go +++ b/pipeline/backend/kubernetes/secrets_test.go @@ -208,7 +208,7 @@ func TestUsernameAndPasswordNeedsSecret(t *testing.T) { func TestRegistrySecret(t *testing.T) { const expected = `{ "metadata": { - "name": "wp-workflownametest-go-test-01he8", + "name": "wp-workflownametest-go-test-01he8-0", "namespace": "woodpecker", "creationTimestamp": null, "labels": { @@ -222,7 +222,7 @@ func TestRegistrySecret(t *testing.T) { }` secret, err := mkRegistrySecret(&types.Step{ - UUID: "01he8bebctabr3kgk0qj36d2me-0", + UUID: "01he8-0", Name: "go-test", Image: "meltwater/drone-cache", AuthConfig: types.Auth{ diff --git a/pipeline/backend/kubernetes/service_test.go b/pipeline/backend/kubernetes/service_test.go index 338d35cfbc0..185c5a3811e 100644 --- a/pipeline/backend/kubernetes/service_test.go +++ b/pipeline/backend/kubernetes/service_test.go @@ -24,15 +24,15 @@ import ( ) func TestServiceName(t *testing.T) { - name, err := serviceName(&types.Step{Name: "database", UUID: "01he8bebctabr3kgk0qj36d2me"}, "workflowNameTest") + name, err := serviceName(&types.Step{Name: "database", UUID: "01he8"}, "workflowNameTest") assert.NoError(t, err) assert.Equal(t, "wp-svc-workflownametest-database-01he8", name) - name, err = serviceName(&types.Step{Name: "wp-workflownametest-clone-01he8-0-services-0.woodpecker-runtime.svc.cluster.local", UUID: "01he8bebctabr3kgk0qj36d2me"}, "workflowNameTest") + name, err = serviceName(&types.Step{Name: "wp-workflownametest-clone-01he8-0-services-0.woodpecker-runtime.svc.cluster.local", UUID: "01he8"}, "workflowNameTest") assert.NoError(t, err) assert.Equal(t, "wp-svc-workflownametest-wp-workflownametest-clone-01he8-0-services-0.woodpecker-runtime.svc.cluster.local-01he8", name) - name, err = serviceName(&types.Step{Name: "awesome_service", UUID: "01he8bebctabr3kgk0qj36d2me"}, "workflowNameTest") + name, err = serviceName(&types.Step{Name: "awesome_service", UUID: "01he8"}, "workflowNameTest") assert.NoError(t, err) assert.Equal(t, "wp-svc-workflownametest-awesome-service-01he8", name) } @@ -41,7 +41,7 @@ func TestService(t *testing.T) { expected := ` { "metadata": { - "name": "wp-svc-workflownametest-bar-01he8", + "name": "wp-svc-workflownametest-bar-01he8-0", "namespace": "foo", "creationTimestamp": null }, @@ -66,7 +66,7 @@ func TestService(t *testing.T) { } ], "selector": { - "service": "wp-svc-workflownametest-bar-01he8" + "service": "wp-svc-workflownametest-bar-01he8-0" }, "type": "ClusterIP" }, @@ -81,7 +81,7 @@ func TestService(t *testing.T) { } s, err := mkService(&types.Step{ Name: "bar", - UUID: "01he8bebctabr3kgk0qj36d2me-0", + UUID: "01he8-0", Ports: ports, }, &config{Namespace: "foo"}, "workflowNameTest") assert.NoError(t, err) diff --git a/pipeline/backend/kubernetes/utils_test.go b/pipeline/backend/kubernetes/utils_test.go index 7836ebb1fed..17da4165f44 100644 --- a/pipeline/backend/kubernetes/utils_test.go +++ b/pipeline/backend/kubernetes/utils_test.go @@ -21,17 +21,17 @@ import ( ) func TestDNSName(t *testing.T) { - name, err := dnsName("wp_01he8bebctabr3kgk0qj36d2me_0_services_0") + name, err := dnsName("wp_01he8_0_services_0") assert.NoError(t, err) - assert.Equal(t, "wp-01he8bebctabr3kgk0qj36d2me-0-services-0", name) + assert.Equal(t, "wp-01he8-0-services-0", name) name, err = dnsName("a.0-AA") assert.NoError(t, err) assert.Equal(t, "a.0-aa", name) - name, err = dnsName("wp-01he8bebctabr3kgk0qj36d2me-0-services-0.woodpecker-runtime.svc.cluster.local") + name, err = dnsName("wp-01he8-0-services-0.woodpecker-runtime.svc.cluster.local") assert.NoError(t, err) - assert.Equal(t, "wp-01he8bebctabr3kgk0qj36d2me-0-services-0.woodpecker-runtime.svc.cluster.local", name) + assert.Equal(t, "wp-01he8-0-services-0.woodpecker-runtime.svc.cluster.local", name) _, err = dnsName(".0-a") assert.ErrorContains(t, err, "name is not a valid kubernetes DNS name")