napalm_ping incorrectly setting failed flag for unsuccessful pings

[davidban77]

Hello!

I have been working with napalm_ping module and noticed that it incorrectly specifies as failed: False to unsuccessful pings.

This is the returned output of 2 pings (one good and one bad), and you can see that both are marked as failed: False

[
  {
    'changed': False,
    'ping_results': {
      'success': {
        'probes_sent': 5,
        'packet_loss': 5,
        'rtt_min': 0.0,
        'rtt_max': 0.0,
        'rtt_avg': 0.0,
        'rtt_stddev': 0.0,
        'results': [

        ]
      }
    },
    'invocation': {
      'module_args': {
        'hostname': 'lab01',
        'username': 'xxx',
        'password': 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER',
        'dev_os': 'ios',
        'destination': '10.77.42.2',
        'provider': {
          'hostname': 'lab01',
          'username': None,
          'password': 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER',
          'timeout': 60,
          'dev_os': 'ios'
        },
        'timeout': 60,
        'optional_args': None,
        'source': None,
        'ttl': None,
        'ping_timeout': None,
        'size': None,
        'count': None,
        'vrf': None
      }
    },
    'failed': False,
    'item': {
      'ip': '10.77.42.2',
    },
    'ansible_loop_var': 'item'
  },
  {
    'changed': False,
    'ping_results': {
      'success': {
        'probes_sent': 5,
        'packet_loss': 0,
        'rtt_min': 5.0,
        'rtt_max': 10.0,
        'rtt_avg': 6.0,
        'rtt_stddev': 0.0,
        'results': [
          {
            'ip_address': '10.77.43.3',
            'rtt': 0.0
          },
          {
            'ip_address': '10.77.43.3',
            'rtt': 0.0
          },
          {
            'ip_address': '10.77.43.3',
            'rtt': 0.0
          },
          {
            'ip_address': '10.77.43.3',
            'rtt': 0.0
          },
          {
            'ip_address': '10.77.43.3',
            'rtt': 0.0
          }
        ]
      }
    },
    'invocation': {
      'module_args': {
        'hostname': 'lab01',
        'username': 'xxxx',
        'password': 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER',
        'dev_os': 'ios',
        'destination': '10.77.43.3',
        'provider': {
          'hostname': 'lab01',
          'username': None,
          'password': 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER',
          'timeout': 60,
          'dev_os': 'ios'
        },
        'timeout': 60,
        'optional_args': None,
        'source': None,
        'ttl': None,
        'ping_timeout': None,
        'size': None,
        'count': None,
        'vrf': None
      }
    },
    'failed': False,
    'item': {
      'ip': '10.77.43.3',
    },
    'ansible_loop_var': 'item'
  }
]

You can see that the probes sent and the packet loss is the same for the first one (100% packet loss).

I can take a look at the module and submit a PR if you want.

[ktbyers]

@davidban77 This is the expected behavior--you can definitely argue whether this should be the behavior or not.

In both cases the ping occurred i.e. the operation executed. In other words, failed is not tracking the number of ping responses, but whether the operation executed.

You would need to drill into the data structure and decide what constitutes failure in your context.

I am somewhat doubtful it is worth changing this since it would be a breaking change.

[davidban77]

Got it @ktbyers, thanks for the reply. I worked on the playbook by looking deeper into the data structure and comparing the probes and the packet loss.

I just would have thought that the task would have flagged the failed field when you have 100% packet loss.