search

YangModels / yang

YANG modules from standards organizations such as the IETF, The IEEE, The Metro Ethernet Forum, open source such as Open Daylight or vendor specific modules
1.51k stars 1.2k forks source link

Avalanche of errors and warnings in Cisco Yang Modules when converted to tree format with pyang #139

Open jean-christophe-manciot opened 7 years ago

jean-christophe-manciot commented 7 years ago

@einarnn I've discovered many issues when trying to convert all Cisco Yang modules to tree format with pyang -f tree(pyang 1.7.1):

Also, other errors appear when it is set at cisco/xr/<xr_version>.

I'm not even listing the ton of warnings for unnecessary imports regarding modules across the board.

The whole log is available here when search_path="cisco/\<nos>". The whole log is available here when search_path="cisco/\<nos>/\<nos_version>".

FYI, all latest official openconfig Yang modules (https://github.com/openconfig/public) pass the same conversion with flying colors.

einarnn commented 7 years ago

Jean-Christophe,

Sorry for the delay in replying.

First, I'd like to say that there are a number of error reported from running pyang over Cisco models when certain options are used and when certain libraries are also present.

May I ask if in your Python environment you have also installed lxml? Or another XML parser that pyang can utilize? When such a library is present, pyang implements more validation (e.g. trapping regex errors). Sadly, when I put together the CI scripts I did not understand this dependency as it was something pyang didn't express in its module dependencies, instead choosing to warp the import in a try, and silently ignoring a missing XML library. See here:

https://github.com/mbj4668/pyang/blob/071193490467dcc1d862d51f6feaa28b9f6b3671/pyang/types.py#L429-L465

By the time I realized this, we had too many modules, not just Cisco modules, including IETF ones, that generated errors, and I chose to NOT install lxml or libxml2 in .travis.yml for the YangModels/yang repository as that would have broken the CI build almost completely, with no ability to fix ALL the models that would have broken as a result.

As I said, these errors are, in general, known, and we are working to address them. In each directory that contains Cisco models, we have also included a shell script, "check-models.sh", that filters out a number of warnings/errors when compiling the models. The reason for the inclusion of this script was to show developers explicitly what we consider to be non-fatal warnings or errors reported by pyang. The reasoning behind filtering out certain warnings and error is that the ones we are ignoring do not impact code generation or the processing related to, for example, generating YANG tree output.

If I have this virtualenv:

(pyang) ✔ /opt/git-repos
17:16 $ pip freeze
appdirs==1.4.3
packaging==16.8
pyang==1.7.1
pyparsing==2.2.0
six==1.10.0

And then invoke pyang -f tree *.yang in the directory vendor/cisco/xr/621, then the only errors generated are for unused imports. In contrast, if I then install lxml, I do, indeed, encounter an avalanche or errors and warnings.

FYI, all latest official openconfig Yang modules (https://github.com/openconfig/public) pass the same conversion with flying colors.

Yep, I'm aware of this. Unfortunately, we do not yet implement the latest models on our platforms, and originally I chose to include the unmodified openconfig models, rather than providing vendor-modified models. This means that there are some errors not present in the latest revisions. However, I have manually edited the openconfig models in the latest release, vendor/cisco/xr/621, to compile correctly. Unfortunately, if you download the openconfig models from an XR platform, you will get ones that are exactly as openconfig provided them, which means there will be compilation errors.

In conclusion, I recommend that when compiling the Cisco native models for tree output, please use a restricted environment for pyang that has neither lxml nor libxml2 installed and do not use options such as --ietf or --lint.

Apart from mass editing of models (I like to try and keep what is in git as close to what the devices themselves will allow via the <get-schema> RPC), do you have any other suggestions? As we move forward, we are trying to tighten up on these errors and make things better, but we will not be retrospectively fixing models at this time.

Cheers,

Einar

jean-christophe-manciot commented 7 years ago

May I ask if in your Python environment you have also installed lxml? Or another XML parser that pyang can utilize?

# pip show lxml
Name: lxml
Version: 3.7.3

do you have any other suggestions?

Aim for simplicity, simplicity, simplicity and trickle down that advice to all Yang modules developers on all Cisco platforms (NX-OS, IOS-XE, IOS-XR). For instance, who can really easily troubleshoot that kind of regular expression: 

'^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])$|^(([a-zA-Z]|[a-zA-Z][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z]|[A-Za-z][A-Za-z0-9\-]*[A-Za-z0-9])$|^(?:(?:(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):){6})(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:::(?:(?:(?:[0-9a-fA-F]{1,4})):){5})(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})))?::(?:(?:(?:[0-9a-fA-F]{1,4})):){4})(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):){0,1}(?:(?:[0-9a-fA-F]{1,4})))?::(?:(?:(?:[0-9a-fA-F]{1,4})):){3})(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):){0,2}(?:(?:[0-9a-fA-F]{1,4})))?::(?:(?:(?:[0-9a-fA-F]{1,4})):){2})(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):){0,3}(?:(?:[0-9a-fA-F]{1,4})))?::(?:(?:[0-9a-fA-F]{1,4})):)(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):){0,4}(?:(?:[0-9a-fA-F]{1,4})))?::)(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):){0,5}(?:(?:[0-9a-fA-F]{1,4})))?::)(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):){0,6}(?:(?:[0-9a-fA-F]{1,4})))?::))))$'

And do you really expect people writing these things to do so error free?

The more complex your products are, the more expensive and harder to maintain & troubleshoot they are. Unfortunately, you cannot easily attain simplicity when you start at an extreme level of complexity. So the road will be very long, if you even consider taking it. May I remind you the words of your CEO, Chuck Robbins, which are already almost 2 years old?

"What we do is incredibly complicated, but we need to simplify everything," he said. "We need to simplify all that we do internally and externally."

Otherwise, your company will continue to lose its market shares.

einarnn commented 7 years ago

May I ask if in your Python environment you have also installed lxml? Or another XML parser that pyang can utilize?

# pip show lxml
Name: lxml
Version: 3.7.3

In that case, per my initial response, I suggest that while you are generating trees (a process which is not sensitive to the bulk of errors generated by pyang) that you either filter errors out (similar to what I do in the check-models.h scripts I include or that you create a Python virtualenv without lxml installed.

I realise this is just a workaround, not a fix, but it is all I can suggest right now. Also, Cisco will not be going back and retrospectively fixing the existing models in GitHub as they are, for the most part, what our devices actually return when invoking a <get-schema> RPC.

do you have any other suggestions?

Aim for simplicity, simplicity, simplicity and trickle down that advice to all Yang modules developers on all Cisco platforms (NX-OS, IOS-XE, IOS-XR).

For instance, who can really easily troubleshoot that kind of regular expression: 

'^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])$|^(([a-zA-Z]|[a-zA-Z][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z]|[A-Za-z][A-Za-z0-9\-]*[A-Za-z0-9])$|^(?:(?:(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):){6})(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:::(?:(?:(?:[0-9a-fA-F]{1,4})):){5})(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})))?::(?:(?:(?:[0-9a-fA-F]{1,4})):){4})(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):){0,1}(?:(?:[0-9a-fA-F]{1,4})))?::(?:(?:(?:[0-9a-fA-F]{1,4})):){3})(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):){0,2}(?:(?:[0-9a-fA-F]{1,4})))?::(?:(?:(?:[0-9a-fA-F]{1,4})):){2})(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):){0,3}(?:(?:[0-9a-fA-F]{1,4})))?::(?:(?:[0-9a-fA-F]{1,4})):)(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):){0,4}(?:(?:[0-9a-fA-F]{1,4})))?::)(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):){0,5}(?:(?:[0-9a-fA-F]{1,4})))?::)(?:(?:[0-9a-fA-F]{1,4})))|(?:(?:(?:(?:(?:(?:[0-9a-fA-F]{1,4})):){0,6}(?:(?:[0-9a-fA-F]{1,4})))?::))))$'

And do you really expect people writing these things to do so error free?

It is perhaps worth understanding that most of the "native" models Cisco provides, including the one you took the example above from, are actually auto-generated from a Cisco-proprietary model-based representation. As such, while admittedly very complex, the regular expression is not hand-authored. We are trying to remove the manual elements where possible, and IOS-XR and NX-OS are two operating systems where we have model-based infrastructure in place and where those models may be transformed to YANG models. This means that when we fix issues, like compliance to RFC 6087 guidelines (something that causes a lot of pyang errors & warnings with options like --ietf and --lint but does not cause problems with, say, code auto-generation), then we see the quality of our native models take a step-change improvement.

The more complex your products are, the more expensive and harder to maintain & troubleshoot they are. Unfortunately, you cannot easily attain simplicity when you start at an extreme level of complexity. So the road will be very long, if you even consider taking it. May I remind you the words of your CEO, Chuck Robbins, which are already almost 2 years old?

"What we do is incredibly complicated, but we need to simplify everything," he said. "We need to simplify all that we do internally and externally."

And this is exactly why Cisco, along with other vendors, are trying to adopt well-defined YANG data models and, where possible, open & non-proprietary models from bodies such at the IETF, OpenConfig, IEEE, MEF and ITU-T. And we started this journey in Cisco well before the quote above from Chuck Robbins!

Cisco has 25+ years of coding sitting behind it, and while newer code can be dealt with more easily, bringing our pre-existing code base to the point where it meets modern, model-based standards, takes time. I hope you, and other developers, will start to see improvements over time and will stay engaged and continue providing constructive feedback.

The vendor community, which Cisco is part of, is trying to do better!

jean-christophe-manciot commented 7 years ago

@einarnn When trying to experiment with Yang models on latest NX-OSv-9k, I stumbled across another issue: where should I report it so that there is a chance it will be addressed and solved or at least worked around?

einarnn commented 7 years ago

Jean-Christophe,

I've drawn the NX-OS engineering team's attention to that issue and I'll follow up with the.

Cheers,

Einar

On 27 Apr 2017, 05:53 -0700, Jean-Christophe Manciot notifications@github.com, wrote:

@einarnn (https://github.com/einarnn) When trying to experiment with Yang models on latest NX-OSv-9k, I stumbled across another issue (https://supportforums.cisco.com/discussion/13283641/nx-os-9k-cannot-install-component-rpm-packages-due-missing-dependencies): where should I report it so that there is a chance it will be addressed?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub (https://github.com/YangModels/yang/issues/139#issuecomment-297704635), or mute the thread (https://github.com/notifications/unsubscribe-auth/AFKGcGHcOziSAqkMhYJk0tiibHsaotpHks5r0I-vgaJpZM4M80Gj).

jean-christophe-manciot commented 7 years ago

@einarnn Thanks. Now, another cisco open source project - Yang Explorer - has serious issues and no one has been answering for the last 3 weeks as far as I'm concerned, and since last... November as far as other people are concerned::