Skip to content
This repository was archived by the owner on Feb 24, 2020. It is now read-only.

Conversation

@lucab
Copy link
Member

@lucab lucab commented Oct 20, 2016

This PR turns off by default all image and networking diagnostic output, moving it behind explicit --debug flag. This turns an error-free rkt execution from this:

$ sudo rkt run quay.io/coreos/alpine-sh --exec echo -- "hello world!"
image: using image from local store for image name coreos.com/rkt/stage1-coreos:1.17.0
image: using image from local store for image name quay.io/coreos/alpine-sh
networking: loading networks from /etc/rkt/net.d
networking: loading network default with type ptp
[18489.453547] alpine-sh[5]: hello world!

into this:

$ sudo ./rkt run  quay.io/coreos/alpine-sh --exec echo -- "hello world!"
[18521.078185] alpine-sh[5]: hello world!

This makes rkt strictly following the UNIX rule of silence, but in the long term we need a better way of handling level/structured logging across all packages and more granular verbosity-switches for CLI (--verbose=<level>). A complete switch to logrus is being considered, but out of scope here.

Fixes #2622

@jonboulle
Copy link
Contributor

Pushing back a little on silence-as-default: are we concerned about the UX of rkt "hanging" when it's not outputting anything while fetching images etc?

@lucab
Copy link
Member Author

lucab commented Oct 20, 2016

@jonboulle in general yes, this concern exists, which is why we'd better have some verbosity switch at some point. Another concern is that we vendor libraries out of our control, which is way rkt prepare --quiet internally masks stdout fd.

For the specific network fetching case, some bits of image&networking are left untouched here, so for example:

$ sudo run --no-store  quay.io/coreos/alpine-sh --exec echo -- "hello world!"
image: keys already exist for prefix "quay.io/coreos/alpine-sh", not fetching again
Downloading signature: [=======================================] 473 B/473 B
Downloading ACI: [=============================================] 2.65 MB/2.65 MB
image: signature verified:
  Quay.io ACI Converter (ACI conversion signing key) <[email protected]>
[19627.613248] alpine-sh[5]: hello world!

@lucab
Copy link
Member Author

lucab commented Oct 24, 2016

@s-urbaniak @jonboulle ping for a review. I think Jon was mostly concerned about fetching progress, but that shouldn't have changed.

Copy link
Contributor

@s-urbaniak s-urbaniak left a comment

Choose a reason for hiding this comment

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

LGTM

@s-urbaniak
Copy link
Contributor

As discussed oob ideally we would like to inject the lighter, but that is our of scope for this pr.

@s-urbaniak
Copy link
Contributor

haha, autocorrection hit me ;)

s/lighter/logger

@lucab lucab merged commit 0d4c0fe into rkt:master Oct 24, 2016
@jonboulle
Copy link
Contributor

lgtm

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants