Support for private vlans (l2_interfaces)

UvixCreative commented 2 years ago

SUMMARY

cisco.ios (and cisco.nxos eventually) modules should support private vlan. I believe this needs to be added to l2_interfaces

ISSUE TYPE

Feature Idea

COMPONENT NAME

ios_l2_interfaces

ADDITIONAL INFORMATION

This would support a feature which has been a part of ios since at least version 15.2(7)E. There may be more modules that I've missed that would require modifications to fully support private vlans. I haven't fully looked into the "switchport private-vlan mapping" command

Proposed example usage:

---
ios_l2_interfaces:
  - name: gi1/0/1
    mode: private-vlan
    private_vlan:
      mode: host
      association:
        primary: 10
        secondary: 25
  - name: gi1/0/2
    mode: private-vlan
    private_vlan:
      mode: trunk
      promiscuous : true
      association:
        primary: 10
        secondary: 40
      mapping:
        primary: 10
        secondary:
          - 25
          - 40
    trunk:
      allowed_vlans:
        - 1
        - 5
        - 10
        - 25
        - 40

Notes:

It may make it easier to split "association" in l2_interfaves into host_association and trunk_association
Similarly, it might be easier to create a distinct "trunk" parameter rather than reusing the existing one. My thinking is to try to make the process reasonably transparent to the user and to make it less confusing

Reference: https://www.cisco.com/c/en/us/td/docs/switches/lan/catalyst4500/12-2/31sga/configuration/guide/config/pvlans.html

UvixCreative commented 2 years ago

I'm interested in working on this, but, frankly, it would be my first contribution to an open-source project, so this isn't something I'm used to. As such, I would appreciate if someone could give me any pointers on where to start. I've been looking through the code and I think I could figure it out, and I'll probably start testing with it over the weekend or so

KB-perByte commented 2 years ago

Hey, @UvixCreative I appreciate your interest in contributing here. We are working on some minor optimizations for this module, you may keep an eye on the PR here. This was a resource module developed the old way, we are upgrading it to our newer standards of resource modules. you may find more insights in the docs -

I can keep the Feature request open for you to take it up once the above-mentioned PR is merged. Let me know if you need anything.

Regards

KB-perByte commented 2 years ago

@UvixCreative our supported IOSXE version across the collection is version 17.3, specifically for L2-based modules we haven't upgraded the supported version yet. But it is encouraged to test out the newer features which are added with a non-EOL'd IOSXE version.

UvixCreative commented 2 years ago

Ok, thank you very much for the resources. I'll check out the pr and documentation. IOS 17 is the version I usually work with these days, so I'm already familiar and it should be relatively easy for me to test against that.

Thanks again

UvixCreative commented 2 years ago

Hey! So I've ran into a bit of a wall. I was making some progress and I was about to start working on the facts file for ios_vlans, but I was going to run it with "gathered" first to see what it does. It gave me an argspec error.

Point being, it looks like the file at https://github.com/ansible-network/resource_module_models/blob/master/models/ios/vlans/ios_vlans.yaml is out of date. This file was last updated in 2019 and it doesn't include "gathered" (among a couple other parameters/options) in the argspec.

So the question is, what would be my next steps? You can see above that I tried to follow the documented process (with the pull request above ansible-network/resource_module_models#188), but if that argspec is out of date, I'm not certain what to do next. Do you know where the model is that was used to generate the argspec that's in prod right now?

KB-perByte commented 2 years ago

Hey @UvixCreative the PR is merged you may start looking at it to add your options. As per this issue. Regards

KB-perByte commented 2 years ago

Hey! So I've ran into a bit of a wall. I was making some progress and I was about to start working on the facts file for ios_vlans, but I was going to run it with "gathered" first to see what it does. It gave me an argspec error.

Point being, it looks like the file at https://github.com/ansible-network/resource_module_models/blob/master/models/ios/vlans/ios_vlans.yaml is out of date. This file was last updated in 2019 and it doesn't include "gathered" (among a couple other parameters/options) in the argspec.

So the question is, what would be my next steps? You can see above that I tried to follow the documented process (with the pull request above ansible-network/resource_module_models#188), but if that argspec is out of date, I'm not certain what to do next. Do you know where the model is that was used to generate the argspec that's in prod right now?

So ios_vlans is an older Resource Module, where the scaffolding tools won't help you to generate the argspec and other boilerplate stuff. That is why we gradually update the older resource modules in terms of our newer development practices (the way we just did for l2_interfaces). Basically, you have to edit the documentation (not .rsts) and argspec manually with the attributes you are adding to the vlans module and then write code in facts and config side to leverage the attributes added. resource_module_models is just a repo where we stage our module docs. Regards

UvixCreative commented 2 years ago

Ohh, okay. So in that case, I believe the documentation on here is out of date, I think. That's the guide/resource I was trying to follow.

So then the process should be:

I fork ansible-collections/cisco.ios
Modify the argspec and documentation (are you able to provide a list of places that I need to look for to ensure all the proper docs are updated?)
Modify the code
Submit a PR back into ansible-collections/cisco.ios
Address any issues/comments/suggestions until the PR is accepted

Is that about right?

Thanks

KB-perByte commented 2 years ago

@UvixCreative The understanding is correct, the public docs need to be updated we are working on them and will be updated shortly. Until then please follow this guide and let me know if any part of it is confusing or needs improvement. This should cover everything as per the latest resource module development process we have.

Now, specifically for ios_vlans you have to change the module file with the attributes and add the same attributes in argspec file and then make necessary changes in the facts file for vlans to parse the lines and produce the structured data.

KB-perByte commented 2 years ago

And it would be great if you could draft a new issue for vlans and keep this one specific for l2_interfaces. It would help us track the issues better. Thanks

UvixCreative commented 2 years ago

Awesome, will do. That's super helpful. Thanks again!

UvixCreative commented 1 year ago

Starting to poke at this in my free time, but I have a question. I see that the module already supports the modes:

private_vlan_host
private_vlan_trunk
private_vlan_promiscuous

This is missing private-vlan trunk promiscuous (which is an easy fix), but it also works differently than the model that I proposed. Am I okay to change it over to that model? I believe it works better than the current model for being able to actually work with and configure private vlan options.

In short, the difference between models is

# Existing model
cisco.ios.ios_l2_interfaces:
  config:
    mode: private_vlan_host

# Proposed model
cisco.ios.ios_l2_interfaces:
  config:
    mode: private_vlan
    private_vlan:
      mode: host
      association:
        primary: 10
        secondary: 15

So you would just specify the mode as a general "private_vlan" interface, then configure the private_vlan options in its own dict

UvixCreative commented 1 year ago

After trying to actually work with it, I realized I had to rework the model a bit to better support some of the funky things that a promiscuous trunk can do. Below is an example of a complex setup that utilizes all options in the new model for private VLANs that I'm proposing.

# Environment/setup
cisco.ios.ios_vlans:
  config:
    - vlan_id: 10
    - vlan_id: 90
      private_vlan:
        type: primary
        associated:
          - 100
          - 101
    - vlan_id: 91
      private_vlan:
        type: primary
        associated:
          - 110
          - 111
    - vlan_id: 100
      private_vlan:
        type: isolated
    - vlan_id: 101
      private_vlan:
        type: community
    - vlan_id: 110
      private_vlan:
        type: isolated
    - vlan_id: 111
      private_vlan:
        type: community

# l2_interfaces complex example
cisco.ios.ios_l2_interfaces:
  config:
    - name: GigabitEthernet1/0/1
      mode: private_vlan
      private_vlan:
        mode: host
        host_association:
          primary: 90
          secondary: 100
    - name: GigabitEthernet1/0/2
      mode: private_vlan
      private_vlan:
        mode: promiscuous
        mapping:
          primary: 90
          secondary:
            - 100
            - 101
    - name: GigabitEthernet1/0/3
      mode: private_vlan
      private_vlan:
        mode: trunk
        trunk_association:
          - primary: 90
            secondary: 100
          - primary: 91
            secondary: 110
    - name: GigabitEthernet1/0/4
      mode: private_vlan
      private_vlan:
        mode: promiscuous_trunk
        trunk_mapping:
          - primary: 90
            secondary:
              - 100
              - 101
          - primary: 91
            secondary:
              - 110
              - 111
        trunk_association:
          - primary: 90
            secondary: 100
          - primary: 91
            secondary: 110
    - name: GigabitEthernet1/0/5
      mode: access
      access:
        vlan: 10
      description: Just a regular access port

UvixCreative commented 1 year ago

Just an update, I do still intend to work on this when I have time, but things have been busy personally since the holidays

KB-perByte commented 1 year ago

Hey @UvixCreative, sorry for the late reply, please consider tagging me in the conversation would help me catch up soon. As of now, we know we have private_vlan-s as a choice and I see the point where you want to add attributes that are not a choice but rather dict options. Please share the configurations that we are planning to implement. Regards

UvixCreative commented 1 year ago

@KB-perByte I believe these files are what you're referring to? This is basically the only work I've done so far, let me know if this looks about right and if it's accepted. Thanks!

plugins/modules/ios_l2_interfaces.py plugins/module_utils/network/ios/argspec/l2_interfaces/l2_interfaces.py

KB-perByte commented 1 year ago

Hey @UvixCreative I did not get what you are for here? What can I help you with? Regards

UvixCreative commented 1 year ago

@KB-perByte I was under the impression you were looking for the schema proposal that I had come up with. The two linked files describe the structure/schema that I'm proposing and planning to work with in order to implement private VLAN configuration on interfaces. I assume there's some sort of approval process for any contributor who wants to change the data structure for module inputs/parameters.

For now I've just planned out this structure, I haven't moved forward with changing the facts module or anything else functionality-wise to avoid potentially wasting work. If these look good, I can go ahead and start working on the functionality.

Thanks!

KB-perByte commented 1 year ago

Hello @UvixCreative the process is rather simple you may move ahead with a simple PR with all your proposed changes in, we can discuss over the PR itself. And, when you mention structure change does it mean that the change impacts the current playbook structure and implementation of the module and users have to redo the playbooks on release or you are proposing just attribute addition? I am fine with looking at a PR with changes directly. Regards

UvixCreative commented 1 year ago

I'd have to review it again more closely, but I believe I did change something that could affect backwards compatibility. I removed 2 existing options from config > mode and replaced them with private_vlan which would then trigger the rest of the module to read the configuration from the private_vlan dict block of configuration.

I've only changed the argspec and the inline documentation. Where would I submit a PR? Should I just submit a PR into the main repo (here) and leave it open for discussion until I'm done with the functionality and testing? (Again, not very familiar with the process/etiquette contributing to projects on GitHub, I appreciate the help!)

UvixCreative commented 1 year ago

@KB-perByte forgot to tag

KB-perByte commented 1 year ago

@UvixCreative yep just fork the main repo make your changes and raise a PR. We can take it forward from there. Regards

anubisg1 commented 1 year ago

Hello, what's the status of this?

are pvlans supported now or not? from the documentation i can see that we can configure mode private vlan (on each mode) but there doesn't seem a way to configure mappings

UvixCreative commented 1 year ago

@anubisg1 There currently isn't a way to configure private VLANs on an interface without using something lower-level like the ios_commands module. I've been meaning to work on this for a while in my own time, but I just haven't gotten around to it. I still do plan to get around to it, but I don't believe there's any first-party plans to work on this. If you're able to, feel free to contribute!

ansible-collections / cisco.ios