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

Remove command descriptions in case the whole application is failing #4290

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gkrenn
Copy link
Contributor

@gkrenn gkrenn commented Jan 14, 2025

Description

Most of the operator subcommands are invoked by hardcoded pod commands. When these commands fail, a usage description is printed. This is unnecessary and causes confusion in the logs, so the print statements should be removed for subcommands that are not directly called by the user.

Example of confusing log:

{"level":"info","ts":"2024-10-09T13:16:50.592Z","logger":"injection-startup","msg":"CONTAINER_INFO environment variable is missing"}
Usage:
  dynatrace-operator init [flags]

Flags:
  -h, --help   help for init

Jira

How can this be tested?

Make the init container fail with an error, it should only log the error.

@gkrenn gkrenn added the core Changes to core functionality of the Operator label Jan 14, 2025
@gkrenn gkrenn requested a review from a team as a code owner January 14, 2025 14:52
@gkrenn gkrenn force-pushed the feature/better-container-fail-msg branch from eb9d95f to 012156e Compare January 14, 2025 14:52
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.

Project coverage is 64.28%. Comparing base (67c2fad) to head (012156e).

Files with missing lines Patch % Lines
cmd/csi/init/builder.go 0.00% 3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4290   +/-   ##
=======================================
  Coverage   64.28%   64.28%           
=======================================
  Files         402      402           
  Lines       26944    26950    +6     
=======================================
+ Hits        17321    17326    +5     
- Misses       8256     8257    +1     
  Partials     1367     1367           
Flag Coverage Δ
unittests 64.28% <83.33%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andriisoldatenko
Copy link
Contributor

tested and it works as expected.

@andriisoldatenko
Copy link
Contributor

Just quick note: toplevel command will still print --help in case of error (I think it's exepected and waht we want:

./cmd
{"level":"info","ts":"2025-01-14T20:01:53.192+0100","logger":"install-config","msg":"envvar not set, using default","envvar":"modules.json"}
{"level":"info","ts":"2025-01-14T20:01:53.192+0100","logger":"install-config","msg":"problem unmarshalling envvar content, using default","envvar":"modules.json","err":"unexpected end of JSON input"}
{"level":"info","ts":"2025-01-14T20:01:53.192+0100","logger":"install-config","msg":"envvar content read and set","envvar":"modules.json","value":""}
Error: operator binary must be called with one of the subcommands
Usage:
  dynatrace-operator [flags]
  dynatrace-operator [command]

Available Commands:
  completion      Generate the autocompletion script for the specified shell
  csi-init
  csi-provisioner
  csi-server
  help            Help about any command
  init
  operator
  startup-probe
  support-archive
  troubleshoot
  webhook-server

Flags:
  -h, --help   help for dynatrace-operator

Use "dynatrace-operator [command] --help" for more information about a command.

{"level":"info","ts":"2025-01-14T20:01:53.193+0100","logger":"main","msg":"operator binary must be called with one of the subcommands"}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Changes to core functionality of the Operator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants