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

Change default image volume mode to "anonymous" #1820

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

mheon
Copy link
Member

@mheon mheon commented Jan 24, 2024

We have not supported type=bind image volumes since pre-1.0 Podman - we phased them out when we added support for actual volumes. Also, our image volume valid modes checker did not even allow the actual default (anonymous). This is technically a breaking change, so it will go into Podman 5.0 - but I strongly doubt anyone is actually using this field if no one has noticed this issue before now.

Copy link
Contributor

openshift-ci bot commented Jan 24, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon

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

@mheon
Copy link
Member Author

mheon commented Jan 24, 2024

Needed for containers/podman#21339

@mheon mheon force-pushed the no_bind_image_vols branch 2 times, most recently from 2001192 to 1e5675c Compare January 24, 2024 14:40
@mheon
Copy link
Member Author

mheon commented Jan 24, 2024

@dfr FYI, it looks like this undoes a previous FreeBSD change, but as long as there is volume support in Podman on FreeBSD it shouldn't break anything.

We have not supported type=bind image volumes since pre-1.0
Podman - we phased them out when we added support for actual
volumes. Also, our image volume valid modes checker did not even
allow the actual default (anonymous). This is technically a
breaking change, so it will go into Podman 5.0 - but I strongly
doubt anyone is actually using this field if no one has noticed
this issue before now.

Signed-off-by: Matt Heon <mheon@redhat.com>
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

@dfr
Copy link
Contributor

dfr commented Jan 24, 2024

I'll take a look and make sure it doesn't affect volume support on FreeBSD

@mheon
Copy link
Member Author

mheon commented Jan 29, 2024

@dfr Any update? I'd like to get this merged this week.

@dfr
Copy link
Contributor

dfr commented Jan 29, 2024

@dfr Any update? I'd like to get this merged this week.

I believe that this change won't affect FreeBSD - podman contains code which substitutes define.TypeBind with "anonymous". I patched the changes to pkg/config/config.go and pkg/config/default.go into podman and tested image mounts on FreeBSD and they continue to work when the default is "anonymous".

@mheon
Copy link
Member Author

mheon commented Jan 29, 2024

TY!
@containers/podman-maintainers PTAL and merge

@rhatdan
Copy link
Member

rhatdan commented Jan 29, 2024

/approve
LGTM

@Luap99
Copy link
Member

Luap99 commented Jan 29, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jan 29, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit c1732f2 into containers:main Jan 29, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants