Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rework Initializer container and handling its verdict #450

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Rework Initializer container and handling its verdict #450

wants to merge 3 commits into from

Commits on Aug 22, 2024

  1. Use exit codes (of k6 archive + inspect) as sole indication for initi… 

    …alization 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: grafana#435
    @frittentheke
    frittentheke committed Aug 22, 2024
    Configuration menu
    Copy the full SHA
    24d1922 View commit details
    Browse the repository at this point in the history

  2. Set TerminationMessagePolicy on Initializer to use logs as fallback 

    While not explicitly used, with the removal of the log fetching from the
Initializer (Job->Pod->Container) any potential source for verbose information
about the error might be lost. The Kubernetes approach to provide more details
than the exit code about the reason a container terminated is the termination
message (see [1]).

Per default this message needs to be explictly written to `/dev/termination-log`,
but there also is the option to use the last bit of log output as fallback source.

[1] https://kubernetes.io/docs/tasks/debug/debug-application/determine-reason-pod-failure/
    @frittentheke
    frittentheke committed Aug 22, 2024
    Configuration menu
    Copy the full SHA
    2868535 View commit details
    Browse the repository at this point in the history

  3. Remove fetching of container logs from initializer 

    Following the focus on the exit code of the command using in the initializer job (Pod),
this commit removes the fetching of the container log.

There was only a basic JSON unmarshalling applied with no interpretion of what it contained.
This is either covered by `k6 inspect` exiting with rc=0 or should be added to the initializer
job.

If further details about the failure reason of the initalizer container was to be required,
the source should be the termination message. It could be used to enrich the TestRun CR to provide
a higher level of detail about the failure to the user.

[1] https://kubernetes.io/docs/tasks/debug/debug-application/determine-reason-pod-failure/#customizing-the-termination-message
    @frittentheke
    frittentheke committed Aug 22, 2024
    Configuration menu
    Copy the full SHA
    1fd1c56 View commit details
    Browse the repository at this point in the history