fixup! feat(plz): pass new config from GCk6 to PLZ test runs

Author: yorugac

pkg/cloud/types.go

@@ -18,10 +18,11 @@ var reservedGCk6EnvVars = map[string]struct{}{}
 const (
 	// Reserved vars set for PLZ tests, as described here:
 	// https://grafana.com/docs/grafana-cloud/testing/k6/author-run/cloud-scripting-extras/cloud-execution-context-variables/
+	// These are not passed from GCk6, but set by k6-operator directly.
 	lzCloudExecVar    = "K6_CLOUDRUN_LOAD_ZONE"
 	distrCloudExecVar = "K6_CLOUDRUN_DISTRIBUTION"
 	trIDCloudExecVar  = "K6_CLOUDRUN_TEST_RUN_ID"
-	// it's exported as it must be set in external package, as part of TestRun CRD flow
+	// IIDCloudExecVar is exported as it must be set in external package, as part of TestRun CRD flow
 	IIDCloudExecVar = "K6_CLOUDRUN_INSTANCE_ID"
 
 	secretSourceEnvVar      = "K6_SECRET_SOURCE"
@@ -96,14 +97,19 @@ type TestRunData struct {
 	// LZDistribution holds label -> distribution mapping relevant
 	// for the given script and PLZ
 	LZDistribution `json:"load_zone_distribution,omitempty"`
+
+	// TagArgs and EnvArgs are sorted CLI argument strings, populated by Build().
+	TagArgs string `json:"-"`
+	EnvArgs string `json:"-"`
 }
 
 func (trd *TestRunData) TestRunID() string {
 	return fmt.Sprintf("%d", trd.TestRunId)
 }
 
-// Build adds specific for GCk6 tags and env vars to data.
-// Returns error if it's impossible.
+// Build adds specific for GCk6 tags and env vars to data,
+// and produces sorted CLI argument strings for tags and environment.
+// Returns error if distribution is empty.
 func (trd *TestRunData) Build() error {
 	if len(trd.LZDistribution) != 1 {
 		return fmt.Errorf("only tests with one load zone are supported, provided: %+v", trd.LZDistribution)
@@ -119,9 +125,27 @@ func (trd *TestRunData) Build() error {
 	trd.Environment[distrCloudExecVar] = trd.LZLabel()
 	trd.Environment[trIDCloudExecVar] = trd.TestRunID()
 
+	trd.TagArgs = sortedArgs("--tag", trd.Tags)
+	trd.EnvArgs = sortedArgs("-e", trd.Environment)
+
 	return nil
 }
 
+// sortedArgs builds a CLI argument string from a map, sorted by key.
+func sortedArgs(flag string, m map[string]string) string {
+	keys := make([]string, 0, len(m))
+	for k := range m {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+
+	var s string
+	for _, k := range keys {
+		s += " " + flag + " " + fmt.Sprintf(`%s=%s`, k, m[k])
+	}
+	return s
+}
+
 type LZConfig struct {
 	RunnerImage   string `json:"load_runner_image,omitempty"`
 	InstanceCount int    `json:"instance_count,omitempty"`
@@ -145,9 +169,9 @@ type CLIArgs struct {
 	UserAgent            string            `json:"user_agent,omitempty"`
 }
 
-type LZDistribution map[string]distribution
+type LZDistribution map[string]Distribution
 
-type distribution struct {
+type Distribution struct {
 	LoadZone string `json:"loadZone"`
 	Percent  int    `json:"percent"`
 }

pkg/plz/worker.go

@@ -192,22 +192,12 @@ func (w *PLZWorker) complete(tr *v1alpha1.TestRun, trData *cloud.TestRunData) {
 	// building argument list to k6
 	bips := `--blacklist-ip="` + strings.Join(trData.BlacklistIPs, ",") + `"`
 	bhns := `--block-hostnames="` + strings.Join(trData.BlockedHostnames, ",") + `"`
-	var tags string
-	for k, v := range trData.Tags {
-		tags += " --tag " + fmt.Sprintf(`%s=%s`, k, v)
-	}
-
-	var evs string
-	for k, v := range trData.Environment {
-		evs += " -e " + fmt.Sprintf(`%s=%s`, k, v)
-	}
-
 	tr.Spec.Arguments = fmt.Sprintf(`--out cloud %s %s %s --no-thresholds --user-agent=%s --log-output=loki=https://cloudlogs.k6.io/api/v1/push,label.lz=%s,label.test_run_id=%s,header.Authorization="Token $(K6_CLOUD_TOKEN)" %s`,
 		bips, bhns,
-		tags, trData.UserAgent,
+		trData.TagArgs, trData.UserAgent,
 		w.plz.Name,
 		trData.TestRunID(),
-		evs)
+		trData.EnvArgs)
 
 	if trData.IncludeSystemEnvVars {
 		tr.Spec.Arguments += " --include-system-env-vars"
@@ -237,6 +227,11 @@ func (w *PLZWorker) handle(testRunId string) {
 		w.logger.Error(err, fmt.Sprintf("Failed to retrieve test run data for `%s`", testRunId))
 		return
 	}
+	if err = trData.Build(); err != nil {
+		w.logger.Error(err, fmt.Sprintf("Failed to sort out test run data for `%s`", testRunId))
+		return
+	}
+
 	w.complete(tr, trData)
 
 	w.logger.Info(fmt.Sprintf("PLZ test run has been prepared with image `%s` and `%d` instances",

pkg/plz/worker_test.go

@@ -133,6 +133,9 @@ func Test_complete_correctDefinitionOfTestRun(t *testing.T) {
 			"ENV": "VALUE",
 			"foo": "bar",
 		}
+		someLZDistribution = cloud.LZDistribution{
+			"some-label": cloud.Distribution{LoadZone: "some-zone", Percent: 100},
+		}
 		// podTemplate test values
 		someAllowPrivEscalation       = false
 		someRunAsUser           int64 = 1000
@@ -199,7 +202,11 @@ func Test_complete_correctDefinitionOfTestRun(t *testing.T) {
 	cloudFieldsTestRun.Spec.Parallelism = int32(someInstances)
 
 	cloudEnvVarsTestRun = cloudFieldsTestRun // build up on top of cloud fields case
-	cloudEnvVarsTestRun.Spec.Arguments += " -e ENV=VALUE -e foo=bar"
+	cloudEnvVarsTestRun.Spec.Arguments = strings.Replace(cloudEnvVarsTestRun.Spec.Arguments,
+		`--block-hostnames=""  --no-thresholds`,
+		`--block-hostnames=""  --tag load_zone=some-label --no-thresholds`,
+		1)
+	cloudEnvVarsTestRun.Spec.Arguments += " -e ENV=VALUE -e K6_CLOUDRUN_DISTRIBUTION=some-label -e K6_CLOUDRUN_LOAD_ZONE=some-zone -e K6_CLOUDRUN_TEST_RUN_ID=6543 -e foo=bar"
 
 	podTemplateTolerationsTestRun = requiredFieldsTestRun
 	podTemplateTolerationsTestRun.Spec.Runner.Tolerations = someTolerations
@@ -321,7 +328,11 @@ func Test_complete_correctDefinitionOfTestRun(t *testing.T) {
 					InstanceCount: someInstances,
 					ArchiveURL:    someArchiveURL,
 					Environment:   someEnvVars,
+					CLIArgs: cloud.CLIArgs{
+						Tags: map[string]string{},
+					},
 				},
+				LZDistribution: someLZDistribution,
 			},
 			ingestUrl: mainIngest,
 			expected:  &cloudEnvVarsTestRun,
@@ -448,6 +459,7 @@ func Test_complete_correctDefinitionOfTestRun(t *testing.T) {
 			worker := NewPLZWorker(testCase.plz, "token", c, logr.Logger{})
 
 			tr := worker.template.Create()
+			_ = testCase.cloudData.Build()
 			worker.complete(tr, testCase.cloudData)
 
 			if diff := deep.Equal(tr, testCase.expected); diff != nil {