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

Add a flag -o/--output to kubermatic-installer to allow json output #10137

Merged
merged 2 commits into from Jun 22, 2022

Conversation

embik
Copy link
Member

@embik embik commented Jun 21, 2022

What does this PR do / Why do we need it:
Not sure if we want to merge this, but I thought it would be neat to have the option for json log formatting in kubermatic-installer. The default of course stays with the nice formatter that logrus uses for human readability (filled under "console" in this new flag), but an --output (or shorthand -o) flag will allow to get json output if that is wanted (I can see this being useful with the upcoming mirror-images subcommand, for example).

This is what it would look like, as an example for an early failure on deploying:

$ ./_build/kubermatic-installer deploy
INFO[0000] 🚀 Initializing installer…                     edition="Enterprise Edition" version=2a9ece9131272f12e01b6331769fdd301239230d
ERRO[0000] ❌ Operation failed: no kubeconfig (--kubeconfig or $KUBECONFIG) given.

$  ./_build/kubermatic-installer deploy -o json
{"edition":1,"level":"info","msg":"🚀 Initializing installer…","time":"2022-06-21T17:00:30+02:00","version":"2a9ece9131272f12e01b6331769fdd301239230d"}
{"level":"error","msg":"❌ Operation failed: no kubeconfig (--kubeconfig or $KUBECONFIG) given.","time":"2022-06-21T17:00:30+02:00"}

WDYT?

Does this PR close any issues?:
Fixes #

Special notes for your reviewer:

Documentation:

Does this PR introduce a user-facing change?:

`kubermatic-installer` output can be formatted as json via a new `--output` flag

Signed-off-by: Marvin Beckers marvin@kubermatic.com

Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
@embik embik requested a review from xrstf June 21, 2022 15:07
@kubermatic-bot kubermatic-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. approved Indicates a PR has been approved by an approver from all required OWNERS files. sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 21, 2022
@embik
Copy link
Member Author

embik commented Jun 21, 2022

/retest

Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
@xrstf
Copy link
Contributor

xrstf commented Jun 21, 2022

Not sure what's so neat about this in this case, but I only have minor things to mention:

  • In all other apps, we use -log-format, should we use the same here instead of -o? I don't mind -o, just wondering.
  • The log messages do have indentations, so we would end up with something like "msg:" foo". Can you live with that? (I tried sooo hard to somehow automate the indentation somehow, as if it was structured logging, but manually indenting all lines was much easier, hence the many empty spaces).

@embik
Copy link
Member Author

embik commented Jun 22, 2022

In all other apps, we use -log-format, should we use the same here instead of -o? I don't mind -o, just wondering.

I was thinking about that, but since we're already doing different logging in the installer, I thought offering something that looks more like other Kubernetes client applications was better. I don't have strong opinions about that though, I can change it if you prefer it to be consistent.

The log messages do have indentations, so we would end up with something like "msg:" foo". Can you live with that? (I tried sooo hard to somehow automate the indentation somehow, as if it was structured logging, but manually indenting all lines was much easier, hence the many empty spaces).

I don't think that's an issue 🤷🏻

@xrstf
Copy link
Contributor

xrstf commented Jun 22, 2022

/approve
/lgtm

I wish we had telemetry to tell us how many users discovered and then used the JSON output in the future :D

@kubermatic-bot kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 22, 2022
@kubermatic-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: f3d82d414ce7700c7a978a72f8f108cb5796dd9b

@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: embik, xrstf

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

@embik
Copy link
Member Author

embik commented Jun 22, 2022

/retest

@kubermatic-triage-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs

Review the full test history

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

1 similar comment
@kubermatic-triage-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs

Review the full test history

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubermatic-bot kubermatic-bot merged commit 0e42d4c into kubermatic:master Jun 22, 2022
@embik embik deleted the installer-json-logging branch June 22, 2022 18:56
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. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants