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

libnetwork/pasta: always pass --quiet #1868

Merged
merged 1 commit into from
Feb 25, 2024

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Feb 22, 2024

We want to log pasta warnings/errors even if pasta did not fail to start, however by default it prints a bit to noisy for us. With --quiet we get the things users should care about.

Note right now --quiet defaults to errors only but it should also do warnings based on the discussion with Stefano[1]. However this change is still safe to do right now.

[1] https://archives.passt.top/passt-dev/20240216114304.7234a83f@elisabeth/T/#m42652824644973674e84baf9e0bf1d0e88104450

Copy link
Contributor

openshift-ci bot commented Feb 22, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99

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

@Luap99
Copy link
Member Author

Luap99 commented Feb 22, 2024

@mheon PTAL
cc @sbrivio-rh

@Luap99 Luap99 added the 5.0 label Feb 22, 2024
@Luap99
Copy link
Member Author

Luap99 commented Feb 22, 2024

/hold
Please do not merge, I am testing this right now but there is a problem with it.

@sbrivio-rh
Copy link
Collaborator

I thought you'd just pass --quiet, and we'd fix --quiet to log messages up to LOG_WARN (instead of LOG_ERR), and for the rest you'd leave everything as it is, instead of catching stdout.

@Luap99
Copy link
Member Author

Luap99 commented Feb 22, 2024

I thought you'd just pass --quiet, and we'd fix --quiet to log messages up to LOG_WARN (instead of LOG_ERR), and for the rest you'd leave everything as it is, instead of catching stdout.

Yes that is my goal but do you write anything do stdout? I don't think so and IMO it is easier that way to code here. I can of course make the code only read stderr if that makes a real difference.

ps: I am in the process to send a patch to fix pasta.

@sbrivio-rh
Copy link
Collaborator

I thought you'd just pass --quiet, and we'd fix --quiet to log messages up to LOG_WARN (instead of LOG_ERR), and for the rest you'd leave everything as it is, instead of catching stdout.

Yes that is my goal but do you write anything do stdout?

No, just for -h / --help (https://bugs.passt.top/show_bug.cgi?id=52) and --version.

I don't think so and IMO it is easier that way to code here.

Sure.

I can of course make the code only read stderr if that makes a real difference.

No no, it shouldn't -- I was just wondering.

ps: I am in the process to send a patch to fix pasta.

Oh, thank you.

@mheon
Copy link
Member

mheon commented Feb 22, 2024

LGTM

We want to log pasta warnings/errors even if pasta did not fail to
start, however by default it prints a bit to noisy for us. With --quiet
we get the things users should care about.

Unfortunately it still prints to much right now, I will work on some
pasta changes so we can bump it up later.

Note right now --quiet defaults to errors only but it should also do
warnings based on the discussion with Stefano[1]. However this change is
still safe to do right now.

[1] https://archives.passt.top/passt-dev/20240216114304.7234a83f@elisabeth/T/#m42652824644973674e84baf9e0bf1d0e88104450

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
@Luap99
Copy link
Member Author

Luap99 commented Feb 22, 2024

OK this should be good right now, logging this on warning right now is not possible as it is still to noisy.
For the first problem I posted https://archives.passt.top/passt-dev/20240222171739.141623-2-pholzing@redhat.com/
but I also see a dns warning Couldn't get any nameserver address which I need to investigate further.

@rhatdan
Copy link
Member

rhatdan commented Feb 22, 2024

/lgtm
/hold

@rhatdan
Copy link
Member

rhatdan commented Feb 25, 2024

/unhold

@openshift-merge-bot openshift-merge-bot bot merged commit 91fc964 into containers:main Feb 25, 2024
7 checks passed
@Luap99 Luap99 deleted the pasta-log branch February 25, 2024 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants