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

[systemd]: Add platform-init and role-config services; Deprecate rc.local #427

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
2 changes: 2 additions & 0 deletions files/build_templates/dhcp_relay.service.j2
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
[Unit]
Description=DHCP relay container
Requires=docker.service
After=role-config.service
After=interfaces-config.service
ConditionPathExists=/etc/sonic/role/dhcp_relay

[Service]
User={{ sonicadmin_user }}
Expand Down
14 changes: 12 additions & 2 deletions files/build_templates/sonic_debian_extension.j2
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,18 @@ sudo bash -c "echo enabled=false > $FILESYSTEM_ROOT/etc/sonic/updategraph.conf"
# Copy SNMP configuration files
sudo cp $IMAGE_CONFIGS/snmp/snmp.yml $FILESYSTEM_ROOT/etc/sonic/

# Copy platform-init script and service unit file, enable service
sudo cp $IMAGE_CONFIGS/platform/platform-init.service $FILESYSTEM_ROOT/etc/systemd/system/
sudo LANG=C chroot $FILESYSTEM_ROOT systemctl enable platform-init.service
sudo cp $IMAGE_CONFIGS/platform/platform-init.sh $FILESYSTEM_ROOT/usr/bin/

# Copy role-config script and service unit file, enable service and create dir to
# house files that will be used to conditionally start services based on device role
sudo cp $IMAGE_CONFIGS/role/role-config.service $FILESYSTEM_ROOT/etc/systemd/system/
sudo LANG=C chroot $FILESYSTEM_ROOT systemctl enable role-config.service
sudo cp $IMAGE_CONFIGS/role/role-config.sh $FILESYSTEM_ROOT/usr/bin/
sudo mkdir -p $FILESYSTEM_ROOT/etc/sonic/role/

# Copy sudoers configuration file
sudo cp $IMAGE_CONFIGS/sudoers/sudoers $FILESYSTEM_ROOT/etc/

Expand Down Expand Up @@ -173,8 +185,6 @@ sudo dpkg --root=$FILESYSTEM_ROOT -P {{ debname }}

sudo rm -f $FILESYSTEM_ROOT/usr/sbin/policy-rc.d

## copy platform rc.local
sudo cp $IMAGE_CONFIGS/platform/rc.local $FILESYSTEM_ROOT/etc/

{% if installer_images.strip() -%}
{% for image in installer_images.strip().split(' ') -%}
Expand Down
2 changes: 2 additions & 0 deletions files/build_templates/teamd.service.j2
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
[Unit]
Description=TEAMD container
Requires=database.service
After=role-config.service
After=database.service
ConditionPathExists=/etc/sonic/role/teamd

[Service]
User={{ sonicadmin_user }}
Expand Down
13 changes: 13 additions & 0 deletions files/image_config/platform/platform-init.service
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[Unit]
Description=First boot platform initialization
Before=updategraph.service
ConditionPathExists=/host/platform/firsttime

[Service]
Type=oneshot
ExecStartPre=/bin/echo "First boot detected. Initializing platform..."
ExecStart=/usr/bin/platform-init.sh
ExecStartPost=/bin/rm -f /host/platform/firsttime
Copy link
Collaborator

Choose a reason for hiding this comment

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

/host/platform/firsttime [](start = 25, length = 24)

Is it possible to disable the service, instead of manipulating a file?

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 believe it would be possible to disable the service after it completes. I hadn't considered the possibility, as the 'firsttime' file method is currently used, and it just seems a little strange to have a service disable itself.

Anyone else have an opinion on this?


[Install]
WantedBy=multi-user.target
24 changes: 24 additions & 0 deletions files/image_config/platform/platform-init.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/bin/bash
#
# platform-init.sh
#
# Script to perform tasks which configure platform upon first boot
# To be run by platform-init.service if it detects first boot
#

PLATFORM=`sonic-cfggen -v platform`

if [ -n $PLATFORM ]; then
echo "SONiC platform: $PLATFORM"

echo "Copying default minigraph for $PLATFORM to /etc/sonic/"
cp /usr/share/sonic/device/$PLATFORM/minigraph.xml /etc/sonic/

if [ -d /host/platform/$PLATFORM ]; then
echo "Installing platform-dependent packages for $PLATFORM..."
dpkg -i /host/platform/$PLATFORM/*.deb
fi
else
echo "SONiC platform unknown. Could not install platform-dependent packages."
fi

39 changes: 0 additions & 39 deletions files/image_config/platform/rc.local

This file was deleted.

10 changes: 10 additions & 0 deletions files/image_config/role/role-config.service
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[Unit]
Description=Role-specific configuration
After=updategraph.service

[Service]
Type=oneshot
ExecStart=/usr/bin/role-config.sh

[Install]
WantedBy=multi-user.target
33 changes: 33 additions & 0 deletions files/image_config/role/role-config.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#!/bin/bash
#
# role-config.sh
#
# Script to perform tasks which configure device based on its role as determined by its minigraph
# To be run by role-config.service
#

# Disable DHCP relay service if device does not require it
DHCP_RELAY_SERVICE_START_FILE=/etc/sonic/role/dhcp_relay
DEVICE_ROLE=`sonic-cfggen -m /etc/sonic/minigraph.xml -v "minigraph_devices[minigraph_hostname]['type']"`

if [ $DEVICE_ROLE == "ToRRouter" ]; then
echo "Device requires DHCP relay service. Ensuring start file exists..."
touch $DHCP_RELAY_SERVICE_START_FILE
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we do systemctl disable dhcp-relay.service instead of using additional files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was my original intent because it's very clean, and I tested it first. Unfortunately, when systemd starts up, it loads all of the unit files into memory, so calling systemctl disable has no effect on the current boot - the service you disabled will still be started (or vice-versa, if you enable a disabled service, it will not be started). The changes made will not take effect until the next boot.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also feel using systemctl disable/enable is cleaner. can we by default disable those two service (teamd/dhcprelay) and only enabled them ondemand?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm afraid that @jleveque is right and using systemd enable/disable will require starting services manually or restarting device for changes to take effect.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean disable them by default, and then if we need to enable it, we can do start first and enable for the next reboot.

Copy link
Contributor Author

@jleveque jleveque Mar 30, 2017

Choose a reason for hiding this comment

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

Then the question becomes "When do we start the role-specific services?" Doing what you describe (starting the services manually) defeats the dependency mechanism in systemd. The only safe solution would be to start these services last. How would we ensure this? Also, what if in the future we have dependencies between role-specific services? That would be nearly impossible to manage manually.

Also, what happens if someone changes the minigraph, and role-specific services are no longer needed? We would have to do the inverse: stop an already-started service, which feels quite sloppy to me.

else
echo "Device does not require DHCP relay service. Deleting start file..."
rm -f $DHCP_RELAY_SERVICE_START_FILE
fi


# Disable teamd service if device does not require it
TEAMD_SERVICE_START_FILE=/etc/sonic/role/teamd
NUM_PORTCHANNELS=`sonic-cfggen -m /etc/sonic/minigraph.xml -v "minigraph_portchannels.keys() | count"`

if [ $NUM_PORTCHANNELS -gt 0 ]; then
echo "Device requires teamd service. Ensuring start file exists..."
touch $TEAMD_SERVICE_START_FILE
else
echo "Device does not require teamd service. Deleting start file..."
rm -f $TEAMD_SERVICE_START_FILE
fi

1 change: 1 addition & 0 deletions files/image_config/updategraph/updategraph.service
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[Unit]
Description=download minigraph from graph service
After=platform-init.service
Before=ntp-config.service
Before=rsyslog-config.service
Before=interfaces-config.service
Expand Down