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

fix(Containers resolver): properly detect Docker runtime in containers (#2781) #2786

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

molhamalnasr
Copy link

Previously, the Containers resolver would always return a value from read_environ, preventing read_cgroup from being called. As a result, the hypervisors and virtual facts were often set incorrectly to container_other and displayed a misleading warning.

Changes in this commit:

  • read_cgroup now returns early unless Docker or LXC matches are found.
  • read_environ returns nil when encountering an unsupported container.
  • If both methods return nil, vm defaults to container_other with a warning.

These updates ensure that Facter correctly detects Docker and properly reports the virtual and hypervisors facts when running inside a Docker container.

issue: (#2781)

puppetlabs#2781)

Previously, the `Containers` resolver would always return a value from
`read_environ`, preventing `read_cgroup` from being called. As a result,
the `hypervisors` and `virtual` facts were often set incorrectly to
`container_other` and displayed a misleading warning.

Changes in this commit:
- `read_cgroup` now returns early unless Docker or LXC matches are found.
- `read_environ` returns `nil` when encountering an unsupported container.
- If both methods return `nil`, `vm` defaults to `container_other` with a warning.

These updates ensure that Facter correctly detects Docker and properly
reports the `virtual` and `hypervisors` facts when running inside a
Docker container.
@molhamalnasr molhamalnasr requested a review from a team as a code owner January 14, 2025 10:35
@puppetlabs-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@CLAassistant
Copy link

CLAassistant commented Jan 14, 2025

CLA assistant check
All committers have signed the CLA.

@lollipopman
Copy link
Contributor

looks good to me, thanks

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.

5 participants