Skip to content

Some fixes for Docker library #6

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.

Sign up for GitHub

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

Open
awilfox wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Some fixes for Docker library #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09bda7f
Update ModuleInfo for post-0.2.0 work
awilfox May 27, 2025
6d9c7b2
Add Podman check to #running_in_container?
awilfox May 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/berkeley_library/docker.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module BerkeleyLibrary
module Docker
class << self
def running_in_container?
File.exist?('/.dockerenv') || init_cgroup_is_dockerish?
File.exist?('/.dockerenv') || init_cgroup_is_dockerish? || env_is_podmanish?
end

private
Expand All @@ -19,6 +19,10 @@ def init_cgroup_is_dockerish?
false
end
end

def env_is_podmanish?
ENV.fetch('container', '') == 'podman'
Copy link
Member

@danschmidt5189 danschmidt5189 Dec 2, 2025

Choose a reason for hiding this comment

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

I might be overthinking this, but from my reading of the podman-run documentation the contract is for the presence of a file rather than a particular environment variable:

Several files will be automatically created within the container… Additionally, a container environment file is created in each container to indicate to programs they are running in a container. This file is located at /run/.containerenv (or /var/run/.containerenv for FreeBSD containers).

I did see a few StackOverflow/similar comments referencing ENV.container, however the above was the only official-ish thing I could find. So I think what you have here works incidentally, but maybe isn't the actual contract. What do you think?

Copy link
Member Author

@awilfox awilfox Dec 2, 2025

Choose a reason for hiding this comment

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

The contract is there for systemd (cite comment from podman/podman#11836), but yes, it is probably safer to use the file specified in podman-run.1. Would you rather this be added as another File.exist? or keep it in a separate private method?

Copy link
Member

@danschmidt5189 danschmidt5189 Dec 2, 2025

Choose a reason for hiding this comment

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

Thanks for the systemd link. That actually is a bit more general:

To allow systemd (and other programs) to identify that it is executed within a container, please set the $container environment variable for PID 1 in the container to a short lowercase string identifying your implementation. With this in place the ConditionVirtualization= setting in unit files will work properly. Example: container=lxc-libvirt

I.e. it just has to be set, and doesn't necessarily have to be "podman". (Even for podman itself; there are some comments saying it's set differently in certain versions.)

So, I would probably make this one a bit more general (referencing the systemd spec), and if we really want to tighten up the podman check we could have a separate method checking for /run/.containerenv or /var/run/.containerenv. How does that sound?

Copy link
Member Author

@awilfox awilfox Dec 4, 2025

Choose a reason for hiding this comment

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

I think that approach makes more sense in general, but will drastically complicate the test code as it stands now.

I'm thinking we might be able to clean it up by testing each function separately (init_cgroup_is_dockerish?, has_podman_file?, has_container_env?) and then testing the running_in_docker? by simply mocking each one to return true/false, as in:

allow(Docker).to receive_messages(init_cgroup_is_dockerish?: true, has_podman_file?: false)

Though, that is still a lot of combinations, and will rapidly become even more unwieldy as we add more (i.e. #5).

end
end
end
end
18 changes: 10 additions & 8 deletions lib/berkeley_library/docker/module_info.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
# frozen_string_literal: true

module BerkeleyLibrary
module Docker
class ModuleInfo
NAME = 'berkeley_library-docker'.freeze
AUTHOR = 'Dan Schmidt'.freeze
AUTHOR_EMAIL = '[email protected]'.freeze
SUMMARY = 'Utility functions for Dockerizing Ruby apps'.freeze
DESCRIPTION = 'Utility functions for making Ruby apps "just work" in Docker containers.'.freeze
LICENSE = 'MIT'.freeze
VERSION = '0.2.0'.freeze
HOMEPAGE = 'https://github.com/BerkeleyLibrary/docker'.freeze
NAME = 'berkeley_library-docker'
AUTHOR = 'Dan Schmidt'
AUTHOR_EMAIL = '[email protected]'
SUMMARY = 'Utility functions for Dockerizing Ruby apps'
DESCRIPTION = 'Utility functions for making Ruby apps "just work" in Docker containers.'
LICENSE = 'MIT'
VERSION = '0.3.0.a1'
HOMEPAGE = 'https://github.com/BerkeleyLibrary/ruby-docker'

private_class_method :new
end
Expand Down
24 changes: 24 additions & 0 deletions spec/berkeley_library/docker_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,21 @@ module BerkeleyLibrary

it 'is true when /.dockerenv exists' do
mock_dockerenv
mock_podman_env(false)
expect(BerkeleyLibrary::Docker.running_in_container?).to be true
end

it 'is true when /proc/1/cgroup is docker-like' do
mock_dockerenv(false)
mock_init_cgroup
mock_podman_env(false)
expect(BerkeleyLibrary::Docker.running_in_container?).to be true
end

it 'is false when /proc/1/cgroup is traditional' do
mock_dockerenv(false)
mock_init_cgroup(false)
mock_podman_env(false)
expect(BerkeleyLibrary::Docker.running_in_container?).to be false
end

Expand All @@ -55,6 +58,21 @@ module BerkeleyLibrary
expect(File)
.to receive(:open).with('/proc/1/cgroup')
.and_raise(Errno::ENOENT)
mock_podman_env(false)
expect(BerkeleyLibrary::Docker.running_in_container?).to be false
end

it 'is true when container=podman is present in environment' do
mock_dockerenv(false)
mock_init_cgroup(false)
mock_podman_env(true)
expect(BerkeleyLibrary::Docker.running_in_container?).to be true
end

it 'is false when container=podman is not present in environment' do
mock_dockerenv(false)
mock_init_cgroup(false)
mock_podman_env(false)
expect(BerkeleyLibrary::Docker.running_in_container?).to be false
end

Expand All @@ -72,6 +90,12 @@ def mock_init_cgroup(dockerish = true)
.and_return(
StringIO.new(dockerish ? DOCKERISH_CGROUP : TRADITIONAL_CGROUP))
end

def mock_podman_env(exists = true)
allow(ENV)
.to receive(:fetch).with('container', '')
.and_return(exists ? 'podman' : '')
end
end
end
end