Skip to content

Conversation

keonchennl
Copy link

Fixes #13261

Signed-off-by: Xueyuan Chen [email protected]

@keonchennl keonchennl force-pushed the pod-logs-add-flag branch 2 times, most recently from 548c11c to 3ad95b1 Compare February 26, 2022 23:10
@keonchennl
Copy link
Author

I noticed that showing container names in the log was already implemented for container logs, so I just added the same --names flag for pod logs, which resolves this issue #13261.

Copy link
Member

@rhatdan rhatdan left a comment

Choose a reason for hiding this comment

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

Thanks @keonchennl
A couple of Nits

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 27, 2022

[APPROVALNOTIFIER] This PR is APPROVED

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

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 Feb 27, 2022
@keonchennl keonchennl force-pushed the pod-logs-add-flag branch 5 times, most recently from a3978d4 to 9990172 Compare February 27, 2022 12:06
@keonchennl
Copy link
Author

keonchennl commented Feb 27, 2022

Hi Daniel @rhatdan I am sort of stuck in fixing the pipeline for this PR:
My local test run passes (on ubuntu 20.04) and manual testing also gave the correct result: the container names show up if I run podman pod logs --names testPod.
But the integration tests in the pipeline keep failing on my test case: the container names are even missing in the log with the added "--names" flag.
Could you help have a look at what went wrong?

Below is the result of one of the integration tests (int-podman-ubuntu-2110-rootless-host)

[+0831s] Podman logs
podman pod logs with container names
/var/tmp/go/src/github.com/containers/podman/test/e2e/logs_test.go:418

     [BeforeEach] Podman logs
       /var/tmp/go/src/github.com[/containers/podman/test/e2e/logs_test.go:33](https://github.com/containers/podman/blob/d45bc76bd5ec2460a3b4add99b13f8d07deb1937/test/e2e/logs_test.go#L33)
     [It] podman pod logs with container names
       /var/tmp/go/src/github.com[/containers/podman/test/e2e/logs_test.go:418](https://github.com/containers/podman/blob/d45bc76bd5ec2460a3b4add99b13f8d07deb1937/test/e2e/logs_test.go#L418)
     $ podman [options] --network-backend cni --storage-driver vfs pod create --name=testPod
     9f637ee52baad9e514102e49d9c3d33df9ddebb1134c12cae7a04839b7466921
     $ podman [options] --network-backend cni --storage-driver vfs run --name container1 -d --pod testPod quay.io/libpod/busybox:latest /bin/sh -c echo log1
     fc75b91a47df22d53817ce5d1f30d1fba840cc9e2767b06c0c857671d194787b
     $ podman [options] --network-backend cni --storage-driver vfs run --name container2 -d --pod testPod quay.io/libpod/busybox:latest /bin/sh -c echo log2
     18de183755f796dc82b4f9ab79111b3feca77a8216ed46c5d22ec8cb535b7bca
     $ podman [options] --network-backend cni --storage-driver vfs pod logs --names testPod
      log2
      log1
     [AfterEach] Podman logs
       /var/tmp/go/src/github.com[/containers/podman/test/e2e/logs_test.go:45](https://github.com/containers/podman/blob/d45bc76bd5ec2460a3b4add99b13f8d07deb1937/test/e2e/logs_test.go#L45)
     $ podman [options] --network-backend cni --storage-driver vfs stop -a --time 0
     fc75b91a47df22d53817ce5d1f30d1fba840cc9e2767b06c0c857671d194787b
     18de183755f796dc82b4f9ab79111b3feca77a8216ed46c5d22ec8cb535b7bca
     53c4bf046abb489ed34d72125542a77c40c0310229f3106b03aaee218566f83c
     $ podman [options] --network-backend cni --storage-driver vfs pod stop -a -t 0
     9f637ee52baad9e514102e49d9c3d33df9ddebb1134c12cae7a04839b7466921
     $ podman [options] --network-backend cni --storage-driver vfs pod rm -fa
     9f637ee52baad9e514102e49d9c3d33df9ddebb1134c12cae7a04839b7466921
     $ podman [options] --network-backend cni --storage-driver vfs rm -fa
     "unlinkat /tmp/podman_test312261090/root/vfs/dir/afc1e1000492d3b2af02f16519a85e38822038b68c9ebda54fd6d5272e832617/catatonit: permission denied"
     
     

     • Failure [3.071 seconds]
     Podman logs
     /var/tmp/go/src/github.com[/containers/podman/test/e2e/logs_test.go:26](https://github.com/containers/podman/blob/d45bc76bd5ec2460a3b4add99b13f8d07deb1937/test/e2e/logs_test.go#L26)
       podman pod logs with container names [It]
       /var/tmp/go/src/github.com[/containers/podman/test/e2e/logs_test.go:418](https://github.com/containers/podman/blob/d45bc76bd5ec2460a3b4add99b13f8d07deb1937/test/e2e/logs_test.go#L418)
     
       Expected
           <[]string | len:2, cap:2>: [" log2", " log1"]
       to contain element matching
           <*matchers.ContainSubstringMatcher | 0xc000a42270>: {Substr: "container1", Args: nil}
     
       /var/tmp/go/src/github.com[/containers/podman/test/e2e/logs_test.go:441](https://github.com/containers/podman/blob/d45bc76bd5ec2460a3b4add99b13f8d07deb1937/test/e2e/logs_test.go#L441)
     
       Full Stack Trace
       github.com/containers/podman/v4/test/e2e.glob..func36.25()
       	/var/tmp/go/src/github.com[/containers/podman/test/e2e/logs_test.go:441](https://github.com/containers/podman/blob/d45bc76bd5ec2460a3b4add99b13f8d07deb1937/test/e2e/logs_test.go#L441) +0x7a2
       github.com/containers/podman/v4/test/e2e.TestLibpod(0x0)
       	/var/tmp/go/src/github.com[/containers/podman/test/e2e/common_test.go:100](https://github.com/containers/podman/blob/d45bc76bd5ec2460a3b4add99b13f8d07deb1937/test/e2e/common_test.go#L100) +0x109
       testing.tRunner(0xc000261040, 0x15f9c78)
       	/usr/local/go/src/testing/testing.go:1259 +0x102
       created by testing.(*T).Run
       	/usr/local/go/src/testing/testing.go:1306 +0x35a

@keonchennl keonchennl force-pushed the pod-logs-add-flag branch 2 times, most recently from b5d6118 to d45bc76 Compare February 27, 2022 15:47
@rhatdan
Copy link
Member

rhatdan commented Feb 28, 2022

Add this patch
patch.txt

@rhatdan
Copy link
Member

rhatdan commented Feb 28, 2022

LGTM other then @TomSweeneyRedHat nit.

@TomSweeneyRedHat
Copy link
Member

LGTM other than that nit of a nit

@rhatdan
Copy link
Member

rhatdan commented Feb 28, 2022

/lgtm
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 28, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 28, 2022
@keonchennl
Copy link
Author

It seems some integration test cases get unstable. I'm not sure if the changes cause this. @rhatdan

In last run:

This one was flaky:

[Fail] podman system service verify pprof endpoints [It] are not available

This one failed in int-podman-ubuntu-2110-rootless-host:

[Fail] Podman run with --mac-address flag [It] Podman run --mac-address with custom network

@rhatdan
Copy link
Member

rhatdan commented Mar 1, 2022

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 1, 2022
@openshift-merge-robot openshift-merge-robot merged commit 8bdda91 into containers:main Mar 1, 2022
@keonchennl keonchennl deleted the pod-logs-add-flag branch March 1, 2022 16:58
@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 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

podman pod logs enhancements: option to display names

4 participants