Merged request to fix -f to stop following logs

[jgallucci32]

This is a MERGED request from @QiWang19 and myself. It contains the test cases developed by her in addition to the modifications made to handle multiple cases. Close #6531

Fixes an issue with the first PR where a container would exit while following logs and the log tail continued to follow. This creates a subroutine which checks the state of the container and instructs the tailLog to stop when it reaches EOF.

Tested the following conditions:

• Tail and follow logs of running container
• Tail and follow logs of stopped container
• Tail and follow logs of running container which exits after some time

[openshift-ci-robot]

Hi @jgallucci32. Thanks for your PR.

I'm waiting for a containers member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[rhatdan]

@jgallucci32 You need to sign your commits.

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

[jgallucci32]

@rhatdan Thanks for the guidance. Git commit has been signed and it is reflected in the commit comments now for this PR.

[rhatdan]

/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jgallucci32, 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:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[QiWang19]

Will this also need to be added to WaitGroup?

[jgallucci32]

@QiWang19 When t.Stop() is called is should be unblocking the t.Lines() called in the other subroutine which would allow the other subroutine to end which calls options.WaitGroup.Done() before it returns.

[QiWang19]

@jgallucci32 Can you rebase them to one commit and git commit -a --amend -s sign-off the commit

[jgallucci32]

@QiWang19 This seem to be failing on two Ubuntu integration tests and looking at how it failed it appears to be a race condition when following the logs of a container created with the --rm flag and immediately creating a new container with the same name 1 second later. Because we have a subroutine which has a 1 second wait, the Ubuntu test also has a 1 second wait so we create a race condition.

Is there a way we could use Signaling, rather than a loop/sleep routine, which would execute t.Stop() when the container exits? This way the subroutine could simply be in a wait state for a signal rather than a loop checking for a condition.

Here is the exact point of failure from the integration test

[+0317s] Podman logs 
           streaming output
           /var/tmp/go/src/github.com/containers/libpod/test/e2e/logs_test.go:275
         [BeforeEach] Podman logs
           /var/tmp/go/src/github.com/containers/libpod/test/e2e/logs_test.go:22
         [It] streaming output
           /var/tmp/go/src/github.com/containers/libpod/test/e2e/logs_test.go:275
         Running: podman [options] run --rm --name logs-f-rm -dt docker.io/library/alpine:latest sh -c echo podman; sleep 1; echo podman
         e35944de52d177fbe8979eed78e379aa774352ae391223386df60f51f7482fee
         Running: podman [options] logs -f logs-f-rm
         podman
[+0318s] podman
         Running: podman [options] run --rm --name logs-f-rm -dt docker.io/library/alpine:latest true
         Error: error creating container storage: the container name "logs-f-rm" is already in use by "e35944de52d177fbe8979eed78e379aa774352ae391223386df60f51f7482fee". You have to remove that container to be able to reuse that name.: that name is already in use
         [AfterEach] Podman logs
           /var/tmp/go/src/github.com/containers/libpod/test/e2e/logs_test.go:32
         Running: podman [options] stop -a --time 0
         Running: podman [options] pod stop -a -t 0
         Running: podman [options] pod rm -fa
         Running: podman [options] rm -fa

Ref: https://storage.googleapis.com/cirrus-ci-5385732420009984-fcae48/artifacts/containers/libpod/6616670028431360/html/integration_test.log.html

[jgallucci32]

@jgallucci32 Can you rebase them to one commit and git commit -a --amend -s sign-off the commit

I attempted to do that but messed it up because I was going back and forth between GitHub.com and my Linux box. I've reset the head to the merge I did 30 min ago and have amended the commit to have a signature. Hopefully that fixed it.

[mheon]

The tests seem green - did the rerun fix them?

[rhatdan]

/lgtm

[QiWang19]

Ref: https://storage.googleapis.com/cirrus-ci-5385732420009984-fcae48/artifacts/containers/libpod/6616670028431360/html/integration_test.log.html

@jgallucci32 The failure seems to be a test flake not a race condition.

[jgallucci32]

@QiWang19 I found a better way to implement this which uses Event Listeners rather than sleep statements. This is just in case there is a race condition untested.
I created PR #6614 to see if it passes all the tests.

If it passes all the checks I would prefer we use this option rather than a sleep statement since it uses a safer approach.