facts.server, operations.server: Make Mount fact and operation portable #1382
Conversation
Using `platform.system(...)` is not a good idea because it runs on the local machine and we need to take into account the operating system of the remote machine.
DtxdF
changed the title
| @@ -208,7 +208,7 @@ class Mounts(FactBase[Dict[str, MountsDict]]): | |||
|
|
|||
| @override | |||
| def command(self): | |||
| if platform.system() == "FreeBSD": | |||
| if host.get_fact(Kernel).strip() == "FreeBSD": | |||
There was a problem hiding this comment.
Unfortunately this won’t work (can’t call facts from facts). Will need to include the check condition within the command itself.
Also noting to self should make this error properly here, as shown by the failing test case.
There was a problem hiding this comment.
Somehow the check get_fact call does work though. It returns what it should on my remote FreeBSD machine:
pyinfra inventories/physical.py fact server.Mounts
--> Fact data for: server.Mounts
{
"xx.xx.xx.xx": {
"/": {
"device": "zroot/ROOT/FreeBSD",
"type": "zfs",
...
WIthout this change it returns an error:
[x] Loaded fact server.Os
[x] cat: /proc/self/mountinfo: No such file or directory
[x] Error: could not load fact: server.Mounts
Unintentional side effect?
There was a problem hiding this comment.
Interesting! Not intentional (which is why unit tests fail since they don’t mock this path), but this feels like something that would be very useful..
Need to have a proper think about the implications, but this would be a good addition I think.
There was a problem hiding this comment.
Yes, I had doubts about that change but I have the same result as @jgelens comments, it works. The other solution is to put a conditional check through sh(1) in the command(...) function but using pyinfra for this is better.
This is what happens when I run pyinfra (excerpt):
...
--> Gathering facts...
[control-centralita] Loaded fact server.Kernel
[control-centralita] Loaded fact server.Kernel
[control-centralita] Loaded fact server.Mounts
...
Using
platform.system(...)is not a good idea because it runs on the local machine and we need to take into account the operating system of the remote machine.3.xat this time)scripts/dev-test.sh)scripts/dev-lint.sh)