Repository: ansible/ansible
Pull request: setup: properly detect is_chroot on Btrfs
##### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
Fixes #55006
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Bugfix Pull Request

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below -->
setup
systemd

Problem statement:
is_chroot fails on btrfs for non-root user
##### SUMMARY
When the `setup` module runs with non-root privileges and the root filesystem is on a btrfs (sub-)volume, the `is_chroot` fact is always true, which obviously, is not always correct.
This is due to the the fallback code in [line 25 of `module_utils/facts/system/chroot.py`](../blob/devel/lib/ansible/module_utils/facts/system/chroot.py#L25), which assumes that (only) the real `/` has inode 2. This is not the case for btrfs, where all (sub-)volume roots seem to have inode 256.
To be clear: I correctly get `is_chroot = false` when running `setup` as `root`.

I suspect that, when `setup` is run *with* root privileges, a chroot in the root of a different subvolume than the `/` (sub-)volume on the same device may cause a false negative for `is_chroot`. This may sounds like an academic setup, but I would probably put every chroot I had on it's own subvolume. So, this may actually be quite common when using chroots on btrfs.
In any case, I did not try this second part, it's just my understanding of the code.

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
module_utils/facts/system/chroot.py

##### ANSIBLE VERSION
```
ansible 2.7.8
  config file = None
  configured module search path = ['/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib64/python3.6/site-packages/ansible
  executable location = /usr/lib/python-exec/python3.6/ansible
  python version = 3.6.5 (default, Apr 22 2018, 15:17:10) [GCC 6.4.0]
```
I also tried latest `devel` (0ff689a315), to the same effect.

##### CONFIGURATION
default configuration

##### OS / ENVIRONMENT
The target runs Linux kernel 4.19.27.

##### STEPS TO REPRODUCE
1. Set up a system using btrfs for `/`
2. Run the `setup` module
3. Watch for `is_chroot`

##### EXPECTED RESULTS
`is_chroot` is false

##### ACTUAL RESULTS
`is_chroot` is true

Hints from issue discussion:
Files identified in the description:
* [lib/ansible/module_utils/facts/system/chroot.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/facts/system/chroot.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->

Commit message: Use get_bin_path for stat

Patch to review:
diff --git a/changelogs/fragments/55006-setup-is_chroot-btrfs.yaml b/changelogs/fragments/55006-setup-is_chroot-btrfs.yaml
new file mode 100644
index 00000000000000..b47741bdc1cb1f
--- /dev/null
+++ b/changelogs/fragments/55006-setup-is_chroot-btrfs.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+  - setup - properly detect is_chroot on Btrfs (https://github.com/ansible/ansible/issues/55006)
diff --git a/lib/ansible/module_utils/facts/system/chroot.py b/lib/ansible/module_utils/facts/system/chroot.py
index 5eb89d695aa55c..96db3dc62f4e2d 100644
--- a/lib/ansible/module_utils/facts/system/chroot.py
+++ b/lib/ansible/module_utils/facts/system/chroot.py
@@ -8,7 +8,7 @@
 from ansible.module_utils.facts.collector import BaseFactCollector
 
 
-def is_chroot():
+def is_chroot(module=None):
 
     is_chroot = None
 
@@ -22,7 +22,18 @@ def is_chroot():
             is_chroot = my_root.st_ino != proc_root.st_ino or my_root.st_dev != proc_root.st_dev
         except Exception:
             # I'm not root or no proc, fallback to checking it is inode #2
-            is_chroot = (my_root.st_ino != 2)
+            fs_root_ino = 2
+
+            if module is not None:
+                # on Btrfs, the first-created subvolume (root) has inode #256
+                stat_path = module.get_bin_path('stat')
+                if stat_path:
+                    cmd = [stat_path, '-f', '--format=%T', '/']
+                    rc, out, err = module.run_command(cmd)
+                    if 'btrfs' in out:
+                        fs_root_ino = 256
+
+            is_chroot = (my_root.st_ino != fs_root_ino)
 
     return is_chroot
 
@@ -32,4 +43,4 @@ class ChrootFactCollector(BaseFactCollector):
     _fact_ids = set(['is_chroot'])
 
     def collect(self, module=None, collected_facts=None):
-        return {'is_chroot': is_chroot()}
+        return {'is_chroot': is_chroot(module)}
diff --git a/lib/ansible/modules/system/systemd.py b/lib/ansible/modules/system/systemd.py
index a2d2d7d2eae58a..10c9e42f253525 100644
--- a/lib/ansible/modules/system/systemd.py
+++ b/lib/ansible/modules/system/systemd.py
@@ -508,7 +508,7 @@ def main():
                         if rc != 0:
                             module.fail_json(msg="Unable to %s service %s: %s" % (action, unit, err))
             # check for chroot
-            elif is_chroot():
+            elif is_chroot(module):
                 module.warn("Target is a chroot. This can lead to false positives or prevent the init system tools from working.")
             else:
                 # this should not happen?


Code review comments:
diff_hunk: @@ -22,7 +22,18 @@ def is_chroot():
             is_chroot = my_root.st_ino != proc_root.st_ino or my_root.st_dev != proc_root.st_dev
         except Exception:
             # I'm not root or no proc, fallback to checking it is inode #2
-            is_chroot = (my_root.st_ino != 2)
+            fs_root_ino = 2
+
+            if module is not None:
+                # on Btrfs, the first-created subvolume (root) has inode #256
path: lib/ansible/module_utils/facts/system/chroot.py
text: @user1:
On my systems, `stat` shows inode 256 for all subvolumes, not only the first created. Moreover, any subvolume may be mounted on `/`, not only the first one.

@author:
Hrm, not sure what I meant by that, removed. Thanks.