user-cont / conu

conu - python API for your containers
http://conu.readthedocs.io/en/latest/
MIT License
165 stars 33 forks source link

[WIP] libpod/podman backend #200 #298

Closed fasashen closed 5 years ago

fasashen commented 5 years ago

PodmanBackend implementation based on DockerBackend

At this moment a few basic features of podman's backend are implemented, that pass tests:

Tested inside vagrant's VM. There is an unsolved issue: podman's container doesn't run with no --privileged flag, throwing status Exited (139).

Part of Red Hat Open Source Contest

centos-ci commented 5 years ago

Can one of the admins verify this patch?

fasashen commented 5 years ago

@rpitonak Now podman backend passes almost all tests, except manipulating with copy to/from and layers. All the test_podman.py tests are nearly the same as test_docker.py with small changes.

But I haven't tested it properly: there are some problems with docker in vagrant on my machine: I cannot run make test-in-vm even with original Vagrantfile from master, docker doesn't work properly, I reinstalled docker manually, and then install podman from dnf.

TomasTomecek commented 5 years ago

I tried to run tests in vagrant using the master branch and with this PR https://github.com/user-cont/conu/pull/307 they succeeded. Therefore, please rebase (especially after that PR is merged).

TomasTomecek commented 5 years ago

Unfortunately, I have some bad news for you: you've rebased your branch incorrectly and need to do it once more. Only your commits should sit on top of the commits from master branch.

Here are some good tutorials how to perform a rebase: https://github.com/edx/edx-platform/wiki/How-to-Rebase-a-Pull-Request https://www.digitalocean.com/community/tutorials/how-to-rebase-and-update-a-pull-request

Please let us know if you need help with this.

rpitonak commented 5 years ago

You need to rebase again because of #311, and then we can trigger CI build to see the results of podman tests.

fasashen commented 5 years ago

You need to rebase again because of #311, and then we can trigger CI build to see the results of podman tests.

@rpitonak Done.

TomasTomecek commented 5 years ago

[test]

TomasTomecek commented 5 years ago

Regarding the failed tests, we might need to run all the podman commands with:

podman --storage-driver=vfs

while testing.

I would suggest adding a list of default options to the podman backend and for every invocation of podman use those.

TomasTomecek commented 5 years ago

I'm playing with the test suite right now and trying to make it pass on rootless podman. I'll send a PR with the changes.

TomasTomecek commented 5 years ago

Current status:

=== 2 failed, 19 passed in 151.01 seconds ===

The missing 2 tests fail b/c rootless containers don't provide any networking info.

https://github.com/containers/libpod/issues/1453#issuecomment-440678730

TomasTomecek commented 5 years ago

@fasashen I created a PR for your branch: https://github.com/fasashen/conu/pull/1

We can merge and resolve CI and the networking problem with subsequent PRs.

@rpitonak WDYT?

rpitonak commented 5 years ago

I tried to run tests locally inside CentOS VM. I needed to do changes which Tomas suggested earlier in this conversation:

Regarding the failed tests, we might need to run all the podman commands with:

podman --storage-driver=vfs

while testing. I would suggest adding a list of default options to the podman backend and for every invocation of podman use those.

We should have a "command builder" method, where we can easily add default options. You can inspire here: https://github.com/user-cont/conu/blob/9d93556182ee6bd4f99fd63f60cd31b189da548a/conu/backend/origin/backend.py#L81-L89

fasashen commented 5 years ago

I tried to run tests locally inside CentOS VM. I needed to do changes which Tomas suggested earlier in this conversation:

Regarding the failed tests, we might need to run all the podman commands with:

podman --storage-driver=vfs

while testing. I would suggest adding a list of default options to the podman backend and for every invocation of podman use those.

We should have a "command builder" method, where we can easily add default options. You can inspire here: conu/conu/backend/origin/backend.py

Lines 81 to 89 in 9d93556

 def _oc_command(self, args): 
     """ 
     return oc command to run 

     :param args: list of str, arguments and options passed to oc binary 
     :return: list of str 
     """ 
     oc_command_exists() 
     return ["oc"] + args

Should we run it each time podman is called, or it's enough to run it once at the backed instance creation? For example, --storage-driver could be set once, but --log-level option should be passed each time.

TomasTomecek commented 5 years ago

We should only change the storage backend while testing, by default we should not fiddle with it (so that people can tinker it themselves).

Therefore I suggest to create a fixture for podman backend and use it in every podman test:

import pytest

@pytest.fixuture
def podman_backend():
  backend = PodmanBackend(logging_level=10)
  # set the storage option now
  yield backend
  #deinitialize backend now

and then

def test_something(podman_backend):
  podman_backend.ImageClass(...
TomasTomecek commented 5 years ago

I played with this a bit and managed to reach this:

22 passed in 269.14 seconds

https://github.com/fasashen/conu/pull/2

rpitonak commented 5 years ago

[test]

TomasTomecek commented 5 years ago

@rpitonak agreed

"Error adding network: operation not permitted" 

same thing happening for me locally, we can tinker that in next PR, so we don't overgrow this one or we can run tests inside a vagrant VM in the CI, up to us

merging

Thank you everyone for coop and thank you to @fasashen for your awesome contribution!

Tada