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

Data not copying up to volumes for containers created but not started #10262

Closed
vikas-goel opened this issue May 7, 2021 · 18 comments · Fixed by #10970
Closed

Data not copying up to volumes for containers created but not started #10262

vikas-goel opened this issue May 7, 2021 · 18 comments · Fixed by #10970
Assignees
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

@vikas-goel
Copy link
Contributor

vikas-goel commented May 7, 2021

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

/kind bug

Description

When volume and container with this volume are created by docker, that volume gets populated (copied up from the image) and can then be consumed by another container using --volumes-from. It does not seem to work with podman. With podman, the workaround is to run podman container init after creating the container as described in #8041.

Running podman container init gives error same as mentioned in #8041 (comment). The command, however, does the job.

I am using Docker Compose to create data volume container and then adding the same as a dependency for the service/application in the same Docker Compose configuration YAML file.

With Podman 3.0.2 and later version, I am using podman-docker package to manage my podman containers using Docker Compose. As the said podman command (podman container init) for the workaround is very specific to podman, Docker Compose does not have option to do the same.

Can we have a podman (global) configuration option, say setup-volume-mount, that can stay turned off by default. When the option is turned on, the podman create --name <name> <container-image> behaves same as corresponding docker command. That is, the podman should internally prepare the mount, i.e. copy the data without requiring additional commands (podman container init). The error described in #8041 (comment) should be ignored.

Steps to reproduce the issue:
Same as described in #8041

Describe the results you received:
Same as described in #8041

Describe the results you expected:
Same as described in #8041

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

Output of podman version:

Version:      3.0.2-dev
API Version:  3.0.0
Go Version:   go1.15.7
Built:        Tue Mar  2 07:10:06 2021
OS/Arch:      linux/amd64

Output of podman info --debug:

host:
  arch: amd64
  buildahVersion: 1.19.4
  cgroupManager: systemd
  cgroupVersion: v1
  conmon:
    package: conmon-2.0.26-1.module+el8.4.0+10198+36d1d0e3.x86_64
    path: /usr/bin/conmon
    version: 'conmon version 2.0.26, commit: 0a5175681bdd52b99f1f0f442cbba8f8c126a1c9'
  cpus: 8
  distribution:
    distribution: '"rhel"'
    version: "8.4"
  eventLogger: file
  hostname: flex-vm-02.dc2.ros2100.veritas.com
  idMappings:
    gidmap: null
    uidmap: null
  kernel: 4.18.0-293.el8.x86_64
  linkmode: dynamic
  memFree: 14160437248
  memTotal: 33511845888
  ociRuntime:
    name: runc
    package: runc-1.0.0-70.rc92.module+el8.4.0+10198+36d1d0e3.x86_64
    path: /usr/bin/runc
    version: 'runc version spec: 1.0.2-dev'
  os: linux
  remoteSocket:
    exists: true
    path: /run/podman/podman.sock
  security:
    apparmorEnabled: false
    capabilities: CAP_NET_RAW,CAP_CHOWN,CAP_DAC_OVERRIDE,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_NET_BIND_SERVICE,CAP_SETFCAP,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_SYS_CHROOT
    rootless: false
    seccompEnabled: true
    selinuxEnabled: true
  slirp4netns:
    executable: ""
    package: ""
    version: ""
  swapFree: 16921104384
  swapTotal: 16924012544
  uptime: 61h 34m 49.72s (Approximately 2.54 days)
registries:
  search:
  - registry.access.redhat.com
  - registry.redhat.io
  - docker.io
store:
  configFile: /etc/containers/storage.conf
  containerStore:
    number: 5
    paused: 0
    running: 2
    stopped: 3
  graphDriverName: overlay
  graphOptions:
    overlay2.size: 10G
  graphRoot: /var/lib/containers/storage
  graphStatus:
    Backing Filesystem: xfs
    Native Overlay Diff: "true"
    Supports d_type: "true"
    Using metacopy: "false"
  imageStore:
    number: 3
  runRoot: /run/containers/storage
  volumePath: /var/lib/containers/storage/volumes
version:
  APIVersion: 3.0.0
  Built: 1614697806
  BuiltTime: Tue Mar  2 07:10:06 2021
  GitCommit: ""
  GoVersion: go1.15.7
  OsArch: linux/amd64
  Version: 3.0.2-dev

Package info (e.g. output of rpm -q podman or apt list podman):

podman-3.0.1-3.module+el8.4.0+10198+36d1d0e3.x86_64

Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide? (https://github.com/containers/podman/blob/master/troubleshooting.md)

No

Additional environment details (AWS, VirtualBox, physical, etc.):
Red Hat Enterprise Linux 8.4 Beta
VMware virtual machine

@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label May 7, 2021
@vikas-goel
Copy link
Contributor Author

Hi @mheon , @rhatdan,
Is it possible to get this fixed in podman-3.2?

@mheon
Copy link
Member

mheon commented May 17, 2021

I doubt it. This would involve a significant refactor of the way volume mount and copy-up happens to move it to container creation from first-time initialization. We're planning 3.2.0 final later this week, and this is too much work and too large of a change to land before then. Maybe in 3.3.x.

@vikas-goel
Copy link
Contributor Author

I see. Is there a timeline for 3.3?

@mheon
Copy link
Member

mheon commented May 17, 2021

If we maintain our current pace, mid to late July seems reasonable?

@vikas-goel
Copy link
Contributor Author

That will work. Thank you.

@vikas-goel
Copy link
Contributor Author

Hi @mheon , just checking on this one.

@vikas-goel
Copy link
Contributor Author

@mheon , is this planned for the July release?

@mheon
Copy link
Member

mheon commented Jul 9, 2021

No one has begun work on it yet.

@vikas-goel
Copy link
Contributor Author

vikas-goel commented Jul 11, 2021

@mheon ,
I looked at the code for fixing the issue. Would the following two patches be good?

  1. containers/common
diff --git a/pkg/config/config.go b/pkg/config/config.go
index 68076b1..b30b4ff 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -125,6 +125,9 @@ type ContainersConfig struct {
        // container that forwards signals and reaps processes.
        Init bool `toml:"init,omitempty"`

+       // Initialize container during creation
+       InitOnCreate bool `toml:"init_on_create,omitempty"`
+
        // InitPath is the path for init to run if the Init bool is enabled
        InitPath string `toml:"init_path,omitempty"`

  1. container/podman
diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go
index 6c69d1b..1d17ab4 100644
--- a/libpod/runtime_ctr.go
+++ b/libpod/runtime_ctr.go
@@ -446,6 +446,14 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
                toLock.lock.Lock()
                defer toLock.lock.Unlock()
        }
+
+       // Auto initialize the container during creation if user opted to.
+       if r.config.Containers.InitOnCreate {
+               if err := ctr.init(ctx, false); err != nil {
+                       return nil, err
+               }
+       }
+
        // Add the container to the state
        // TODO: May be worth looking into recovering from name/ID collisions here
        if ctr.config.Pod != "" {

I'm unable to create branches for these repos. How can I contribute to this issue?

@rhatdan
Copy link
Member

rhatdan commented Jul 11, 2021

Why can't you create branches for these repos?

@mheon
Copy link
Member

mheon commented Jul 11, 2021

The patch also won't work - it's happening too soon. Initialization must happen after the container is added to the state.

@vikas-goel
Copy link
Contributor Author

vikas-goel commented Jul 11, 2021

Why can't you create branches for these repos?

GitHub does not show me option to create branch when I type a unique name in the branch text field.

image

Pushing the changes fail.

Password for 'https://[email protected]':
remote: Permission to containers/common.git denied to vikas-goel.
fatal: unable to access 'https://[email protected]/containers/common.git/': The requested URL returned error: 403

image

Looks like I am receiving the error because I am not member of the “Containers” GitHub organization.

@vikas-goel
Copy link
Contributor Author

vikas-goel commented Jul 11, 2021

Thanks @mheon for the review and comment.

The patch also won't work - it's happening too soon. Initialization must happen after the container is added to the state.

Not sure what "happening to soon" mean. I changed the logic to just prepare container instead of initializing so that there is no init event sent. It would also avoid the "executable file not found in" error as observed in the issue. With this change, I guess we should be okay calling prepare before adding the container to the state. Let me know if that's not the case.

The new patches look like

  1. containers/common
diff --git a/pkg/config/config.go b/pkg/config/config.go
index 68076b1..1bb980e 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -158,6 +158,9 @@ type ContainersConfig struct {
        // PidNS indicates how to create a pid namespace for the container
        PidNS string `toml:"pidns,omitempty"`

+       // Prepare container to mount storage during creation
+       PrepareOnCreate bool `toml:"prepare_on_create,omitempty"`
+
        // RootlessNetworking depicts the "kind" of networking for rootless
        // containers.  Valid options are `slirp4netns` and `cni`. Default is
        // `slirp4netns`
  1. containers/podman
diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go
index 6c69d1b..f5515ab 100644
--- a/libpod/runtime_ctr.go
+++ b/libpod/runtime_ctr.go
@@ -446,6 +446,14 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
                toLock.lock.Lock()
                defer toLock.lock.Unlock()
        }
+
+       // Prepare container to mount storage during creation if user opted to.
+       if r.config.Containers.PrepareOnCreate {
+               if err := ctr.prepare(); err != nil {
+                       return nil, err
+               }
+       }
+
        // Add the container to the state
        // TODO: May be worth looking into recovering from name/ID collisions here
        if ctr.config.Pod != "" {

@mheon
Copy link
Member

mheon commented Jul 11, 2021

This does not fix the core problem of the earlier patch - the prepare is happening before the container is added to the state, so configuration information is not being saved to the DB. The old init() code was fine but it must be more further down in the function.

@vikas-goel
Copy link
Contributor Author

vikas-goel commented Jul 11, 2021

@mheon ,
I see. Thanks for the quick one. I am moving the call after adding the container to the state. I am still keeping it to prepare() so as to avoid the "executable file not found" error.

diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go
index 6c69d1b..200c1a6 100644
--- a/libpod/runtime_ctr.go
+++ b/libpod/runtime_ctr.go
@@ -460,6 +460,14 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
        } else if err := r.state.AddContainer(ctr); err != nil {
                return nil, err
        }
+
+       // Prepare container to mount storage during creation if user opted to.
+       if r.config.Containers.PrepareOnCreate {
+               if err := ctr.prepare(); err != nil {
+                       return nil, err
+               }
+       }
+
        ctr.newContainerEvent(events.Create)
        return ctr, nil
 }

Is this okay?

I also wanted to check if the prepare() needs to be a conditional call or can it happen by default? I remember you mentioned that doing this may have performance impact on podman run. I was wondering about that.

@vikas-goel
Copy link
Contributor Author

Could someone set the “In Progress” label and assign this to me?

@rhatdan
Copy link
Member

rhatdan commented Jul 12, 2021

@vikas-goel I am suggesting you fork the project create a branch and open a PR for MAIN. Not that you create a branch on the main repo.

@vikas-goel
Copy link
Contributor Author

Thank you @rhatdan for the suggestion and assignment.

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
4 participants