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 --quiet and --no-info flags to podman machine start #16186

Merged
merged 1 commit into from
Nov 15, 2022

Conversation

ashley-cui
Copy link
Member

@ashley-cui ashley-cui commented Oct 14, 2022

Add quiet and no-info flags to podman machine start.
No-info suppresses helpful informational tips
Quiet suppresses machine start progress output, as well as informational
tips.
Signed-off-by: Ashley Cui [email protected]

Closes: #15525

Does this PR introduce a user-facing change?

Add quiet and no-info flags to podman machine start

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ashley-cui

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 14, 2022
@ashley-cui
Copy link
Member Author

Probably need someone to test this on Windows.

@Luap99
Copy link
Member

Luap99 commented Oct 17, 2022

Do we really need to flags for that? Also there is already a podman --noout flag

@ashley-cui
Copy link
Member Author

@Luap99 --noout does not suppress machine commands output. There was a community request for the behavior of the --no-info flag (#15525), and I thought I'd add the -q while I was at it.

@mheon
Copy link
Member

mheon commented Oct 17, 2022

Wait, --noout doesn't work? That should work for everything - if it doesn't we should be fixing --noout.

I don't object to adding --quiet or similar, but we also have to make sure --noout does not print anything.

@ashley-cui
Copy link
Member Author

@mheon Sure, I'll open an issue for fixing --noout for podman machine

@rhatdan
Copy link
Member

rhatdan commented Oct 17, 2022

Does that mean we no longer need this PR?

@ashley-cui
Copy link
Member Author

I'd argue the initial request (which is the --no-info flag in this pr) does something different than --noout since it only suppresses helpful tips and not start status, but I'm open to changing my mind

@rhatdan
Copy link
Member

rhatdan commented Oct 17, 2022

Wouldn't piping to grep also do that?

@ashley-cui
Copy link
Member Author

I don't think so, since we don't label info lines, they're just printed? So there's no pattern to match necessarily?

@rhatdan
Copy link
Member

rhatdan commented Oct 28, 2022

Can you remove the --quiet option since this seems to duplicate --noout and then make --noout work.

@ashley-cui ashley-cui changed the title Add --quiet and --no-info flags to podman machine start Add --no-info flag to podman machine start Oct 31, 2022
@vrothberg
Copy link
Member

friendly ping

@rhatdan
Copy link
Member

rhatdan commented Nov 9, 2022

LGTM
@vrothberg PTAL

@vrothberg
Copy link
Member

Apologies, I should have checked that before the ping. What's the reasoning for not using -q/--quiet? I don't see any flags yet for machine start and -q is used conventionally across so many tools.

@TomSweeneyRedHat
Copy link
Member

Changes LGTM, but like @vrothberg , I'd much prefer -q / --quiet unless there's a strong reason not to.

Add quiet and no-info flags to podman machine start.
No-info suppresses helpful informational tips
Quiet suppresses machine start progress output, as well as informational
tips.

Signed-off-by: Ashley Cui <[email protected]>
@ashley-cui ashley-cui changed the title Add --no-info flag to podman machine start Add --quiet and --no-info flags to podman machine start Nov 11, 2022
@ashley-cui
Copy link
Member Author

--quiet is back :)

@rhatdan
Copy link
Member

rhatdan commented Nov 12, 2022

My understanding would be people want --quiet to also mean --no-info, IE Drop --no-info.

@ashley-cui
Copy link
Member Author

I feel like --quiet means that it suppresses all status output, whereas no-info only surpasses troubleshooting tips. I feel like they serve different purposes.

@vrothberg
Copy link
Member

I feel like --quiet means that it suppresses all status output, whereas no-info only surpasses troubleshooting tips. I feel like they serve different purposes.

I think that's fair.

@rhatdan
Copy link
Member

rhatdan commented Nov 14, 2022

What value does --no-output serve which would not be serve by
$ podman machine ... |grep -v

IE Why complicate things. We should only output stuff that is important to the user, if the output is not important then it should be logrus.Info or Debug and not output at all.

If user does not want output the --quiet should suffice and they can check the exit code.

@ashley-cui
Copy link
Member Author

ashley-cui commented Nov 14, 2022

The troubleshooting tips are there because they ended up being common questions/configurations that pop up. I still feel like each flag have different purposes. Here are the differences:

No flags output:
This prints machine starting status as well as tips on common places where podman may fail.

$ podman machine start
Starting machine "podman-machine-default"
Waiting for VM ...
Mounting volume... /Users/pellcorp:/Users/pellcorp

This machine is currently configured in rootless mode. If your containers
require root permissions (e.g. ports < 1024), or if you run into compatibility
issues with non-podman clients, you can switch using the following command: 

	podman machine set --rootful

API forwarding listening on: /Users/pellcorp/.local/share/containers/podman/machine/podman-machine-default/podman.sock

The system helper service is not installed; the default Docker API socket
address can't be used by podman. If you would like to install it run the
following commands:

	sudo /opt/homebrew/Cellar/podman/4.2.0/bin/podman-mac-helper install
	podman machine stop; podman machine start

You can still connect Docker API clients by setting DOCKER_HOST using the
following command in your terminal session:

	export DOCKER_HOST='unix:///Users/pellcorp/.local/share/containers/podman/machine/podman-machine-default/podman.sock'

Machine "podman-machine-default" started successfully

No info flag (this was the specfic behavior asked for in the issue)
This still prints status updates on the machine but does not remind the user about other configs

$ podman machine --no-info start
Starting machine "podman-machine-default"
Waiting for VM ...
Mounting volume... /Users/pellcorp:/Users/pellcorp
Machine "podman-machine-default" started successfully

Quiet:

$ podman machine -q start
Machine "podman-machine-default" started successfully

--noout

$ podman --noout machine  start

I still feel like the suppressed information on each (troubleshooting vs start status) is unique enough to warrant two flags.

@vrothberg
Copy link
Member

I agree with @ashley-cui in this case. Exceptions confirm the rule :^)

@rhatdan
Copy link
Member

rhatdan commented Nov 15, 2022

I would argue that:

Starting machine "podman-machine-default"
Waiting for VM ...
Mounting volume... /Users/pellcorp:/Users/pellcorp
Machine "podman-machine-default" started successfully

Is info.
But fine, if you two want this, fine.

@vrothberg
Copy link
Member

I agree with both of you :-) Not sure that helps. How about you bring this up during Cabal? I won´t be able to join but you already know I like everything.

@ashley-cui
Copy link
Member Author

Ready to go! :)

@rhatdan
Copy link
Member

rhatdan commented Nov 15, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 15, 2022
@openshift-merge-robot openshift-merge-robot merged commit 7cd3bae into containers:main Nov 15, 2022
@ashley-cui ashley-cui deleted the shh branch February 9, 2023 20:09
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 10, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Anyway to suppress warnings on Mac when running podman machine start
7 participants