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

Make fluentd log level configurable #196

Merged
merged 2 commits into from
Apr 12, 2021

Conversation

kkapoor1987
Copy link
Contributor

@kkapoor1987 kkapoor1987 commented Apr 12, 2021

Solves #181

  • Tested by changing to different log level which fluentd supports
  • If loglevel provided is not supported then we fail with validation error.
  • If log level is not provided then default logLevel info is used

Signed-off-by: Karan Kapoor karan.kapoor@pega.com

@vmwclabot
Copy link
Member

@kkapoor1987, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

Signed-off-by: Karan Kapoor karan.kapoor@pega.com

Signed-off-by: Karan Kapoor <karan.kapoor@pega.com>
@kkapoor1987
Copy link
Contributor Author

@Cryptophobia can you review this

@Cryptophobia Cryptophobia self-requested a review April 12, 2021 15:50
Copy link
Contributor

@Cryptophobia Cryptophobia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, looks good. Only a minor mistake in the camel case and we need to add the same env var in the values.yaml file.

charts/log-router/templates/daemonset.yaml Outdated Show resolved Hide resolved
config-reloader/templates/fluent.conf Show resolved Hide resolved
@Cryptophobia
Copy link
Contributor

@kkapoor1987 , can you also add it to the env vars in the docs here: https://github.com/vmware/kube-fluentd-operator#synopsis with a small description. Also, make sure to say --log-level is for config-reloader and --fluentd-loglevel is for fluentd container log level.

@vmwclabot
Copy link
Member

@kkapoor1987, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

@vmwclabot
Copy link
Member

@kkapoor1987, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

@kkapoor1987
Copy link
Contributor Author

@kkapoor1987 , can you also add it to the env vars in the docs here: https://github.com/vmware/kube-fluentd-operator#synopsis with a small description. Also, make sure to say --log-level is for config-reloader and --fluentd-loglevel is for fluentd container log level.

Updated Readme with correct description

Signed-off-by: Karan Kapoor <karan.kapoor@pega.com>
Copy link
Contributor

@Cryptophobia Cryptophobia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@Cryptophobia
Copy link
Contributor

Thank you for the contribution @kkapoor1987 ! Another release will be coming soon.

@Cryptophobia Cryptophobia merged commit 23ef050 into vmware:master Apr 12, 2021
@kkapoor1987 kkapoor1987 deleted the feature/configureloglevel branch April 13, 2021 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants