Junos: pass self.ignore_warning in compare_config() to match behavior of commit_config()

[ekoyle]

Pass ignore_warning to device.cu.diff, since the API accepts it.

Edit: On devices with ignore_warning set, a diff with warnings will raise an exception, even though a commit will succeed. This change allows the diff to succeed in cases a commit would. There is a similar issue with discard/rollback, however PyEZ does not currently accept ignore_warning for the rollback call.

[ktbyers]

No, we would need an argument that was passed in via optional_args on init. The default would need to be False i.e. the warnings would not be ignored.

I don't think it is proper to have it just ignore the warning by default here (since the thing it is warning about is pretty dangerous: changing a set of ports including other ports to a different potentially non-working speed). I.E. ports beyond what you explicitly configured are being changed.

Also, we would need a fix for the rollback issue on ignore_warnings. At a minimum gracefully catching the exception that Junos generates (since the rollback does in fact go through).

Also the optional_args argument should probably be more specific than just ignore_warnings i.e. that is really generic and would be easy to misconstrue.

Regards, Kirk

[ekoyle]

It is currently using the ignore_warning option, which is an optional_arg that defaults to False on the Juniper driver.

This is for the compare_config function. Ironically, napalm is already passing ignore_warning through when you call commit_config -- so if I were committing without doing a diff, I wouldn't get this exception.

PyEZ's ignore_warning arg can actually be a string or list of strings that are used with regex matching to exclude specific warnings. Napalm is passing the argument through and it is only ignoring the specific warning that I'm expecting.

I agree that the name is not great.

Setting the "ignore_*" at the driver level has always made me a bit uncomfortable -- usually when we get warnings, it's only for a specific operation where we are expecting the issue. I worry that someone will accidentally re-use the connection to do something else before or after the critical step(s). An argument to the function seems like it would be safer, but I think that goes against the design of napalm.

[ktbyers]

@ekoyle Okay, good point. I didn't realize there was an already existing ignore_warning that was used for commit_config.

Yeah, in order to add an argument for the method, we would need to add the argument to all of the NAPALM core drivers and possibly need to retrofit additional tools that are built on top of NAPALM (like napalm-ansible for example). I think adding it as init argument is probably better here (with default to False so you have to explicitly say you are okay with ignoring the warnings).

[ekoyle]

@dbarrosop can you take a look at this since you merged the ignore_warning change in #389?

[ktbyers]

Please don't ping other individuals directly. It is not really fair to them.

It will likely be either myself or Mircea reviewing this change, most likely me since I already started working on it.

[mirceaulinic]

Very quickly, I think I'm fine to reuse the same ignore_warnings - and if we'll feel like we need more specific (i.e., per scope / method) we can add them later for more granularity. I somewhat see ignore_warnings as ignore all the warnings. But I don't have any strong views either way. :-)

[ekoyle]

If py-junos-eznc pull 1131 gets merged, we could make a similar change for the rollback() in discard_config() with a future version of pyez.

For reference, setting ignore_warning to ["25g config will be applied to ports"] seems to have the desired effect of only ignoring the warnings that match the regex.

[mirceaulinic]

This looks good to me.