logs: k8s-file: fix race

[vrothberg]

Fix a race in the k8s-file logs driver. When "following" the logs,
Podman will print the container's logs until the end. Previously,
Podman logged until the state transitioned into something non-running
which opened up a race with the container still running, possibly in
the "stopping" state.

To fix the race, log until we've seen the wait event for the specific
container. In that case, conmon will have finished writing all logs to
the file, and Podman will read it until EOF.

Further tweak the integration tests for testing logs -f on a running
container. Previously, the test only checked for one of two lines
stating that there was a race. Indeed the race was in using run --rm
where a log file may be removed before we could fully read it.

Fixes: #10596
Signed-off-by: Valentin Rothberg [email protected]

[Luap99]

I don't think this works.

[Luap99]

I don't understand this. You never read the event channel and this will just hang forever and never exit.

[vrothberg]

Absolutely, thanks! Took a phone call and thought I added the remaining bits :^)

[Luap99]

Also take a look at the following for a possible test:

podman/test/e2e/logs_test.go

Lines 186 to 193 in 9a3a732

	// TODO: we should actually check for two podman lines,
	// but as of 2020-06-17 there's a race condition in which
	// 'logs -f' may not catch all output from a container
	Expect(results.OutputToString()).To(ContainSubstring("podman"))
	// Container should now be terminatING or terminatED, but we
	// have no guarantee of which: 'logs -f' does not necessarily
	// wait for cleanup. Run 'inspect' and accept either state.

[vrothberg]

Also take a look at the following for a possible test:

podman/test/e2e/logs_test.go

Lines 186 to 193 in 9a3a732

	// TODO: we should actually check for two podman lines,
	// but as of 2020-06-17 there's a race condition in which
	// 'logs -f' may not catch all output from a container
	Expect(results.OutputToString()).To(ContainSubstring("podman"))
	// Container should now be terminatING or terminatED, but we
	// have no guarantee of which: 'logs -f' does not necessarily
	// wait for cleanup. Run 'inspect' and accept either state.

Good catch. With the recent fixes in the journald logger and this PR, we should be good to go. I'll take a look.

[Luap99]

LGTM

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, vrothberg

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:

• OWNERS [Luap99,vrothberg]

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

[rhatdan]

/lgtm
/hold

[rhatdan]

/hold cancel

[maybe-sybr]

@mheon I've also hit the issue introduced by 84b55ee in 3.2.1 (picked as c3f6ef6) and would appreciate if you could include this patch in 3.2.2 :)

[vrothberg]

@mheon I've also hit the issue introduced by 84b55ee in 3.2.1 (picked as c3f6ef6) and would appreciate if you could include this patch in 3.2.2 :)

Roger :) I backported a fix for the fix yesterday to the v3.2 branch, so it'll be fixed in 3.2.2.