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

change mgmt_ipv4/6 to mgmt-ipv4/6 #1370

Merged
merged 3 commits into from
May 11, 2023
Merged

Conversation

steiler
Copy link
Collaborator

@steiler steiler commented May 9, 2023

Besides changing dash to underscore for ipv{4,6}_subnet via #1365.
I think the mgmt_ipv4/6 config stanza is the last remaining underscore topology setting.
I think, it would make sense to bundle this and #1365 in a single release, to have such breaking change just once.

This will in then in my view be enough to close #749

@hellt
Copy link
Member

hellt commented May 9, 2023

Yes, these changes definitely must go together.
Besides that, I think we need to create a special error message notifying people to check release notes when unmarshalling error happens for unrecognized fields

@steiler
Copy link
Collaborator Author

steiler commented May 9, 2023

That would be here then.
https://github.com/srl-labs/containerlab/blob/main/clab/file.go#L76
What do you propose as the message to show?

@hellt
Copy link
Member

hellt commented May 9, 2023

@steiler

Failed to read the topology definition file. Consult with release notes to see if any fields were changed/removed.

@hellt
Copy link
Member

hellt commented May 9, 2023

PS. Currently I already see that this message is emitted:

 dclab des -c -t private/linux.clab.yml
Error: failed to read topology file: yaml: unmarshal errors:
  line 5: field ipv4-subnet not found in type types.MgmtNet
  line 6: field ipv4-range not found in type types.MgmtNet

@codecov
Copy link

codecov bot commented May 11, 2023

Codecov Report

Merging #1370 (76ef72d) into main (ccd6e3b) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main    #1370   +/-   ##
=======================================
  Coverage   20.82%   20.82%           
=======================================
  Files          57       58    +1     
  Lines        6286     6333   +47     
=======================================
+ Hits         1309     1319   +10     
- Misses       4856     4891   +35     
- Partials      121      123    +2     
Impacted Files Coverage Δ
clab/file.go 54.54% <0.00%> (-2.60%) ⬇️
types/node_definition.go 17.24% <ø> (ø)

... and 4 files with indirect coverage changes

@hellt hellt merged commit 5543595 into srl-labs:main May 11, 2023
18 checks passed
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.

check topology elements to use dashes in names, not underscores
2 participants