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

RUN command after VOLUME changes mountpoint owner #2202

Closed
carbonin opened this issue Mar 3, 2020 · 1 comment · Fixed by #2216
Closed

RUN command after VOLUME changes mountpoint owner #2202

carbonin opened this issue Mar 3, 2020 · 1 comment · Fixed by #2216

Comments

@carbonin
Copy link
Contributor

carbonin commented Mar 3, 2020

Description

Any RUN command following a VOLUME command changes the volume user and group ownership.

Steps to reproduce the issue:

  1. Build the following two Dockerfiles using buildah bud
FROM centos:8
RUN dnf -y install --setopt=tsflags=nodocs postgresql-server
VOLUME [ "/var/lib/pgsql/data" ]
FROM centos:8
RUN dnf -y install --setopt=tsflags=nodocs postgresql-server
VOLUME [ "/var/lib/pgsql/data" ]
RUN touch /root/test
  1. Run the resulting images with the following command
podman run --rm <image> ls -l /var/lib/pgsql

Describe the results you received:

Image one:

$ podman run --rm 5237d257a48dc623f3c1ecb00ccfc0a2eba00daa2895d75339db8da21cec1a14 ls -l /var/lib/pgsql
total 0
drwx------. 1 postgres postgres 0 May 31  2019 backups
drwx------. 1 postgres postgres 0 Mar  3 18:57 data

Image two:

$ podman run --rm 51a2b48b5a97cd0b05eb942239534595093e57012821befbf2843bba5f68dc2c ls -l /var/lib/pgsql
total 0
drwx------. 1 postgres postgres 0 May 31  2019 backups
drwx------. 1 root     root     0 Mar  3 18:58 data

Describe the results you expected:
I expected the user and group of the data directory to not be changed by running touch /root/test

Output of rpm -q buildah or apt list buildah:

buildah-1.12.0-2.fc30.x86_64

Output of buildah version:

Version:         1.12.0
Go Version:      go1.12.13
Image Spec:      1.0.1-dev
Runtime Spec:    1.0.1-dev
CNI Spec:        0.4.0
libcni Version:  
image Version:   5.0.0
Git Commit:      
Built:           Wed Dec 31 19:00:00 1969
OS/Arch:         linux/amd64

Output of podman version if reporting a podman build issue:

Version:            1.7.0
RemoteAPI Version:  1
Go Version:         go1.12.14
OS/Arch:            linux/amd64

Output of cat /etc/*release:

Fedora release 30 (Thirty)
NAME=Fedora
VERSION="30 (Workstation Edition)"
ID=fedora
VERSION_ID=30
VERSION_CODENAME=""
PLATFORM_ID="platform:f30"
PRETTY_NAME="Fedora 30 (Workstation Edition)"
ANSI_COLOR="0;34"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:30"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f30/system-administrators-guide/"
SUPPORT_URL="https://fedoraproject.org/wiki/Communicating_and_getting_help"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=30
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=30
PRIVACY_POLICY_URL="https://fedoraproject.org/wiki/Legal:PrivacyPolicy"
VARIANT="Workstation Edition"
VARIANT_ID=workstation
Fedora release 30 (Thirty)
Fedora release 30 (Thirty)

Output of uname -a:

Linux localhost.localdomain 5.4.19-100.fc30.x86_64 #1 SMP Tue Feb 11 22:27:11 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

Output of cat /etc/containers/storage.conf:

# This file is is the configuration file for all tools
# that use the containers/storage library.
# See man 5 containers-storage.conf for more information
# The "container storage" table contains all of the server options.
[storage]

# Default Storage Driver
driver = "overlay"

# Temporary storage location
runroot = "/var/run/containers/storage"

# Primary Read/Write location of container storage
graphroot = "/var/lib/containers/storage"

[storage.options]
# Storage options to be passed to underlying storage drivers

# AdditionalImageStores is used to pass paths to additional Read/Only image stores
# Must be comma separated list.
additionalimagestores = [
]

# Size is used to set a maximum size of the container image.  Only supported by
# certain container storage drivers.
size = ""

# Path to an helper program to use for mounting the file system instead of mounting it
# directly.
#mount_program = "/usr/bin/fuse-overlayfs"

# OverrideKernelCheck tells the driver to ignore kernel checks based on kernel version
override_kernel_check = "true"

# mountopt specifies comma separated list of extra mount options
mountopt = "nodev,metacopy=on"

# Remap-UIDs/GIDs is the mapping from UIDs/GIDs as they should appear inside of
# a container, to UIDs/GIDs as they should appear outside of the container, and
# the length of the range of UIDs/GIDs.  Additional mapped sets can be listed
# and will be heeded by libraries, but there are limits to the number of
# mappings which the kernel will allow when you later attempt to run a
# container.
#
# remap-uids = 0:1668442479:65536
# remap-gids = 0:1668442479:65536

# Remap-User/Group is a name which can be used to look up one or more UID/GID
# ranges in the /etc/subuid or /etc/subgid file.  Mappings are set up starting
# with an in-container ID of 0 and the a host-level ID taken from the lowest
# range that matches the specified name, and using the length of that range.
# Additional ranges are then assigned, using the ranges which specify the
# lowest host-level IDs first, to the lowest not-yet-mapped container-level ID,
# until all of the entries have been used for maps.
#
# remap-user = "storage"
# remap-group = "storage"

[storage.options.thinpool]
# Storage Options for thinpool

# autoextend_percent determines the amount by which pool needs to be
# grown. This is specified in terms of % of pool size. So a value of 20 means
# that when threshold is hit, pool will be grown by 20% of existing
# pool size.
# autoextend_percent = "20"

# autoextend_threshold determines the pool extension threshold in terms
# of percentage of pool size. For example, if threshold is 60, that means when
# pool is 60% full, threshold has been hit.
# autoextend_threshold = "80"

# basesize specifies the size to use when creating the base device, which
# limits the size of images and containers.
# basesize = "10G"

# blocksize specifies a custom blocksize to use for the thin pool.
# blocksize="64k"

# directlvm_device specifies a custom block storage device to use for the
# thin pool. Required if you setup devicemapper.
# directlvm_device = ""

# directlvm_device_force wipes device even if device already has a filesystem.
# directlvm_device_force = "True"

# fs specifies the filesystem type to use for the base device.
# fs="xfs"

# log_level sets the log level of devicemapper.
# 0: LogLevelSuppress 0 (Default)
# 2: LogLevelFatal
# 3: LogLevelErr
# 4: LogLevelWarn
# 5: LogLevelNotice
# 6: LogLevelInfo
# 7: LogLevelDebug
# log_level = "7"

# min_free_space specifies the min free space percent in a thin pool require for
# new device creation to succeed. Valid values are from 0% - 99%.
# Value 0% disables
# min_free_space = "10%"

# mkfsarg specifies extra mkfs arguments to be used when creating the base.
# device.
# mkfsarg = ""

# use_deferred_removal marks devicemapper block device for deferred removal.
# If the thinpool is in use when the driver attempts to remove it, the driver 
# tells the kernel to remove it as soon as possible. Note this does not free
# up the disk space, use deferred deletion to fully remove the thinpool.
# use_deferred_removal = "True"

# use_deferred_deletion marks thinpool device for deferred deletion.
# If the device is busy when the driver attempts to delete it, the driver
# will attempt to delete device every 30 seconds until successful.
# If the program using the driver exits, the driver will continue attempting
# to cleanup the next time the driver is used. Deferred deletion permanently
# deletes the device and all data stored in device will be lost.
# use_deferred_deletion = "True"

# xfs_nospace_max_retries specifies the maximum number of retries XFS should
# attempt to complete IO when ENOSPC (no space) error is returned by
# underlying storage device.
# xfs_nospace_max_retries = "0"

# If specified, use OSTree to deduplicate files with the overlay backend
ostree_repo = ""

# Set to skip a PRIVATE bind mount on the storage home directory.  Only supported by
# certain container storage drivers
skip_mount_home = "false"
carbonin added a commit to carbonin/manageiq that referenced this issue Mar 3, 2020
This avoids the problems with the mount point caused by
containers/buildah#2202 when
using buildah
carbonin added a commit to carbonin/manageiq that referenced this issue Mar 3, 2020
carbonin added a commit to carbonin/manageiq that referenced this issue Mar 3, 2020
@TomSweeneyRedHat TomSweeneyRedHat self-assigned this Mar 3, 2020
carbonin added a commit to carbonin/buildah that referenced this issue Mar 10, 2020
Closes containers#2202

Signed-off-by: Nick Carboni <ncarboni@redhat.com>
@carbonin
Copy link
Contributor Author

@TomSweeneyRedHat I think I was able to fix this. I opened up #2216, can you take a look?

I haven't worked out a test case yet as I assume installing additional packages isn't the best choice.

bors bot added a commit that referenced this issue Mar 11, 2020
2216: Preserve volume uid and gid through subsequent commands r=TomSweeneyRedHat a=carbonin

<!--
Thanks for sending a pull request!

Please make sure you've read and understood our contributing guidelines
(https://github.com/containers/buildah/blob/master/CONTRIBUTING.md) as well as ensuring
that all your commits are signed with `git commit -s`.
-->

#### What type of PR is this?

<!--
Please label this pull request according to what type of issue you are
addressing, especially if this is a release targeted pull request.

Uncomment only one `/kind <>` line, hit enter to put that in a new line, and
remove leading whitespace from that line:
-->

/kind bug


#### What this PR does / why we need it:

This PR pulls the UID and GID fields from the stored file stat when reconstructing a volume after subsequent commands

#### How to verify it
Build the following Dockerfile:
```Dockerfile
FROM centos:8
RUN dnf -y install --setopt=tsflags=nodocs postgresql-server
VOLUME [ "/var/lib/pgsql/data" ]
RUN touch /root/test
```

Observe the output of the following command:
```bash
$ podman run --rm <image> ls -l /var/lib/pgsql
total 0
drwx------. 1 postgres postgres 0 May 31  2019 backups
drwx------. 1 postgres postgres 0 Mar 10 23:37 data
```

#### Which issue(s) this PR fixes:

<!--
Automatically closes linked issue when PR is merged.
Uncomment the following comment block and include the issue
number or None on one line.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`, or `None`.
-->

Fixes #2202

#### Does this PR introduce a user-facing change?
No
<!--
If no, just write `None` in the release-note block below. If yes, a release note
is required: Enter your extended release note in the block below. If the PR
requires additional action from users switching to the new release, include the
string "action required".

For more information on release notes please follow the kubernetes model:
https://git.k8s.io/community/contributors/guide/release-notes.md
-->

```release-note
None
```



Co-authored-by: Nick Carboni <ncarboni@redhat.com>
@bors bors bot closed this as completed in 769a3e5 Mar 11, 2020
@bors bors bot closed this as completed in #2216 Mar 11, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants