Skip to content

pod logs enhancements: option to color logs#13490

Merged
openshift-merge-robot merged 1 commit intocontainers:mainfrom
gcalin:13266
Apr 4, 2022
Merged

pod logs enhancements: option to color logs#13490
openshift-merge-robot merged 1 commit intocontainers:mainfrom
gcalin:13266

Conversation

@kbaran1998
Copy link

@kbaran1998 kbaran1998 commented Mar 11, 2022

Created an option to colourise pod logs with an option --color. You can recreate with the following steps:

  1. Create a pod with containers
bin/podman pod create --name=pod_testlogs; bin/podman run --name=testlogs_loop1_1 -d --pod=pod_testlogs busybox /bin/sh -c 'for i in `seq 1 10000`; do echo "loop1: $i"; sleep 1; done'; bin/podman run --name=testlogs_loop2_1 -d --pod=pod_testlogs busybox /bin/sh -c 'for i in `seq 1 10000`; do echo "loop2: $i"; sleep 3; done';
  1. Logs with colour:
bin/podman pod logs --tail=10 -f --color pod_testlogs
  1. Kill all pods and remove containers:
bin/podman kill --all; bin/podman rm --all; bin/podman pod rm --all

Closes #13266

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 11, 2022
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 11, 2022
Copy link
Member

Choose a reason for hiding this comment

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

remove extra blank lines.

Copy link
Member

Choose a reason for hiding this comment

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

remove extra blank lines.

Copy link
Member

Choose a reason for hiding this comment

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

Obove the option is --colors?

Copy link
Member

Choose a reason for hiding this comment

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

Why plural? I think just --color would be even better.

Copy link
Member

Choose a reason for hiding this comment

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

SGTM

Copy link
Member

Choose a reason for hiding this comment

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

could we make it a string so in future we can extend it if needed?

e.g. GNU diff and ls have --color={never,always,auto} where --color=auto detects if the stdout is a tty before turning on colors.

Copy link
Member

Choose a reason for hiding this comment

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

We can also extend a bool in the future, it is easy to change it to a string while still keeping the --color only syntax.

Copy link
Member

Choose a reason for hiding this comment

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

Just use Colors and option.Colors

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 13, 2022
@rhatdan
Copy link
Member

rhatdan commented Mar 13, 2022

Please rebase and force push your commit. There should only be one commit.

@openshift-ci openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 16, 2022
@kbaran1998 kbaran1998 requested a review from rhatdan March 16, 2022 15:28
@rhatdan
Copy link
Member

rhatdan commented Mar 17, 2022

Your PR is still includes many old PRs please rebase and only submit your changes.

@keonchennl keonchennl force-pushed the 13266 branch 9 times, most recently from c5e16ee to 9a523ab Compare March 18, 2022 16:35
@keonchennl keonchennl force-pushed the 13266 branch 2 times, most recently from f2689ec to c62587e Compare March 18, 2022 16:55
@keonchennl keonchennl force-pushed the 13266 branch 2 times, most recently from 5b4c683 to 283fd60 Compare March 18, 2022 17:15
@openshift-ci openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 18, 2022
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 24, 2022
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 24, 2022
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 24, 2022
@kbaran1998 kbaran1998 force-pushed the 13266 branch 5 times, most recently from 80d9a4d to a54a783 Compare March 24, 2022 10:38
@openshift-ci openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 24, 2022
@openshift-ci openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 24, 2022
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 24, 2022
@rhatdan
Copy link
Member

rhatdan commented Mar 24, 2022

You need to do something like

git pull origin main
git rebase -i origin
Remove all of the code in the rebase that you did not add and squash all of your commits into a single PR
then do a
git push --force

@kbaran1998
Copy link
Author

kbaran1998 commented Mar 24, 2022

@rhatdan Whenever I squash only what I added into 1 commit, I get the 884 changes and whenever I rebase, I get back 7 commits...

And if I follow instructions in DCO from the line:
To add your Signed-off-by line to every commit in this branch:

I will get back to 7 commits and 13 changes.

I seem to be going in a loop.

@mheon
Copy link
Member

mheon commented Mar 24, 2022

I recommend git fetch to fetch the changes, and then a git rebase upstream/main (where upstream points to this repo, not your fork) to apply your changes on top of latest main

@rhatdan
Copy link
Member

rhatdan commented Mar 24, 2022

Do
git pull upstream main
And then
git rebase -i origin
And see if that clears it up

@rhatdan
Copy link
Member

rhatdan commented Mar 25, 2022

Almost there.

Now cleanup your commit message. removing the excess text.

git commit -a --amend -s
git push --force

Should do it, and then the DCO should pass.

Copy link
Member

Choose a reason for hiding this comment

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

nit

Suggested change
podman container logs --color --names ctrID1 ctrID2
podman container logs --color --names ctrID1 ctrID2

@kbaran1998
Copy link
Author

kbaran1998 commented Mar 28, 2022

For some reason, a test in test/e2e/run_test.go called podman run user capabilities test (line 467) is failing on the pipeline and locally, and I am not sure why. Specifically, it is giving me this output:

time="2022-03-28T20:30:41+02:00" level=warning msg="\"/\" is not a shared mount, this could cause issues or missing mounts with rootless containers"
CapBnd: 00000000a80425fb
CapEff: 0000000000000000
CapInh: 0000000000000000
CapBnd: 00000000a80425fb
CapEff: 00000000a80425fb
CapInh: 00000000a80425fb
CapBnd: 00000000a80425fb
CapEff: 00000000a80425fb
CapAmb: 0000000000000002
CapInh: 0000000000000002
CapAmb: 0000000000000000
CapAmb: 0000000000000000
CapInh: 00000000a80425fb
CapAmb: 0000000000000002
CapInh: 0000000000000000

------------------------------
• Failure [17.948 seconds]
Podman run
/usr/local/go/src/github.com/containers/podman/test/e2e/run_test.go:25
  podman run user capabilities test [It]
  /usr/local/go/src/github.com/containers/podman/test/e2e/run_test.go:467

  Expected
      <string>: CapInh: 0000000000000000
  to contain substring
      <string>: 0000000000000002

  /usr/local/go/src/github.com/containers/podman/test/e2e/run_test.go:553
------------------------------
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSintegration timing results
podman run user capabilities test               17.947786
"unlinkat /tmp/podman/imagecachedir/vfs/dir/2fa52ae883433bc788d7fa1737ca50d2802e0e39d71cae30b4c49378c1207a17/home: permission denied"


Summarizing 1 Failure:

[Fail] Podman run [It] podman run user capabilities test 
/usr/local/go/src/github.com/containers/podman/test/e2e/run_test.go:553

Ran 1 of 114 Specs in 41.103 seconds
FAIL! -- 0 Passed | 1 Failed | 0 Pending | 113 Skipped
--- FAIL: TestLibpod (41.12s)
FAIL
FAIL    command-line-arguments  41.131s
FAIL

I used this command to recreate the error:

GOPATH=~/go go test -v test/e2e/libpod_suite_test.go test/e2e/common_test.go test/e2e/config.go test/e2e/config_amd64.go test/e2e/run_test.go

Would anyone know what could possibly be causing this?

@rhatdan
Copy link
Member

rhatdan commented Mar 28, 2022

You need to rebase your PR.

git pull origin main
git rebase -i origin
git push --force

@TomSweeneyRedHat
Copy link
Member

@kbaran1998 you're still fighting DCO issues, looks like two of your commits are unsigned. Please see: https://github.com/containers/podman/pull/13490/checks?check_run_id=5727855035

Signed-off-by: Krzysztof Baran <krysbaran@gmail.com>
Signed-off-by: gcalin <caling@protonmail.com>
@TomSweeneyRedHat
Copy link
Member

LGTM
and happy green test buttons
Thanks @kbaran1998 !

@rhatdan
Copy link
Member

rhatdan commented Apr 4, 2022

Thanks @kbaran1998
/lgtm
/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 4, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kbaran1998, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

podman pod logs enhancements: option to color logs

8 participants