Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement CNaaS-NMS REST API as alternative PortAdmin management backend #2126

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from
Draft

Implement CNaaS-NMS REST API as alternative PortAdmin management backend #2126

wants to merge 16 commits into from

Conversation

lunkwill42
Copy link
Member

@lunkwill42 lunkwill42 commented Mar 5, 2020 •
edited
Loading

This PR is a work in progress, intending to:

  • Implement low-level support for the CNaaS NMS REST API
  • Implement a hybridized management mode for PortAdmin where
    • Read requests to equipment will still use direct management
    • Write requests will be proxied to the CNaaS NMS API and never talk directly to the device itself.

After deliberations with SUNET, these operations still remain to be mapped to CNaaS-NMS:

  • Implement interface cycling using the CNaaS-NMS bounce operation.
  • Wait for configuration commit operations (device_syncto) to complete and report status back to end user
  • Implement the is_dot1x_enabled() and get_dot1x_enabled_interfaces() PortAdmin operations using CNaaS-NMS API
  • Implement VLAN-changing operations through CNaaS-NMS API rather than directly through the underlying ManagementHandler
  • Configured VLANs should be fetched from CNaaS-NMS rather than directly from the device
  • Ensure uplink ports cannot be edited in PortAdmin by querying port types from CNaaS-NMS API

Also, there was a suggestion to have PortAdmin show port neighbor information in
cases where it exists, but this is outside the scope of this PR.

Reference: https://github.com/SUNET/cnaas-nms

@lunkwill42 lunkwill42 added this to the 5.1.0 milestone Mar 5, 2020
@lunkwill42 lunkwill42 added CNaaS Related to the CNaaS activity portadmin labels Mar 5, 2020
@lunkwill42 lunkwill42 self-assigned this Mar 5, 2020
@lunkwill42 lunkwill42 mentioned this pull request Mar 5, 2020
Closed
2 tasks
@lunkwill42 lunkwill42 force-pushed the feature/cnaas-nms-api branch from 3bebc51 to 413bcc2 Compare March 5, 2020 13:43
@lunkwill42 lunkwill42 force-pushed the feature/cnaas-nms-api branch 2 times, most recently from bd6b79a to a843e2a Compare July 1, 2020 12:02
@lunkwill42
Copy link
Member Author

At this point, there is still more work to be done on abstracting the ManagementHandler base class some more, due to previously poor design decisions made on the original SnmpHandler implementation (made when only SNMP was in scope).

The CNaaS-NMS API functionality works only partially. There is still an issue of how to map VLAN/VXLAN information from CNaaS-NMS to NAV (since NAV has no concept of VXLAN yet).

The new partially-implemented functionality is switched off by default, and the options to switch it on are not yet documented in the example portadmin.conf - i.e. the original functionality should be retained and appear unchanged to the end-user - so if tests pass, this code can be shipped.

I have been instructed to prioritize other aspects of the PortAdmin interface, namely NETCONF support. This work would rely on some of the structural changes and abstractions that have already been made in this PR, so this work will have to be based off this PR until it is merged.

@lunkwill42 lunkwill42 force-pushed the feature/cnaas-nms-api branch from a843e2a to 9d74ae0 Compare July 10, 2020 11:31
@lunkwill42
Copy link
Member Author

I have been instructed to prioritize other aspects of the PortAdmin interface, namely NETCONF support. This work would rely on some of the structural changes and abstractions that have already been made in this PR, so this work will have to be based off this PR until it is merged.

Actually, I will probably try to extract all structural/API changes to a separate PR, and instead base this PR and the new PR on top of that instead.

@lunkwill42 lunkwill42 removed this from the 5.1.0 milestone Nov 26, 2020
@lunkwill42 lunkwill42 force-pushed the feature/cnaas-nms-api branch from 9d74ae0 to 3b5015e Compare March 25, 2021 12:20
@lunkwill42
Copy link
Member Author

Actually, I will probably try to extract all structural/API changes to a separate PR, and instead base this PR and the new PR on top of that instead.

And so I finally did. I.e. the structural changes have mostly taken place as part of the NETCONF/Juniper changes to PortAdmin, so I have rebased this entire branch onto master and removed/rewritten the parts that no longer made sense.

I haven't tested the rebased code yet, I will need to verify my access tokens to SUNET's CNaaS lab environment first.

@lunkwill42 lunkwill42 force-pushed the feature/cnaas-nms-api branch from 3b5015e to f64068c Compare March 26, 2021 07:17
lunkwill42 added 13 commits April 16, 2021 08:39
@lunkwill42
Add initial low-level API library for CNaaS-NMS
f69da5e 
This will change. I do not have access to an actual CNaaS-NMS instance
at the moment, and have too little experience with simple_rest_client to
do a proper test.
@lunkwill42
Add PortAdmin config options for CNaaS NMS API
4a11fe5
@lunkwill42
Add mostly empty CNaaSNMSMixIn implementation
655028d 
Because:
- We want a MixIn class to build hybrid Portadmin handler classes on the
  fly, where read operations can be handled directly by the
  vendor-specific ManagementHandler and write operations can instead be
  dispatched to the CNaaS NMS API.
@lunkwill42
Make CNaaS NMS+ManagementHandler hybrid handlers
d072b86 
Because:
- When CNaaS NMS proxying is enabled, read operations should proceed as
  normal, directly against the device, using whatever ManagementHandler
  implementation deemed appropriate, while write operations should pass
  through the CNaaS NMS API.
@lunkwill42
Partially implement CNaaS NMS API handler methods
5394c34
@lunkwill42
Attempt to appease PyLint
6dae13b 
Because:
- HybridProxyHandler does not implement anything
- The simple_rest_client API class produces dynamic member attributes
  based on the API definition. PyLint is unable to determine what valid
  member attributes exists anyway.
@lunkwill42
Use IP address for CNaaS-NMS device lookups
883b333 
Because:
- Device names aren't guaranteed to be the same in CNaaS-NMS and NAV:
  NAV's name is fetched from DNS, CNaaS-NMS' is set independently of
  DNS.
- The management IP address is expected to be the same in both systems.
@lunkwill42
Extract _get_device_record
d47f800 
Because:
- CNaaSNMSMixIn will need a generic helper method to fetch a matching
  device record from CNaaS-NMS
- This helps separate concerns of generic device retrieval errors to
  that of verifying CNaaS NMS' willingness to actually configure this
  device.
@lunkwill42
Rewrite raise_if_not_configurable()
4ba4689 
Because:
- Error message should be more specific about which CNaaS NMS management
  criteria aren't met.
@lunkwill42
Extract _bounce_interfaces function
dae3eeb 
Because:
- Although the actual method to cycle may be different in different
  subclasses, the logic for verification/filtering of the incoming
  interfaces list remains largely the same.
- This moves the heavy lifting to a private method that can be
  overriden by subclasses who do not need to reimplement or change
  the verification/filter logic.
@lunkwill42
Implement CNaaS-NMS interface_status API endpoint
045f963 
The interface_status endpoint can be used to list the status of each
interface of a device, but also to initiate a bounce operation for
a list of interfaces on said device.
@lunkwill42
Implement interface cycling through CNaaS-NMS
c45928a 
Because:
- CNaaS-NMS supports this operation internally, so we do not need
  to use the inherited generic method from ManagementHandler.
@lunkwill42
Add config option to disable SSL verification
91da97b 
Because:
- The test server runs on a self-signed certificate, which makes the
  underlying HTTP library raise an exception.
- We need to be able to disable SSL verification for testing purposes,
  (but we should never need this option in production!)
@lunkwill42
Use correct device name in all API requests
673db7c 
Because:
- NAV's device sysname does not necessarily correspond to the hostname
  used by CNaaS-NMS, depending on the DNS setup.
- The best way to find the corresponding device in CNaaS-NMS is to
  look it up using its management IP.
- The code to look up the device already exists, so we re-use it to
  provide a device_name property that can be used in all generated
  API requests.
@lunkwill42
Properly define the job API endpoints
93e0274
@lunkwill42
Wait for configuration commit to complete
830a7bd 
Because:
- We need to make sure the commit finishes without error before
  reporting success or failure to the end user.
@lunkwill42 lunkwill42 force-pushed the feature/cnaas-nms-api branch from f64068c to 830a7bd Compare April 16, 2021 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@lunkwill42 lunkwill42

Labels
CNaaS Related to the CNaaS activity enhancement portadmin
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@lunkwill42