Validate SimpleInventory - possible or implementation wanted?

[bytinbit]

Dear all,

I stumbled upon this potential issue, and before I open one, I ask here: I tested my project-setup that uses nornir with a SimpleInventory and wondered why everything suddenly failed until I noticed that my test-SimpleInventory was lacking fields/files.

I reproduced the issue by creating a SimpleInventory that lacked groups.yaml and defaults.yaml, and only has inventory/hosts.yaml:

>>> n = InitNornir(config_file="nr-config.yaml", logging={"enabled": False})
>>> n.inventory.dict()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/bytinbit/proj/myproject/.venv/lib/python3.8/site-packages/nornir/core/inventory.py", line 654, in dict
    "hosts": {n: h.dict() for n, h in self.hosts.items()},
  File "/home/bytinbit/proj/myproject/.venv/lib/python3.8/site-packages/nornir/core/inventory.py", line 654, in <dictcomp>
    "hosts": {n: h.dict() for n, h in self.hosts.items()},
  File "/home/bytinbit/proj/myproject/.venv/lib/python3.8/site-packages/nornir/core/inventory.py", line 310, in dict
    **super().dict(),
  File "/home/bytinbit/proj/myproject/.venv/lib/python3.8/site-packages/nornir/core/inventory.py", line 157, in dict
    "groups": [g.name for g in self.groups],
  File "/home/bytinbit/proj/myproject/.venv/lib/python3.8/site-packages/nornir/core/inventory.py", line 157, in <listcomp>
    "groups": [g.name for g in self.groups],
AttributeError: 'str' object has no attribute 'name'

(it works if the inventory is correctly set up)

I then looked into the code (https://github.com/nornir-automation/nornir/blob/develop/nornir/plugins/inventory/simple.py) and noticed there is no error handling that deals with non-existing files or a potentially erroneous config structure.

Is there a way for me to validate a SimpleInventory for correctness? If not, would it be welcomed to add some kind of error handling and/or validation to the SimpleInventory code? I'd be happy to help contributing, but since this would be my first contribution, I want to be sure I did not miss anything and hear your opinions first.

As always: thanks for your work and answers.

Edit: Running napalm_get on that erroneous SimpleInventory leads to this error - it would be helpful to know that it failed because of the wrong inventory structure:

Host 'R3': task 'napalm_get' failed with traceback:
Traceback (most recent call last):
  File "/home/bytinbit/myproject/.venv/lib/python3.8/site-packages/nornir/core/task.py", line 99, in start
    r = self.task(self, **self.params)
  File "/home/bytinbit/myproject/.venv/lib/python3.8/site-packages/nornir_napalm/plugins/tasks/napalm_get.py", line 32, in napalm_get
    device = task.host.get_connection(CONNECTION_NAME, task.nornir.config)
  File "/home/bytinbit/myproject/.venv/lib/python3.8/site-packages/nornir/core/inventory.py", line 493, in get_connection
    conn = self.get_connection_parameters(connection)
  File "/home/bytinbit/myproject/.venv/lib/python3.8/site-packages/nornir/core/inventory.py", line 427, in get_connection_parameters
    r = self._get_connection_options_recursively(connection)
  File "/home/bytinbit/myproject/.venv/lib/python3.8/site-packages/nornir/core/inventory.py", line 456, in _get_connection_options_recursively
    sp = g._get_connection_options_recursively(connection)
AttributeError: 'str' object has no attribute '_get_connection_options_recursively'

Edit: Colorized console output

[dbarrosop]

Hello,
would you mind sharing a broken inventory and a sample code to reproduce? What you mention is quite strange as missing files is supported. Only hosts.yaml is mandatory and it's handled accordingly:

https://github.com/nornir-automation/nornir/blob/develop/nornir/plugins/inventory/simple.py#L97-L102

(same for groups.yaml)

Anyway, before deciding next steps I'd like to see first hand the error to see what's the actual problem (missing files isn't).

I suspect though the problem might be that you assigned groups to hosts but groups.yaml doesn't exist leading to this weird behavior.

Thanks for taking the time reporting the issue.

[bytinbit]

The folder structure is as follows, and the error appears if I run the code out of root .:

.
├── inventory
│   └── hosts.yaml
├── nr-config.yaml

Here's the hosts file, shortened and redacted (there's more hosts but that's not the issue):

R1:
  hostname: 1.2.3.1
  data:
    interfaces:
      - name: GigabitEthernet2
        ipv4_address: 5.6.7.1
      - name: GigabitEthernet3
        ipv4_address: 5.6.7.2
      - name: GigabitEthernet4
        ipv4_address: 5.6.7.3
      - name: Loopback0
        ipv4_address: 5.6.7.4
  groups:
    - routers

R2:
  hostname: 1.2.3.2
  data:
    interfaces:
      - name: GigabitEthernet2
        ipv4_address: 5.6.7.5
      - name: GigabitEthernet3
        ipv4_address: 5.6.7.6
      - name: GigabitEthernet4
        ipv4_address: 5.6.7.7
      - name: Loopback0
        ipv4_address: 5.6.7.8
  groups:
    - routers

R3:
  hostname: 1.2.3.3
  data:
    interfaces:
      - name: GigabitEthernet2
        ipv4_address: 5.6.7.9
      - name: GigabitEthernet3
        ipv4_address: 5.6.7.10
      - name: Loopback0
        ipv4_address: 5.6.7.11
  groups:
    - routers

And here's nr-config.yaml:

inventory:
    plugin: SimpleInventory
    options:
        host_file: "inventory/hosts.yaml"
        group_file: "inventory/groups.yaml"
        defaults_file: "inventory/defaults.yaml"
runner:
    plugin: threaded
    options:
        num_workers: 100
logging:
    to_console: true

So obviously the defaults and the groups are missing in the folder, which are referenced nevertheless in nr-config.yaml, but the resulting error did not tell me that my SimpleInventory was erroneous, which would have been good to know. That's why I had the idea/input that it would be nice to have something like a validator telling me "hey, there are errors in your SimpleInventory, can't run nornir" or something like that, instead of the rather opqaue error messages.

I think someone else here had a similar problem but never came back to it: #611

[dbarrosop]

The problem is that you are using groups but have no groups.yaml. In any case, I agree this is a bug. Given that having no defaults.yaml or groups.yaml is supported I don't think there is much we can do there, however, the following code:

https://github.com/nornir-automation/nornir/blob/develop/nornir/plugins/inventory/simple.py#L119-L120

Should be outside of this block (right after it). Do you want to give it a try? That will give you an error as it tries to resolve the groups you define in the hosts but that don't exist as real groups.

[bytinbit]

I'll try and hope to contribute :) Should I open an issue?
As an afterthought, https://json-schema.org/ might be helpful to check the validity of SimpleInventory, there are already several Python implementations: https://json-schema.org/implementations.html#validator-python

[dbarrosop]

Should I open an issue?

as you wish, if you plan to open a PR soon-ish I'd say opening the PR directly is enough, otherwise we can open an issue so we don't forget about this.

might be helpful to check the validity of SimpleInventory

The SimpleInventory here is meant to be minimal working code so I'd say to do the bare minimum (specially if adding things requires extra dependencies). For more complex features we have a YAMLInventory plugin here (which, btw, is a copy of SimpleInventory and has the same bug). So if we want to look into adding json-schema (which I am good with) we should do it there.

[bytinbit]

Great, thanks for the quick reply! I just started working on it and see if I can fix the bug there too.

[bytinbit]

To make a back-reference, the issues have been fixed in the PRs here:
#677

nornir-automation/nornir_utils#11