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

Move all minigraph-related action from rc.local to updategraph #1452

Merged
merged 4 commits into from
Mar 10, 2018

Conversation

taoyl-ms
Copy link
Contributor

@taoyl-ms taoyl-ms commented Mar 3, 2018

- What I did

  1. rc.local no longer do configuration-related stuff. Instead, updategraph service now has the functionality to support first-time boot configuration behavior. The detail behavior is that
    -- For pure new installation, generate configuration from default minigraph if updategraph is not enabled during build time; attempt to configure through DHCP if updategraph is enabled.
    -- For sonic version upgrade, generate configuration from minigraph on old system if updategraph is enabled during build time; use old configDB content if not enabled.

  2. Reconsider the dependency between services - updategraph is now after database. All feature services are now after and depending on updategraph. That means restarting updategraph will automatically trigger restarting of feature services.

  3. updategraph now supports the scenario of not updating minigraph from data source but only reloading local graph. We plan to adapt to this approach in sonic-utilities config load_minigraph to simplify the code and unify behavior.

Requires=docker.service teamd.service
After=swss.service teamd.service
Requires=updategraph.service swss.service teamd.service
After=updategraph.service swss.service teamd.service
Copy link
Collaborator

Choose a reason for hiding this comment

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

why dhcp_relay requires teamd service?

Copy link
Contributor

Choose a reason for hiding this comment

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

Portchannels need to be created and up before starting the relay, as the relay agent listens on them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it needs to listen on port channels. Please note that this was the original behavior and I didn't change it in this PR.

if [ ! -f /etc/sonic/init_cfg.json ]; then
echo "{}" > /etc/sonic/init_cfg.json
fi
redis-cli -n 4 FLUSHDB
Copy link
Collaborator

Choose a reason for hiding this comment

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

4 [](start = 17, length = 1)

can you assign config_dbnum to 4 and use it throughout the script?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

fi
mkdir -p /etc/sonic/old_config
mv /host/minigraph.xml /etc/sonic/old_config/
touch /tmp/firstboot
Copy link
Collaborator

Choose a reason for hiding this comment

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

firstboot [](start = 19, length = 9)

why do we need this firstboot, can updategraph check the /etc/sonic/old_config directory for it to pick up the minigraph or config_db from that directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The plan is to keep /etc/sonic/old_config folder there and not to delete it after first boot for easy debugging. Therefore we need a flag to mark whether it is the first boot.

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like the file name firstboot is confusing because we also reference /host/image-$sonic_version/platform/firsttime in this file, which is also used to determine whether it's the initial boot.

Can we give this file a more specific name based on its application?

Also, I personally think firstboot is a better name for /host/image-$sonic_version/platform/firsttime

Copy link
Collaborator

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

can you rebase and resolve the conflict?

@taoyl-ms
Copy link
Contributor Author

taoyl-ms commented Mar 9, 2018

Done.

@jleveque
Copy link
Contributor

jleveque commented Mar 9, 2018

Please see my comment on rc.local regarding the name of the firstboot file.

@taoyl-ms
Copy link
Contributor Author

taoyl-ms commented Mar 9, 2018

Modified to /tmp/pending_config_migration and /tmp/pending_config_initialization according to PR comments.

@taoyl-ms taoyl-ms merged commit e84e093 into sonic-net:master Mar 10, 2018
theasianpianist pushed a commit to theasianpianist/sonic-buildimage that referenced this pull request Feb 5, 2022
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>

Co-authored-by: wenda.ni <wenda.ni@bytedance.com>
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