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

[config-gen] Add interface speed from netbox #136

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

swagner-de
Copy link
Contributor

We need to force the speed on some breakout/QSA interfaces. As
port-channel members are just plain strings, we add the speed to the
port-channel interfaces itself. That is fine, as members can only be of
the same speed anyway. For physical interfaces, we can annotate it
straight to the interface.

@swagner-de swagner-de marked this pull request as ready for review December 12, 2023 10:52
Copy link
Collaborator

@sebageek sebageek left a comment

Choose a reason for hiding this comment

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

Code looks good so far, also tested it and it works. Added a couple of comments.

networking_ccloud/tools/netbox_config_gen.py Outdated Show resolved Hide resolved
networking_ccloud/tools/netbox_config_gen.py Show resolved Hide resolved
networking_ccloud/tools/netbox_config_gen.py Outdated Show resolved Hide resolved
Comment on lines 243 to 244
raise ConfigException(f'Interface {iface.id} has a different speed than other '
f'member interface(s) of LAG {lag.id}, has {speed}, other(s) have '
f'{cls.lag_member_speed_cache[lag.id]}') # type: ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is lag.id enough to identify the port channel or would it be easier to the user to have a devicename (or switchport) + port name to find the offending entity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean lag.id is the primary key in the database table and netbox won't create any duplicate (device_name, interface_name). I wouldn't see a reason to use anything else.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was just thinking about the error message displayed to the user. Using lag.id as dict key totally makes sense to me.

sebageek
sebageek previously approved these changes Dec 12, 2023
Copy link
Collaborator

@sebageek sebageek left a comment

Choose a reason for hiding this comment

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

Error message with extra details is kinda optional for me

We need to force the speed on some breakout/QSA interfaces. As
port-channel members are just plain strings, we add the speed to the
port-channel interfaces itself. That is fine, as members can only be of
the same speed anyway. For physical interfaces, we can annotate it
straight to the interface.
@swagner-de swagner-de merged commit 357ef10 into stable/yoga-m3 Dec 12, 2023
3 checks passed
@swagner-de swagner-de deleted the add-interface-speed branch December 12, 2023 12:35
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