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

VLAN table information for JunOS #1367

Closed
wants to merge 4 commits into from

Conversation

ckozler
Copy link

@ckozler ckozler commented Jan 25, 2021

No description provided.

@ckozler
Copy link
Author

ckozler commented Jan 25, 2021

Related to #1366

@ckozler
Copy link
Author

ckozler commented Jan 25, 2021

Sample output

./bin/napalm --vendor junos -u root --optional_args keepalive=600,auto_probe=600,timeout=600 x.x.x.x call get_vlans
{
"default": {
"name": "default",
"tag": "1",
"members": [
"ge-0/0/0.0*",
"ge-0/0/1.0*",
"ge-0/0/2.0*",
"ge-0/0/3.0*",
"ge-0/0/30.0*",
"ge-1/0/0.0*",
"ge-1/0/1.0*",
"ge-1/0/2.0*"
]
},
"vlan2_MGMT": {
"name": "vlan2_MGMT",
"tag": "2",
"members": "ae0.0*"
},
"vlan3_TEST": {
"name": "vlan3_TEST",
"tag": "3",
"members": "none-assigned"
}
}

Copy link
Member

@mirceaulinic mirceaulinic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start, thanks for the PR. Please install the test dependencies from requirements-dev.txt and run black . to format your text properly.

Would also need to add tests for this; locally you can run tox to ensure everything is fine.


),
"members": (
"none-assigned" # Checking even older junos, it alwayas returns either a list of members or l2ng-l2rtb that you see below
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be just an empty list instead of "none-assigned".

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I can change that. I put this as a catch-all but an empty list as a catch all works too

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How's it going? :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been working on implementing this along side network-importer tools (https://github.com/networktocode/network-importer/) but I can submit the change you recommended to get this moved up. I originally started this because of of network-importer so I wanted to make sure it'd work with it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you wish, @ckozler - if you want to continue working on this with the requested changes, and the tests, I'll be happy to review; otherwise, feel free to close it out. 👍

@mirceaulinic
Copy link
Member

Closing out due to inactivity.

@ckozler if you wanted to continue working on this, let me know and I'll gladly reopen, or feel free to open a separate PR. Remember, the data structure return must align to the NAPALM model, which must be respected to every driver implementing the method: https://github.com/napalm-automation/napalm/blob/develop/napalm/base/test/models.py#L267.

@minefuto
Copy link
Contributor

minefuto commented Mar 12, 2021

Hi @ckozler,
I want to use get_vlans on Junos but it seems this PR is inactivity.
May I submit a PR on behalf of you?

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

Successfully merging this pull request may close these issues.

3 participants