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.

Sign up for GitHub

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

[services] make snmp.timer work again and delay telemetry.service #3742

Merged
merged 6 commits into from
Dec 16, 2019
Merged

[services] make snmp.timer work again and delay telemetry.service #3742

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f9fc076
[services] make snmp.timer work again and delay telemetry.service
Oct 21, 2019
b0e66f5
Merge commit 'refs/pull/3657/head' of github.com:azure/sonic-buildimage
Nov 12, 2019
8f8f1c9
[services][snmp] snmp service should stop/start/restart with
Nov 12, 2019
76504c7
[sonic_debian_extentions] add telemetry timer only when telemetry is …
Nov 12, 2019
0906255
[snmp] add newline at the end of file in service file
Nov 13, 2019
c3e7bfd
Merge branch 'master' into timers
stepanblyschak Dec 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions files/build_templates/snmp.service.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Requires=updategraph.service
Requisite=swss.service
After=updategraph.service swss.service syncd.service
Before=ntp-config.service
Conflicts=snmp.timer
StartLimitIntervalSec=1200
StartLimitBurst=3

Expand All @@ -14,5 +15,3 @@ ExecStop=/usr/bin/{{docker_container_name}}.sh stop
Restart=always
RestartSec=30

[Install]
WantedBy=multi-user.target swss.service
Copy link
Contributor

@yxieca yxieca Nov 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

By remove these 2 lines, restarting swss will no longer restarting snmp. You need to add snmp as a dependent of swss similar https://github.com/Azure/sonic-buildimage/blob/master/files/scripts/swss.sh#L5. But you need to add a logic to ignore this dependency if the uptime is less than 210 seconds.

I see that you did it with snmp.timer below.

I am wondering if we should use systemctrld to manage these dependencies that beyond the scope of systemctrld?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we will ignore snmp dependency in swss.sh if uptime is less than 210 sec, then who will start snmp? looks like we still need snmp.timer so the complexity will not decrease and we will have 210 sec hard coded in two places.
If the desired behavior can be done with systemd then why to do that in scripts?

4 changes: 3 additions & 1 deletion files/build_templates/snmp.timer
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
[Unit]
Description=Delays snmp container until SONiC has started
Conflicts=snmp.service

[Timer]
OnUnitActiveSec=0 sec
OnBootSec=3min 30 sec
Unit=snmp.service

[Install]
WantedBy=timers.target
WantedBy=timers.target swss.service
8 changes: 6 additions & 2 deletions files/build_templates/sonic_debian_extension.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ EOF
## Bind docker path
if [[ $CONFIGURED_ARCH == armhf || $CONFIGURED_ARCH == arm64 ]]; then
sudo mkdir -p $FILESYSTEM_ROOT/dockerfs
sudo mount --bind dockerfs $FILESYSTEM_ROOT/dockerfs
sudo mount --bind dockerfs $FILESYSTEM_ROOT/dockerfs
fi

{% if installer_images.strip() -%}
Expand All @@ -341,7 +341,7 @@ sudo LANG=C chroot $FILESYSTEM_ROOT docker $SONIC_NATIVE_DOCKERD_FOR_DOCKERFS ta
{% endif %}
{% endfor %}
if [[ $CONFIGURED_ARCH == armhf || $CONFIGURED_ARCH == arm64 ]]; then
sudo umount $FILESYSTEM_ROOT/dockerfs
sudo umount $FILESYSTEM_ROOT/dockerfs
sudo rm -fr $FILESYSTEM_ROOT/dockerfs
sudo kill -9 `sudo $SONIC_NATIVE_DOCKERD_FOR_DOCKERFS_PID` || true
else
Expand Down Expand Up @@ -369,6 +369,10 @@ sudo LANG=C cp $SCRIPTS_DIR/syncd.sh $FILESYSTEM_ROOT/usr/local/bin/syncd.sh
# It implements delayed start of services
sudo cp $BUILD_TEMPLATES/snmp.timer $FILESYSTEM_ROOT/etc/systemd/system/
sudo LANG=C chroot $FILESYSTEM_ROOT systemctl enable snmp.timer
{% if enable_system_telemetry == 'y' %}
sudo cp $BUILD_TEMPLATES/telemetry.timer $FILESYSTEM_ROOT/etc/systemd/system/
sudo LANG=C chroot $FILESYSTEM_ROOT systemctl enable telemetry.timer
{% endif %}

sudo LANG=C DEBIAN_FRONTEND=noninteractive chroot $FILESYSTEM_ROOT apt-get purge -y python-dev
sudo LANG=C DEBIAN_FRONTEND=noninteractive chroot $FILESYSTEM_ROOT apt-get clean -y
Expand Down
2 changes: 0 additions & 2 deletions files/build_templates/telemetry.service.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,3 @@ ExecStartPre=/usr/bin/{{docker_container_name}}.sh start
ExecStart=/usr/bin/{{docker_container_name}}.sh wait
ExecStop=/usr/bin/{{docker_container_name}}.sh stop

[Install]
WantedBy=multi-user.target
9 changes: 9 additions & 0 deletions files/build_templates/telemetry.timer
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[Unit]
Description=Delays telemetry container until SONiC has started

[Timer]
OnBootSec=3min 30 sec
Unit=telemetry.service

[Install]
WantedBy=timers.target
1 change: 1 addition & 0 deletions slave.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,7 @@ $(addprefix $(TARGET_PATH)/, $(SONIC_INSTALLERS)) : $(TARGET_PATH)/% : \
export sonic_asic_platform="$(patsubst %-$(CONFIGURED_ARCH),%,$(CONFIGURED_PLATFORM))"
export enable_organization_extensions="$(ENABLE_ORGANIZATION_EXTENSIONS)"
export enable_dhcp_graph_service="$(ENABLE_DHCP_GRAPH_SERVICE)"
export enable_system_telemetry="$(ENABLE_SYSTEM_TELEMETRY)"
export shutdown_bgp_on_start="$(SHUTDOWN_BGP_ON_START)"
export enable_pfcwd_on_start="$(ENABLE_PFCWD_ON_START)"
export installer_debs="$(addprefix $(STRETCH_DEBS_PATH)/,$($*_INSTALLS))"
Expand Down