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.

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

Add Cisco IOL kind #2211

Merged
merged 23 commits into from
Oct 4, 2024
Merged

Add Cisco IOL kind #2211

merged 23 commits into from
Oct 4, 2024

Conversation

kaelemc
Copy link
Contributor

@kaelemc kaelemc commented Sep 30, 2024

Relevant to hellt/vrnetlab#256. This adds Cisco IOL as a kind in containerlab.

The kind is name cisco_iol let me know if you want this changed. I've also created the node as a VRNode type, I figured that'd be the easiest way to get the nice interface names.

IOL also doesn't support NETCONF so configuration saving isn't possible by those means, but the user is able to perform copy run start or write memory locally on IOL and it'll save to the nvram (which is stored under the labdirectory). That way config persistence is achieved.

Still working on documentation and other things.

@kaelemc
Copy link
Contributor Author

kaelemc commented Oct 1, 2024

@hellt. I have tested and got an OSPF adjacency between two IOL nodes connected via an IOL-L2 switch.

image

name: iol
topology:
  nodes:
    r1:
      kind: cisco_iol
      image: vrnetlab/vr-iol:17.12.01
    r2:
      kind: cisco_iol
      image: vrnetlab/vr-iol:17.12.01
    sw1:
      kind: cisco_iol
      image: vrnetlab/vr-iol-l2:17.12.01
  links:
    - endpoints: ["r1:e0/1", "sw1:e0/1"]
    - endpoints: ["r2:e0/1", "sw1:Ethernet0/2"]

I think both PRs should be good to be reviewed. Let me know if i've missed any functionality.

@kaelemc kaelemc marked this pull request as ready for review October 1, 2024 09:21
@hellt
Copy link
Member

hellt commented Oct 1, 2024

Thanks @kaelemc!

nodes/iol/iol.go Outdated Show resolved Hide resolved
@kaelemc kaelemc marked this pull request as draft October 3, 2024 15:47
@kaelemc
Copy link
Contributor Author

kaelemc commented Oct 4, 2024

@hellt IOL type has been added, can be either iol (default) or l2. The configuration will change accordingly.

I've also made it so interfaces can now be defined in a non-contiguous manner. For example you can now have something like this

links:
  - endpoints: ["router:Ethernet1/2", "switch:e0/4"]

previously interfaces had to be added sequentially (if you wanted to use e0/4 it meant that e0/1, e0/2, e0/3 had to be defined first).

I have one problem. IOL interfaces can only have 4 interfaces per slot. That means if I wanted to use eth6 that'd be e1/2. The regex for interface aliasing doesn't seem to support the slot formatting but only <port>. So technically the user could define e0/6 but that wouldn't match with the CLI.

@kaelemc
Copy link
Contributor Author

kaelemc commented Oct 4, 2024

I think we should stick with the contiguous interface naming requirement even though the code can handle otherwise. Even if a user uses a random interface without declaring/using the previous, all interfaces will show up in the CLI anyways which might just create extra confusion.

The regex issue is still a problem as soon as a user wants more than 3 data-plane interfaces in their topology.

@kaelemc
Copy link
Contributor Author

kaelemc commented Oct 4, 2024

I'm not sure if this was the right way to do it but interfaces are assigned to the container following ethX schema.

GetInterfaceMapping() (heavily copied from the SRL definition) will map the interface slot/port into an ethX based endpoint. For example e1/2 becomes eth6.

Interfaces are assigned in groups of four so each slot has max 4 interfaces, because of this the slot is multiplied by 4, the port index is added and then we add them and get out ethX interface index.

The topology will still only accept the CLI style interface names and NOT ethX interface naming.

I have done my 'famous' OSPF adjacency test using the following topology:

name: iol
topology:
  nodes:
    r1:
      kind: cisco_iol
      image: vrnetlab/cisco_iol:17.12.01
    r2:
      kind: cisco_iol
      image: vrnetlab/cisco_iol:17.12.01
    sw1:
      kind: cisco_iol
      image: vrnetlab/cisco_iol:l2-17.12.01
      type: l2
  links:
    - endpoints: ["r1:e0/1", "sw1:e0/1"]
    - endpoints: ["r2:e0/1", "sw1:e1/2"]

Edgeshark shows the mapped interface
image

OSPF verification:
image

@kaelemc kaelemc marked this pull request as ready for review October 4, 2024 10:46
@hellt hellt merged commit 2764acc into srl-labs:main Oct 4, 2024
64 checks passed
Copy link

codecov bot commented Oct 4, 2024

Codecov Report

Attention: Patch coverage is 80.14706% with 27 lines in your changes missing coverage. Please review.

Project coverage is 51.54%. Comparing base (ab4927d) to head (83e54d7).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
nodes/iol/iol.go 81.39% 15 Missing and 9 partials ⚠️
utils/ip.go 50.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2211      +/-   ##
==========================================
+ Coverage   50.70%   51.54%   +0.84%     
==========================================
  Files         171      171              
  Lines       12536    12631      +95     
==========================================
+ Hits         6356     6511     +155     
+ Misses       5291     5212      -79     
- Partials      889      908      +19     
Files with missing lines Coverage Δ
clab/register.go 100.00% <100.00%> (ø)
utils/ip.go 17.64% <50.00%> (+17.64%) ⬆️
nodes/iol/iol.go 81.39% <81.39%> (ø)

... and 7 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants