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

functions/detect_virt: use container env variable #9

Open
wants to merge 1 commit into
base: master
from

Conversation

@CameronNemo
Copy link
Contributor

CameronNemo commented Sep 28, 2018

No description provided.

Copy link

benaryorg left a comment

Mostly fine, except for it does not catch the case of container= (empty string as value), and I'm not sure about removing the check for existance of /proc/self/environ.

if grep -q lxc /proc/self/environ >/dev/null; then
export VIRTUALIZATION=1
fi
test -z "$container" || export VIRTUALIZATION=1

This comment has been minimized.

Copy link
@benaryorg

benaryorg Nov 19, 2019

This only checks whether the variable is not empty, not whether it is set.
I'd recommend using the (deleted) approach of grepping:

grep -zq ^container= /proc/self/environ 2>/dev/null && export VIRTUALIZATION=1

This comment has been minimized.

Copy link
@CameronNemo

CameronNemo Nov 19, 2019

Author Contributor

not gonna work with busybox

/ # busybox grep -z
grep: invalid option -- 'z'

are there any container runtimes that set the var to empty?

This comment has been minimized.

Copy link
@benaryorg

benaryorg Nov 19, 2019

On one hand I doubt so, on the other hand I've seen enough not to be particularly inclined to trust that assumption.
Anyways, which version of busybox are you using, the one I tested on my voidlinux installation and the one on my Gentoo box both have a -z flag.
The Gentoo box running BusyBox v1.31.1.

This comment has been minimized.

Copy link
@CameronNemo

CameronNemo Nov 19, 2019

Author Contributor
cameronnemo@cn-mbook ~> docker run --rm -it voidlinux/voidlinux sh
# xbps-install -Syu ; xbps-install -yu ; xbps-install -y busybox
[...]
# busybox grep -zq ^container= /proc/self/environ
grep: invalid option -- 'z'
BusyBox v1.30.1 (2019-08-02 18:59:22 UTC) multi-call binary.

Usage: grep [-HhnlLoqvsriwFE] [-m N] [-A/B/C N] PATTERN/-e PATTERN.../-f FILE [FILE]...

This comment has been minimized.

Copy link
@leahneukirchen

leahneukirchen Nov 20, 2019

Contributor

What's the purpose of this? Why not use [ "${container+x}" ]

This comment has been minimized.

Copy link
@benaryorg

benaryorg Nov 21, 2019

@leahneukirchen entirely forgot about that construct.
Right, that seems more reasonable.

functions Show resolved Hide resolved
functions Show resolved Hide resolved
@benaryorg

This comment has been minimized.

Copy link

benaryorg commented Nov 19, 2019

Also, I think if your comment starts with fix #6 it will automatically close the related #6 issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.