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

Configurable log level/format for gardener-resource-manager #6830

Merged
merged 3 commits into from
Oct 14, 2022

Conversation

oliver-goetz
Copy link
Member

@oliver-goetz oliver-goetz commented Oct 13, 2022

How to categorize this PR?

/area control-plane
/kind enhancement

What this PR does / why we need it:
This PR creates cli parameters --log-level=<debug/info/error> and --log-format=<json/text> in gardener-resource-manager and enables their configuration in gardener-control-plane helm chart and botanist package.
The later does not include a dedicated configuration for gardener-resource-manager and this PR does not create one for this purpose only. This is the log-level/format configurations:

When running on Seed clusters:
log-level and log-format settings of gardenlet are used for gardener-controller-manager too

When running on Shoot clusters:
There are default values only --log-level=info --log-format=json

In both scenarios different log-levels/format could be set by changing their deployments while ignoring the corresponding managed resource during reconciliation.

Which issue(s) this PR fixes:
Part of #5191

Special notes for your reviewer:
This PR adds a verification of log-levels and log-format for gardener-apiserver which was forgotten in #6817

Release note:

log-level and log-format of gardener-resource-manager can now be configured.

@gardener-prow gardener-prow bot added area/control-plane Control plane related kind/enhancement Enhancement, improvement, extension cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. labels Oct 13, 2022
@gardener-prow gardener-prow bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 13, 2022
@gardener-prow
Copy link
Contributor

gardener-prow bot commented Oct 14, 2022

@oliver-goetz: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-gardener-check-vulnerabilities 20d0930 link false /test pull-gardener-check-vulnerabilities
pull-gardener-e2e-kind-ha-single-zone 20d0930 link false /test pull-gardener-e2e-kind-ha-single-zone
pull-gardener-e2e-kind-ha-multi-zone 20d0930 link false /test pull-gardener-e2e-kind-ha-multi-zone
pull-dependency-watchdog-verify-image-build 20d0930 link true /test pull-dependency-watchdog-verify-image-build

Full PR test history. Your PR dashboard. Command help for this repository.
Please help us cut down on flakes by linking this test failure to an open flake report or filing a new flake report if you can't find an existing one. Also see our testing guideline for how to avoid and hunt flakes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@rfranzke
Copy link
Member

Same here for GRM

2022-10-13T18:13:19.465917487Z stderr F Error: unknown flag: --log-level
2022-10-13T18:13:19.466001249Z stderr F {"level":"error","ts":"2022-10-13T18:13:19.465Z","msg":"Error executing the main controller command","error":"unknown flag: --log-level","stacktrace":"main.main\n\t/go/src/github.com/gardener/gardener/cmd/gardener-resource-manager/main.go:35\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:250"}

@rfranzke rfranzke self-assigned this Oct 14, 2022
@oliver-goetz
Copy link
Member Author

This seems to be an issue with the HA e2e tests. They do not use the local built gardener-core images, but the latest dev version from eu.gcr.io

@oliver-goetz
Copy link
Member Author

Ahhh I see, the image vector overwrites are not set for HA e2e tests in skaffold.yaml
I will fix this in a separate PR.

@rfranzke
Copy link
Member

Awesome, thanks @oliver-goetz for checking!

@rfranzke rfranzke changed the title Configurable log level/format for gardener-resource-mananger Configurable log level/format for gardener-resource-manager Oct 14, 2022
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2022
@gardener-prow
Copy link
Contributor

gardener-prow bot commented Oct 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rfranzke

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 14, 2022
@gardener-prow gardener-prow bot merged commit 6717f39 into gardener:master Oct 14, 2022
@oliver-goetz oliver-goetz deleted the enh/resource-manager-log branch April 14, 2023 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/control-plane Control plane related cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants