Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking Change in Container Config will cause JSON Decode Failures with Older Containers #8613

Closed
mheon opened this issue Dec 6, 2020 · 5 comments · Fixed by #8632
Closed
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@mheon
Copy link
Member

mheon commented Dec 6, 2020

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind bug

Description

In the very old, pre-1.0 days before Libpod had proper named volume support, we emulated image volumes using bind mounts or tmpfs mounts. Part of this support involved a field in container configuration, ImageVolumes (type string), that determined whether we were using a tmpfs or bind mount. Libpod eventually grew support for named volumes, and the field was deprecated and became unused. I removed it in Podman 1.8 with e3a549b.

@vrothberg recently added a new, completely separate feature, where images can be mounted into containers as volumes (e.g. for security inspection). This again adds an ImageVolumes field (https://github.com/containers/podman/blob/master/libpod/container_config.go#L138) but now with a different struct type. These are completely incompatible in JSON terms and means that any containers created with Podman < 1.8.0 will be completely unusable with Podman 2.2.0.

Steps to reproduce the issue:

  1. Check out and build Podman 1.6.4 (any pre-1.8 will work, but we still support 1.6.4 in RHEL).

  2. Create a container (podman create, podman run, doesn't really matter.

  3. Upgrade to Podman 2.2.0 and run podman ps

Describe the results you received:

Podman will not be able to read the container from the database due to a JSON decode error

Describe the results you expected:

Podman works as expected

Additional information you deem important (e.g. issue happens only occasionally):

The old ImageVolumes was not omitempty, so every container pre-removal is guaranteed to have it.

@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Dec 6, 2020
@mheon
Copy link
Member Author

mheon commented Dec 6, 2020

@rhatdan @vrothberg @baude PTAL, this is a bad one. Going to be interesting trying to resolve this without breaking 2.2.0 containers - we may have to rename the JSON field for the new image volumes and accept that any container made on 2.2.0 with image volumes will not have them after upgrade to 2.2.1.

@vrothberg
Copy link
Member

vrothberg commented Dec 7, 2020

Ewwww that's an ugly one. To summarize:

  • There was an imageVolumes field earlier of type FOO which was eventually removed.
  • Podman 2.2 added an imageVolumes field of type BAR (not knowing/forgetting that FOO ever existed).

Containers created with FOO now break with 2.2 as Podman expects it to be BAR.

As we cannot change the past, I suggest to rename the field and drop a HUGE comment at the top-level struct to never ever use that name again.

@mheon
Copy link
Member Author

mheon commented Dec 7, 2020

Concur.

@rhatdan
Copy link
Member

rhatdan commented Dec 7, 2020

SGTM

@TomSweeneyRedHat
Copy link
Member

@mheon, in the release notes, please include something like @vrothberg 's summary.

mheon added a commit to mheon/libpod that referenced this issue Dec 7, 2020
Podman pre-1.8 also included a field with this name, which was a
String. Podman 2.2.0 added a new field reusing the name but as a
Struct. This completely broke JSON decode for pre-1.8 containers
in Podman 2.2, resulting in completely broken behavior.

Re-name the JSON field and add a note that the old name should
not be re-used to prevent this problem from re-occurring. This
will still result in containers from 2.2.0 being broken
(specifically, containers with image volumes will have them
disappear) but this is the lesser of two evils.

Fixes containers#8613

Signed-off-by: Matthew Heon <mheon@redhat.com>
mheon added a commit to mheon/libpod that referenced this issue Dec 7, 2020
Podman pre-1.8 also included a field with this name, which was a
String. Podman 2.2.0 added a new field reusing the name but as a
Struct. This completely broke JSON decode for pre-1.8 containers
in Podman 2.2, resulting in completely broken behavior.

Re-name the JSON field and add a note that the old name should
not be re-used to prevent this problem from re-occurring. This
will still result in containers from 2.2.0 being broken
(specifically, containers with image volumes will have them
disappear) but this is the lesser of two evils.

Fixes containers#8613

Signed-off-by: Matthew Heon <mheon@redhat.com>
pmoogi-redhat pushed a commit to pmoogi-redhat/podman that referenced this issue Dec 15, 2020
Podman pre-1.8 also included a field with this name, which was a
String. Podman 2.2.0 added a new field reusing the name but as a
Struct. This completely broke JSON decode for pre-1.8 containers
in Podman 2.2, resulting in completely broken behavior.

Re-name the JSON field and add a note that the old name should
not be re-used to prevent this problem from re-occurring. This
will still result in containers from 2.2.0 being broken
(specifically, containers with image volumes will have them
disappear) but this is the lesser of two evils.

Fixes containers#8613

Signed-off-by: Matthew Heon <mheon@redhat.com>
@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 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Categorizes issue or PR as related to a bug. 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 a pull request may close this issue.

5 participants