Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Add ability to set log level and enable json logging for all consul-k8s commands #980

Merged
merged 17 commits into from
Jun 30, 2021

Conversation

kschoche
Copy link
Contributor

@kschoche kschoche commented Jun 7, 2021

Changes proposed in this PR:

  • Add ability to set json logging for consul-k8s components : global.logJSON
  • Add ability to control log level for all consul-k8s components global.logLevel

NOTES:

  • Envoy + consul server/client are not currently implemented.

How I've tested this PR:
unit tests, acceptance tests pass

How I expect reviewers to test this PR:
code review, acceptance tests pass
Also have manually testing using steps in hashicorp/consul-k8s#523

Checklist:

  • Bats tests added
  • CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)

@kschoche kschoche self-assigned this Jun 7, 2021
@kschoche kschoche changed the title Add json logging ability and specify log-level to all consul-k8s commands [draft] Add ability to set log level and enable json logging for all consul-k8s commands Jun 22, 2021
@kschoche kschoche marked this pull request as ready for review June 22, 2021 18:30
@kschoche kschoche requested review from a team, lkysow and ndhanushkodi and removed request for a team June 22, 2021 22:48
.circleci/config.yml Outdated Show resolved Hide resolved
@kschoche kschoche added the enhancement New feature or request label Jun 22, 2021
Copy link
Member

@lkysow lkysow left a comment

Choose a reason for hiding this comment

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

Looks god. One question about the move to sh -ec

templates/connect-inject-deployment.yaml Outdated Show resolved Hide resolved
templates/controller-deployment.yaml Show resolved Hide resolved
templates/create-federation-secret-job.yaml Outdated Show resolved Hide resolved
templates/ingress-gateways-deployment.yaml Outdated Show resolved Hide resolved
templates/ingress-gateways-deployment.yaml Outdated Show resolved Hide resolved
values.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@ndhanushkodi ndhanushkodi left a comment

Choose a reason for hiding this comment

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

For a command like consul-sidecar for connect-injected pods that doesn't have its logLevel/logJSON set here, do you need to add something to the consul-k8s PR to invoke that internal command with the right flag based on the global value?

Copy link
Contributor

@ndhanushkodi ndhanushkodi left a comment

Choose a reason for hiding this comment

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

Nice work!!

@lkysow
Copy link
Member

lkysow commented Jun 29, 2021

Should we have a follow-up PR that sets the log level/json for Consul clients/servers too?

@kschoche
Copy link
Contributor Author

Should we have a follow-up PR that sets the log level/json for Consul clients/servers too?

Yup this was only for subcommands.

@kschoche kschoche merged commit 5ae418b into master Jun 30, 2021
@kschoche kschoche deleted the logging-updates-json branch June 30, 2021 01:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants