fix container startup for empty pidfile

[Luap99]

Commit 728b73d introduced a regression. Containers created with a
previous version do no longer start successfully. The problem is that
the PidFile in the container config is empty for those containers. If
the PidFile is empty we have to set it to the previous default.

[NO TESTS NEEDED] We should investigate why the system upgrade test did
not caught this.

Fixes #10274

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99

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]

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

[Luap99]

@edsantiago I see quite a few system upgrade tests are skipped. This one could have caught this regression:

podman/test/upgrade/test-upgrade.bats

Lines 235 to 239 in 54bed10

	@test "start" {
	skip "FIXME: this leaves a mount behind: root/overlay/sha/merged"
	run_podman --cgroup-manager=cgroupfs start -a mydonecontainer
	is "$output" "++$RANDOM_STRING_1++" "start on already-run container"
	}

[rhatdan]

LGTM
@containers/podman-maintainers PTAL

[vrothberg]

LGTM but I prefer waiting until @edsantiago had a chance to look at it.

[mheon]

LGTM

…

On Mon, May 10, 2021 at 06:52 Valentin Rothberg ***@***.***> wrote: ***@***.**** commented on this pull request. LGTM but I prefer waiting until @edsantiago <https://github.com/edsantiago> had a chance to look at it. — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub <#10288 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3AOCBEWBBHKTHWGQWMY3DTM63GFANCNFSM44QU63KQ> .

[edsantiago]

@Luap99 can you get the upgrade test to pass? If so, please feel free to un-skip it. All the skips are because of problems too deep for me to fix.

[Luap99]

@edsantiago I looked into this and found the root cause of the failures in the upgrade tests. Inside the container fuse-overlayfs is used, outside the native overlayfs is used. This causes troubles with the mount points. I need more time to look in this for all test cases.

[Luap99]

Lets get this merged for now.

[vrothberg]

/lgtm

[edsantiago]

Trying to follow up on this, I just checked out and built 5a8b3cf (master, with this PR merged), removed the skip from the upgrade "start" test, then ran:

$ sudo env PODMAN_UPGRADE_FROM=v2.1.1 bats ./test/upgrade/test-upgrade.bats
...
 - pods (skipped: TBI)
 ✗ start
   (from function `die' in file test/upgrade/../system/helpers.bash, line 412,
    from function `run_podman' in file test/upgrade/../system/helpers.bash, line 220,
    in test file test/upgrade/test-upgrade.bats, line 237)
     `run_podman --cgroup-manager=cgroupfs start -a mydonecontainer' failed with status 125
   # podman --cgroup-manager=cgroupfs start -a mydonecontainer
   Error: unable to start container f7c814b606bf06c3f2d0202b4b70cdd23462add3fa92af25c9a192d7d27649cb: opening file `` for writing: No such file or directory: OCI not found
   [ rc=125 (** EXPECTED 0 **) ]
   #/vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
   #| FAIL: exit code is 125; expected 0
   #\^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I would have expected that test to pass (possibly with the leftover-mount problem)?

[Luap99]

@edsantiago It worked for me locally. Did you forget to set PODMAN=bin/podman?

[edsantiago]

Blush. That was it, thank you. I now see the leftover-mount bug and will see if there's some way I can work around it.

[Luap99]

@edsantiago I have working tests locally I will push a PR soon.

[jlebon]

Can we get this into Bodhi ASAP to replace the broken one already pushed to the repos?

[mheon]

We need to back the RC out entirely - replacing it with another RC is not a good idea, we're likely to hit even more bugs. We have to go back to 3.1.2 until we have things stable and ready to release (at least another week). Unfortunately @lsm5 is out today so our main packager is gone.

[edsantiago]

This update has been pushed to stable.

I don't think there's any possible way to undo that. I think the only option is to urgently push out something with a version (or epoch) that overrides the bad release.

[mheon]

Epoch bump is probably our only option now. I am uncomfortable pushing another 3.2.0 build at present.

[TomSweeneyRedHat]

FWIW, I'm fine bumping to 3.1.3 in the near term, but I'm with @mheon on not going to 3.2.0 right now.

[mheon]

3.1.3 won’t help given that we have a 3.2 in thhe stable repos.

…

On Mon, May 10, 2021 at 13:52 Tom Sweeney ***@***.***> wrote: FWIW, I'm fine bumping to 3.1.3 in the near term, but I'm with @mheon <https://github.com/mheon> on not going to 3.2.0 right now. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#10288 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3AOCDDORD6Y6FDXBR2SBDTNAMN5ANCNFSM44QU63KQ> .