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

Allow individual config_db per host via ztp.json #358

Merged
merged 32 commits into from
Dec 13, 2024
Merged

Conversation

iljarotar
Copy link
Contributor

@iljarotar iljarotar commented Nov 21, 2024

Description

Initially, this PR was intended to enable a ZTP-only deployment for SONiC switches, such that all configuration currently handled by the sonic role could be managed entirely by ZTP. The motivation behind a ZTP-only approach is that the template for the config_db.json has become quite complicated and difficult to debug recently, because many special cases needed to be handled. With a ztp.json file it is possible to provide an individual config_db.json for each switch, which makes templating obsolete.

The downside to this way of provisioning is that the SONiC config cannot be changed via CI anymore, but only by resetting the switch completely. For the config_db.json, this seems fine to me, as it reduces the risk of breaking the switch by providing an invalid breakout config for example. However, providing the frr.conf file via ZTP doesn't seem a good idea to me and it would have introduced a lot of redundancy, because the sonic role also renders the frr.conf.

Therefore, I decided to make ztp.json only provide the config_db.json and some setup scripts, but leave the rest to the sonic role. To avoid conflicts with the config_db.json rendered by the sonic role, I introduced a new variable sonic_render_config_db_template which defaults to true, to not break existing deployments.

Noteworthy

It is now possible to provision SONiC switches with a static `config_db.json` provided through ZTP instead of rendering this configuration dynamically during deployment. This approach leads to more stable rollouts and might become our default for SONiC deployments. When using this new provisioning variant, follow the example in the `ztp` role documentation and set `sonic_render_config_db_template` in the `sonic` role to false. Also leave `sonic_ports`, `sonic_breakouts` and `sonic_portchannels` empty. Otherwise there might be conflicts between the provided `config_db.json` and the one rendered by the `sonic` role.

@iljarotar iljarotar requested a review from a team as a code owner November 21, 2024 14:43
Copy link
Contributor

@robertvolkmann robertvolkmann left a comment

Choose a reason for hiding this comment

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

The PR should ensure that AgentX is enabled on the leaves.

@iljarotar iljarotar changed the title Enable SONiC deployment completely via ztp role Allow individual config_db per host via ztp.json Dec 4, 2024
@iljarotar iljarotar self-assigned this Dec 9, 2024
@Gerrit91 Gerrit91 requested a review from mreiger December 10, 2024 13:35
@Gerrit91
Copy link
Contributor

@mreiger yesterday brought up that it might be good to add a comment to the documentation that with this approach it is not possible to simply add something new to the config_db.json. Every change there requires re-provisioning the switch.

partition/roles/ztp/templates/ztp.json.j2 Outdated Show resolved Hide resolved
partition/roles/ztp/templates/ztp.json.j2 Outdated Show resolved Hide resolved
partition/roles/ztp/templates/ztp.json.j2 Outdated Show resolved Hide resolved
partition/roles/ztp/templates/ztp.json.j2 Outdated Show resolved Hide resolved
partition/roles/ztp/templates/ztp.json.j2 Outdated Show resolved Hide resolved
@iljarotar iljarotar merged commit fa75b81 into master Dec 13, 2024
1 check passed
@iljarotar iljarotar deleted the sonic-ztp branch December 13, 2024 14:12
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.

3 participants