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.

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

Scuttle Logging cannot be disabled #179

Closed
cogren1 opened this issue Jan 17, 2023 · 4 comments
Closed

Scuttle Logging cannot be disabled #179

cogren1 opened this issue Jan 17, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@cogren1
Copy link

cogren1 commented Jan 17, 2023

Brief summary

Scuttle enables logging by default. This is problem because its logs end up in the initializer logs, causing the operator to be unable to unmarshal it into JSON. Adding scuttleLogging: false does not work though because the controller does not insert environment variables into the pod spec when their value evaluates to false. This applies to all boolean Scuttle parameters, but it's only a problem for scuttleLogging because default value in Scuttle is true.

Here is the offending line: https://github.com/grafana/k6-operator/blob/main/pkg/resources/jobs/helpers.go#L87

You can see that it checks for a value that evaluates to false instead of checking for the presence of the parameter. I've tested this with scuttleLogging and neverKillIstioOnFailure. When true, the environment variable is present. When false, they are not present. These, and other scuttle values, are defined as boolean in the CRD, which seems unnecessary since the values get converted to strings when inserted as pod environment variables and is one of the reasons this bug occurs.

k6-operator version or image

v0.0.9rc1, v0.0.8, v0.0.8rc3, v0.0.8rc2, v0.0.8rc1, v0.0.7, v0.0.7rc4, v0.0.7rc3, v0.0.7rc2, v0.0.7rc

K6 YAML

apiVersion: k6.io/v1alpha1
kind: K6
metadata:
  name: load-test
  namespace: k6-operator-system
spec:
  parallelism: 10
  runner:
    metadata:
      labels:
        sidecar.istio.io/inject: "true"
  scuttle:
    enabled: "true"
    envoyAdminApi: 'http://127.0.0.1:15000'
    istioQuitApi: 'http://127.0.0.1:15020'
    scuttleLogging: false
  script:
    configMap:
      name: test-cm
      file: test.js

Other environment details (if applicable)

Istio is installed.

Steps to reproduce the problem

On a cluster with Istio installed:

  1. Create a test config map.
  2. Create a K6 object with the following scuttle and runner settings:
      runner:
        metadata:
          labels:
            sidecar.istio.io/inject: "true"
      scuttle:
        enabled: "true"
        scuttleLogging: false

Expected behaviour

SCUTTLE_LOGGING appears in the initializer pod spec under env with value "false".

Actual behaviour

SCUTTLE_LOGGING does not appear in the initializer pod spec.

@yorugac
Copy link
Collaborator

yorugac commented Jan 20, 2023

Hi @cogren1, thank you for reporting this! I've added the fix for it here: #182.

I think this matter with logging was a bug since scutlle was added and just wasn't noticed before 😞 To avoid confusion with what this option means, I've renamed the setting itself to disableLogging -- that should be clear enough. This will be a breaking change for those who relied on scutlleLogging option (hopefully, not too many!) so I'll wait for feedback a bit before merging it.

@MerzMax
Copy link

MerzMax commented Mar 1, 2023

Is there any news about this issue? :) The pull requests seem to be stuck at the moment..

I would love to use k6 in combination with Istio!

@yorugac
Copy link
Collaborator

yorugac commented Mar 2, 2023

Hi @MerzMax, thanks for the ping! Yes actually, I think it can be merged now: nobody complained about breaking change so it should be fine 🙂 I'll merge it in now.

Btw, there might be other changes to Istio setup once we get to #195

@yorugac
Copy link
Collaborator

yorugac commented Mar 2, 2023

Fixed by #182

@yorugac yorugac closed this as completed Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants