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

Add support for the --cgroupns arg #400

Merged
merged 8 commits into from
Jan 29, 2023

Conversation

LewisGaul
Copy link
Collaborator

@LewisGaul LewisGaul commented Jan 18, 2023

See https://docs.docker.com/engine/reference/commandline/create/#options.

This arg was added in docker/cli#2024, v20.10.0 (link), and is also supported by podman.

I've tested this works manually, unsure if you think automated testing should somehow be added too?

@gabrieldemarmiesse
Copy link
Owner

Thanks a lot for the new feature! If it's possible to make a small test that would be the best. If the option can be seen with just a docker inspect on the container, that'd be great :)

@LewisGaul
Copy link
Collaborator Author

Good to be back with a contribution to one of my favourite open source projects!

I've added the cgroupns_mode field to the ContainerHostConfig (thanks for pointing that out). My concern is that if using a version of docker before v20.10.0 (or around then) when support for cgroupns was added, then will this give an error or will the field be set to 'None'? (I made the field Optional[str] in hope of the latter). I can test this out manually today.

@gabrieldemarmiesse
Copy link
Owner

You don't need the optional. This is because a decorator already put Optional around all types of the model automatically to ensure backward compatibility :)

@LewisGaul
Copy link
Collaborator Author

You don't need the optional. This is because a decorator already put Optional around all types of the model automatically to ensure backward compatibility :)

Oh ok! Confirmed this works:

>>> client2 = pow.DockerClient(client_call=["docker18"], host="ssh://vm2")
>>> ctr = client2.ps(all=True)[0]
>>> print(ctr.host_config.cgroupns_mode)
None

@LewisGaul
Copy link
Collaborator Author

The linux tests seem to be failing running wget on https://github.com/docker/compose/releases/download/v2.2.3/docker-compose-linux-x86_64 for some reason...

@gabrieldemarmiesse
Copy link
Owner

Yes, I think something was upgraded automatically in the ci. I'll take a look at it this weekend and I'll fix it. Sorry for the delay!

@LewisGaul
Copy link
Collaborator Author

The CI passed on my other PR - are you able to retrigger it on this PR?

@gabrieldemarmiesse
Copy link
Owner

This is a very strange error, I'm quite confused

@gabrieldemarmiesse gabrieldemarmiesse enabled auto-merge (squash) January 26, 2023 14:35
auto-merge was automatically disabled January 28, 2023 14:08

Head branch was pushed to by a user without write access

@LewisGaul
Copy link
Collaborator Author

I think the CI issue might've been caused by the test I added (container sleeps forever but is started attached!), hopefully I've fixed it now!

@gabrieldemarmiesse
Copy link
Owner

Nice! Thanks a lot! I think I'll make a new release once #407 has been fixed

@gabrieldemarmiesse gabrieldemarmiesse merged commit bcb0158 into gabrieldemarmiesse:master Jan 29, 2023
@LewisGaul LewisGaul deleted the cgroupns-arg branch January 29, 2023 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants