ADVA OS ConfigParser Support #747
Conversation
architekture
commented
Dec 16, 2025
architekture
commented
- Added base ADVA ConfigParser class and two subclasses (FSP150F2, FSP150F3)
- Added compliance and parser testing suites, tests are successful
architekture
requested review from
itdependsnetworks,
jeffkala and
qduk
as code owners
|
Can you link to the vendor this is referencing? also, not normal for us to have separation of subclasses such as FSP150F2, FSP150F3, may make sense but should have context as to the why. |
This was my request because f2 and f3 have many differences and we don't know them all "yet". Also Netmiko has two different drivers for f2 and f3 so we wanted to follow suite on that front. Netmiko drivers: https://github.com/ktbyers/netmiko/blob/develop/PLATFORMS.md#:~:text=adva_fsp150f2,adva_fsp150f3 |
|
Corrected issues with documentation and formatting. All checks green. |
jeffkala
left a comment
jeffkala
left a comment
There was a problem hiding this comment.
LGTM with suggested change to change fragment.
|
Please hold off on this one until I can get a look |
|
To start, it's not clear to me what vendor this is?? Is this adtran? |
ADTRAN Holdings acquired ADVA Optical Networking, these are still using Adva OS so are a different OS vs Adtran which has its own netmiko driver etc. |
Co-authored-by: Jeff Kala <[email protected]>
| @@ -0,0 +1,5 @@ | |||
| features = [ | |||
There was a problem hiding this comment.
Can you test the banner feature explicitly here?
There was a problem hiding this comment.
We certainly can if you prefer. I had it as part of the system featureset since it's not a root-level config context as you'd see with other vendors. Happy to add a banner-specific test for both parsers.
tests/unit/mock/config/parser/base/adva_fsp150f3/adva_fsp150f3_full_received.py
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,5 @@ | |||
| features = [ | |||
There was a problem hiding this comment.
Same question here.
tests/unit/mock/config/compliance/compliance/adva_fsp150f3/adva_fsp150f3_basic_feature.py
Outdated
Show resolved
Hide resolved
netutils/config/parser.py
Outdated
| class ADVAFSP150F3ConfigParser(ADVAConfigParser): | ||
| """ADVA OS FSP-150 F3 ConfigParser.""" | ||
|
|
||
| comment_chars: t.List[str] = ["#", "home", "Preparing configuration file..."] |
There was a problem hiding this comment.
Putting home here seems reasonable. That being said, just want to document the significance. Can you add a comment here describing it?
There was a problem hiding this comment.
After talking with the customer, I've modified the comment chars for both parsers. Apparently the ADVA OS requires the use of home (or exit, depending on version) to return to the default context; you can't switch config contexts like you can with Cisco before continuing.
The parser classes have been updated and the full_received mock files have been updated accordingly.
… with the customer. Updated the full received files accordingly for mock testing.
netutils/lib_mapper.py
Outdated
| # Deep copy the reverse, where there is no actual translation happening with special | ||
| # consideration for OS's not in netmiko. | ||
| _MAIN_LIB_MAPPER = copy.deepcopy(NETMIKO_LIB_MAPPER) | ||
| _MAIN_LIB_MAPPER["adva_fsp150f2"] = "adva_fsp150f2" |
There was a problem hiding this comment.
These should be added to netmiko, as they are in there: https://ktbyers.github.io/netmiko/PLATFORMS.html
There was a problem hiding this comment.
Moved platform mapping from MAIN_LIB_MAPPER to NETMIKO_LIB_MAPPER.
| ), | ||
| ConfigLine(config_line=" pae authentication-control enable", parents=()), | ||
| ConfigLine(config_line=" pae authentication-control enable", parents=("configure system",)), | ||
| ConfigLine(config_line="exit", parents=()), |
There was a problem hiding this comment.
I don't think there should ever be a case where a ConfigLine() is exactly the same. I could be wrong about this, but would want to make sure we are all in sync.
|
We should probably meet internally on this one, it's a bit complicated and don't know what the correct path forward is. |
…terface. Also added explicit banner tests for both ADVA parsers. Updated docs as necessary.
…and A. Byczkowski.
jeffkala
left a comment
jeffkala
left a comment
There was a problem hiding this comment.
LGTM, for general info for @itdependsnetworks the banner test specifically were removed here since banners are in a nested block for Advas. Meaning the config to match is actually configure system and its a child line. This PR goes directly inline with the existing nokia_sros parser which functions in the same way (banner) is a nested config line.
