From 24d1922da1d2be9541d12c41d074387d6c2e1148 Mon Sep 17 00:00:00 2001 From: Christian Rohmann Date: Thu, 22 Aug 2024 09:53:20 +0200 Subject: [PATCH] Use exit codes (of k6 archive + inspect) as sole indication for initialization success 1) Introduce an EmptyDir volume for temporary data This removes the need for any mkdir and avoids writing to the container filesystem, allowing for e.g. `readOnlyRootFilesystem` security contexts ([1]). 2) Remove all output redirection and grepping for error indicating strings in favor of using the exit codes from the commands to indicate success or failure. This approach is much cleaner in regards to any changes to the output of K6 and the vast space of error there is. It also reestablishes the clear contract of the `inspect` command to judge the provided config. `k6 inspect --execution-requirements` ([2,3]) while also printing warnings or the JSON should clearly indicate via `err` if there are issues with the provided tests, which are then converted to a non-zero RC ([4]). [1] https://kubernetes.io/docs/tasks/configure-pod-container/security-context/ [2] https://github.com/grafana/k6/blob/c212bd277aeedbf8395b917df11bd9be4da490a4/cmd/inspect.go#L34 [3] https://github.com/grafana/k6/blob/c212bd277aeedbf8395b917df11bd9be4da490a4/cmd/inspect.go#L64 [4] https://github.com/grafana/k6/blob/master/cmd/root.go#L90-L118 Fixes: #435 --- pkg/resources/jobs/initializer.go | 34 ++++++++++++-------------- pkg/resources/jobs/initializer_test.go | 22 +++++++++++++++-- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/pkg/resources/jobs/initializer.go b/pkg/resources/jobs/initializer.go index 64bb2de4..f93d5028 100644 --- a/pkg/resources/jobs/initializer.go +++ b/pkg/resources/jobs/initializer.go @@ -61,31 +61,29 @@ func NewInitializerJob(k6 v1alpha1.TestRunI, argLine string) (*batchv1.Job, erro ) istioCommand, istioEnabled := newIstioCommand(k6.GetSpec().Scuttle.Enabled, []string{"sh", "-c"}) command := append(istioCommand, fmt.Sprintf( - // There can be several scenarios from k6 command here: - // a) script is correct and `k6 inspect` outputs JSON - // b) script is partially incorrect and `k6` outputs a warning log message and - // then a JSON - // c) script is incorrect and `k6` outputs an error log message - // Warnings at this point are not necessary (warning messages will re-appear in - // runner's logs and the user can see them there) so we need a pure JSON here - // without any additional messages in cases a) and b). In case c), output should - // contain error message and the Job is to exit with non-zero code. - // - // Due to some pecularities of k6 logging, to achieve the above behaviour, - // we need to use a workaround to store all log messages in temp file while - // printing JSON as usual. Then parse temp file only for errors, ignoring - // any other log messages. - // Related: https://github.com/grafana/k6-docs/issues/877 - "mkdir -p $(dirname %s) && k6 archive %s -O %s %s 2> /tmp/k6logs && k6 inspect --execution-requirements %s 2> /tmp/k6logs ; ! cat /tmp/k6logs | grep 'level=error'", - archiveName, scriptName, archiveName, argLine, - archiveName)) + "k6 archive %s -O %s %s && k6 inspect --execution-requirements %s", + scriptName, archiveName, argLine, archiveName)) env := append(newIstioEnvVar(k6.GetSpec().Scuttle, istioEnabled), k6.GetSpec().Initializer.Env...) volumes := script.Volume() + // emptyDir to hold our temporary data + tmpVolume := corev1.Volume{ + Name: "tmpdir", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + } + volumes = append(volumes, tmpVolume) volumes = append(volumes, k6.GetSpec().Initializer.Volumes...) volumeMounts := script.VolumeMount() + // make /tmp an EmptyDir + tmpVolumeMount := corev1.VolumeMount{ + Name: "tmpdir", + MountPath: "/tmp", + } + volumeMounts = append(volumeMounts, tmpVolumeMount) volumeMounts = append(volumeMounts, k6.GetSpec().Initializer.VolumeMounts...) var zero32 int32 diff --git a/pkg/resources/jobs/initializer_test.go b/pkg/resources/jobs/initializer_test.go index 868d13ef..ec03c39a 100644 --- a/pkg/resources/jobs/initializer_test.go +++ b/pkg/resources/jobs/initializer_test.go @@ -21,6 +21,24 @@ func TestNewInitializerJob(t *testing.T) { automountServiceAccountToken := true zero := int32(0) + volumes := script.Volume() + // emptyDir to hold our temporary data + tmpVolume := corev1.Volume{ + Name: "tmpdir", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + } + volumes = append(volumes, tmpVolume) + + volumeMounts := script.VolumeMount() + // make /tmp an EmptyDir + tmpVolumeMount := corev1.VolumeMount{ + Name: "tmpdir", + MountPath: "/tmp", + } + volumeMounts = append(volumeMounts, tmpVolumeMount) + expectedOutcome := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ Name: "test-initializer", @@ -63,7 +81,7 @@ func TestNewInitializerJob(t *testing.T) { Name: "k6", Command: []string{ "sh", "-c", - "mkdir -p $(dirname /tmp/test.js.archived.tar) && k6 archive /test/test.js -O /tmp/test.js.archived.tar --out cloud 2> /tmp/k6logs && k6 inspect --execution-requirements /tmp/test.js.archived.tar 2> /tmp/k6logs ; ! cat /tmp/k6logs | grep 'level=error'", + "k6 archive /test/test.js -O /tmp/test.js.archived.tar --out cloud && k6 inspect --execution-requirements /tmp/test.js.archived.tar", }, Env: []corev1.EnvVar{}, EnvFrom: []corev1.EnvFromSource{ @@ -81,7 +99,7 @@ func TestNewInitializerJob(t *testing.T) { SecurityContext: &corev1.SecurityContext{}, }, }, - Volumes: script.Volume(), + Volumes: volumes, }, }, },