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

Change LX init environ to container=lxc-smartos #415

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

smokris
Copy link

@smokris smokris commented May 10, 2022

Summary

virt-what is a script to identify what type of virtualization is in use.

Running virt-what 1.20 on an LX zone outputs lxc — not exactly true, but close enough for our purposes.

However, running virt-what 1.21+ on an LX zone outputs nothing (i.e. bare-metal) — very wrong. ❌

What changed in virt-what

$ grep -A5 LXC virt-what-1.2*/virt-what.in
virt-what-1.20/virt-what.in:# Check for LXC containers
virt-what-1.20/virt-what.in-# http://www.freedesktop.org/wiki/Software/systemd/ContainerInterface
virt-what-1.20/virt-what.in-# Added by Marc Fournier
virt-what-1.20/virt-what.in-
virt-what-1.20/virt-what.in-if [ -e "${root}/proc/1/environ" ] &&
virt-what-1.20/virt-what.in-    cat "${root}/proc/1/environ" | tr '\000' '\n' | grep -Eiq '^container='; then
--
virt-what-1.21/virt-what.in:# Check for LXC containers
virt-what-1.21/virt-what.in-# http://www.freedesktop.org/wiki/Software/systemd/ContainerInterface
virt-what-1.21/virt-what.in-# Added by Marc Fournier
virt-what-1.21/virt-what.in-
virt-what-1.21/virt-what.in-if [ -e "${root}/proc/1/environ" ] &&
virt-what-1.21/virt-what.in-    cat "${root}/proc/1/environ" | tr '\000' '\n' | grep -Eiq '^container=lxc'; then

That is, previously, if PID 1's environment contained a variable named container, it would identify the system as LXC. This test matches SmartOS LX zones since lxinit.c sets container=zone.

But now it also checks that the value begins with lxc, so SmartOS LX zones no longer match.

Background

We use Puppet to manage our LX zones, and Puppet's Facter component calls virt-what to determine which rules to apply to the zones. A few days ago, CentOS Stream 8 updated virt-what to perform the container=lxc check. Puppet tries to apply bare-metal configuration to non-bare-metal systems, which wreaks havoc.

Fix

I propose changing SmartOS LX environment variable container=zone to container=lxc-smartos.

  • This addresses the above virt-what (and thus Puppet Facter) regression
  • systemd-detect-virt still returns the same container-other value (this was the reason SmartOS LX added the container=zone environment variable in the first place — 1, 2)

Test

✅ virt-what-1.20 + SmartOS 20220421T000508Z

# virt-what
lxc
# facter virtual
lxc
# systemd-detect-virt
container-other

❌ virt-what-1.21 + SmartOS 20220421T000508Z

# virt-what
(no output)
# facter virtual
physical
# systemd-detect-virt
container-other

✅ virt-what-1.21 + local build with this MR

# virt-what
lxc
# facter virtual
lxc
# systemd-detect-virt
container-other

@danmcd
Copy link

danmcd commented May 10, 2022

I don't supposed you can instead get virt-what to detect container=zone ?

@bahamat
Copy link
Member

bahamat commented May 10, 2022

It would be much better if facter can detect SmartOS zones properly. That being the case, I want to wait to see what happens with puppetlabs/facter#2492 before doing anything with this.

It would also be good to fix virt-what upstream so that it can properly detect SmartOS.

How does facter behave for native (joyent) brand zones currently? (virt-what currently does not detect joyent brand zones.)

@smokris
Copy link
Author

smokris commented May 10, 2022

I don't supposed you can instead get virt-what to detect container=zone ?

It would also be good to fix virt-what upstream so that it can properly detect SmartOS.

Agreed; I'm looking into that too. I'll switch this PR to draft status for now.

How does facter behave for native (joyent) brand zones currently?

On a base64 zone, facter's output looks good to me:

# uname -a
SunOS test 5.11 joyent_20220310T212952Z i86pc i386 i86pc illumos
# facter is_virtual
true
# facter virtual
zone
# facter hypervisors
{
  zone => {
    brand => "native",
    id => 5,
    ip_type => "excl",
    name => "a8a13f1a-…",
    uuid => "a8a13f1a-…"
  }
}
# facter os
{
  architecture => "i86pc",
  family => "Solaris",
  hardware => "i86pc",
  name => "Solaris",
  release => {
    full => "5.11",
    major => "5",
    minor => "11"
  }
}
# facter kernelversion
joyent_20220310T212952Z

@smokris smokris marked this pull request as draft May 10, 2022 19:32
@smokris
Copy link
Author

smokris commented May 10, 2022

@danmcd
Copy link

danmcd commented May 10, 2022

I submitted https://listman.redhat.com/archives/virt-tools-list/2022-May/017562.html to virt-what.

THANK YOU! I hope the folks up in virt-what are open and accepting of this. Please keep us informed. (Also, this should work for OmniOS's LX as well, and I'll let Andy F, Dominik, and friends know about this.)

@citrus-it
Copy link

(Also, this should work for OmniOS's LX as well, and I'll let Andy F, Dominik, and friends know about this.)

Thanks for the ping @danmcd, and thanks @smokris for doing the work here.

There are at least three illumos distributions that have LX zones, so perhaps it would be better to use illumos-lx for the string instead of smartos-lx?

@ikozhukhov
Copy link

ikozhukhov commented May 11, 2022

@citrus-it +1 for your proposal.
we have lx zones on DilOS too, but still not automated it as a service.
we have plans to do it later.

@smokris
Copy link
Author

smokris commented May 11, 2022

use illumos-lx for the string instead of smartos-lx?

Sure, that sounds good to me. I'll update the patches.

In the virt-what patch, I'm checking that the zone's PID 1 has environment variable container with value zone, and that /proc/version is BrandZ virtual linux — are both of those conditions also true for other Illumos distributions? (Or is there a better way to detect being inside an illumos-lx zone?)

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