-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[sonic-config-engine] Default config without L2/L3 config #3392
base: master
Are you sure you want to change the base?
Conversation
[Platform] Updated default SKU with default config preset Signed-off-by: Antony Rheneus <[email protected]>
_sample_generators = { | ||
't1': generate_t1_sample_config, | ||
'l2': generate_l2_config, | ||
'default': generate_default_config, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this default configuration? is the switch in l2 mode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"generate_t1_sample_config" IP addresses being assigned from 10.0.0.0 to max port.
"generate_l2_config" all ports were added to Vlan1000
"generate_empty_config" ports were NOT added in config_db.json
To have system without any predefined configs for IP or VLAN, added this "default" config with just port config alone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is no clear definition of empty configuration. is it layer 2 more, layer 3 mode? we need a clear definition of empty configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current empty configuration doesn't even populate ports in config_db.json. User has to populate all the ports manually which can be avoided if scripts can just update all the ports in config and provide user a facility to configure later L2 or L3.
If you dont agree on this, Im ok to abandon this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also current l2 or t1 config generation adds hardcoded configuration which enables testbed suite to run testing. But the same L2/L3 config may not be the one user interested in.
… review Signed-off-by: Antony Rheneus <[email protected]>
Started separate PR 3919 for HW SKU port config which was added in this PR, so that hwsku changes can get through review process |
[Platform] Updated default SKU with default config preset
Signed-off-by: Antony Rheneus [email protected]
- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)