napalm-automation / napalm-junos

Apache License 2.0
22 stars 42 forks source link

Workaround for get-environment-information not providing class data f… #195

Closed bkeifer closed 7 years ago

bkeifer commented 7 years ago

…or each entry.

The get-environment-information RPC output corresponds to the "show chassis environment" CLI command. The output from this command in a multi-chassis EX switch environment is as follows:

> show chassis environment
Class Item                           Status     Measurement
Power FPC 0 Power Supply 0           OK
      FPC 1 Power Supply 0           OK
      FPC 2 Power Supply 0           OK
Temp  FPC 0 CPU Sensor               OK         37 degrees C / 98 degrees F
      FPC 0 PSU Sensor               OK         30 degrees C / 86 degrees F
      FPC 1 CPU Sensor               OK         38 degrees C / 100 degrees F
      FPC 1 PSU Sensor               OK         30 degrees C / 86 degrees F
      FPC 2 CPU Sensor               OK         44 degrees C / 111 degrees F
      FPC 2 PSU Sensor               OK         35 degrees C / 95 degrees F
Fans  FPC 0 Fan Tray 0               OK         Spinning at normal speed
      FPC 0 Fan Tray 1               OK         Spinning at normal speed
      FPC 1 Fan Tray 0               OK         Spinning at normal speed
      FPC 1 Fan Tray 1               OK         Spinning at normal speed
      FPC 2 Fan Tray 0               OK         Spinning at normal speed

That's all well and good on the command line, but the RPC output seems to correspond to the CLI output a bit too faithfully. PrettyPrinting from within the napalm-junos module reveals the structure:

[   (   'FPC 0 Power Supply 0',
        [('status', 'OK'), ('class', 'Power'), ('temperature', None)]),
    (   'FPC 1 Power Supply 0',
        [('status', 'OK'), ('class', None), ('temperature', None)]),
    (   'FPC 2 Power Supply 0',
        [('status', 'OK'), ('class', None), ('temperature', None)]),
    (   'FPC 0 CPU Sensor',
        [('status', 'OK'), ('class', 'Temp'), ('temperature', 37)]),
    (   'FPC 0 PSU Sensor',
        [('status', 'OK'), ('class', 'Temp'), ('temperature', 29)]),
    (   'FPC 1 CPU Sensor',
        [('status', 'OK'), ('class', None), ('temperature', 38)]),
    (   'FPC 1 PSU Sensor',
        [('status', 'OK'), ('class', None), ('temperature', 30)]),
    (   'FPC 2 CPU Sensor',
        [('status', 'OK'), ('class', None), ('temperature', 43)]),
    (   'FPC 2 PSU Sensor',
        [('status', 'OK'), ('class', None), ('temperature', 35)]),
    (   'FPC 0 Fan Tray 0',
        [('status', 'OK'), ('class', 'Fans'), ('temperature', None)]),
    (   'FPC 0 Fan Tray 1',
        [('status', 'OK'), ('class', None), ('temperature', None)]),
    (   'FPC 1 Fan Tray 0',
        [('status', 'OK'), ('class', None), ('temperature', None)]),
    (   'FPC 1 Fan Tray 1',
        [('status', 'OK'), ('class', None), ('temperature', None)]),
    (   'FPC 2 Fan Tray 0',
        [('status', 'OK'), ('class', None), ('temperature', None)])]

Note that each power supply, temperature sensor, or fan after those in the first chassis are given a 'class' of None, likely taken straight from the human-readable output of the CLI command.

This patch modifies the behavior of the main loop within get_environment() so that if a sensor object is encountered with a 'class' of None, the previous valid class is used.

This works on all models of EX switch that I have access to test against. Testing against other JunOS platforms is likely warranted.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.06%) to 83.556% when pulling bb5ab5dae35df1d1139473b0495194c48d42d032 on bkeifer:develop into e6519c6dcde1e502db4b4ddb1d329ece05de7aea on napalm-automation:develop.

dbarrosop commented 7 years ago

Would you mind adding tests? Just add an extra folder here with the relevant mocked data:

https://github.com/napalm-automation/napalm-junos/tree/develop/test/unit/mocked_data/test_get_environment

bkeifer commented 7 years ago

Getting sucked into a meeting. Will take care of the build errors later.

mirceaulinic commented 7 years ago

Getting sucked into a meeting.

Have fun @bkeifer

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-4.7%) to 78.958% when pulling 6d5b4641d9848b38690ab5bdfade49f79bdf60ba on bkeifer:develop into e6519c6dcde1e502db4b4ddb1d329ece05de7aea on napalm-automation:develop.

bkeifer commented 7 years ago

I'm back to the original conditional logic. Here's why:

This line, suggested by @dbarrosop will update the object with the correct class name. structured_object_data['class'] = structured_object_data['class'] or current_class

However, since Juniper only gives us a 'class' for the first instance of an object of that class, we also need to track when we see a new class name to let us know that the following data is about a different class of object. That's where the else: and current_class comes in.

I've added some comments to hopefully explain whats going on a bit better, and Travis seems happier now.