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

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

Merged
merged 1 commit into from
Nov 6, 2019

Conversation

stepanblyschak
Copy link
Collaborator

Signed-off-by: Stepan Blyschak [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)

@stepanblyschak
Copy link
Collaborator Author

Retest vs please

@pavel-shirshov
Copy link
Contributor

retest vs please

@pavel-shirshov
Copy link
Contributor

retest vs please

1 similar comment
@lguohan
Copy link
Collaborator

lguohan commented Oct 31, 2019

retest vs please

@yxieca
Copy link
Contributor

yxieca commented Nov 1, 2019

This change broke some link between swss and snmp. I think if swss restarts, the snmp won't get restart with this change.

This link can be restored by adding snmp as a dependency service in swss service script, where we already have a few.

@yxieca yxieca self-requested a review November 1, 2019 01:53
Copy link
Contributor

@yxieca yxieca left a comment

Choose a reason for hiding this comment

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

As commented above. There is a service link between swss and snmp got broken by this change.

@pavel-shirshov
Copy link
Contributor

What do you mean by "a service link"?

@stepanblyschak
Copy link
Collaborator Author

Isn't Requisite enough?

admin@r-boxer-sw01:~$ cat /etc/systemd/system/snmp.service 
[Unit]
Description=SNMP container
Requires=updategraph.service
Requisite=swss.service
After=updategraph.service swss.service syncd.service
Before=ntp-config.service
StartLimitIntervalSec=1200
StartLimitBurst=3

[Service]
ExecStartPre=/usr/bin/snmp.sh start
ExecStart=/usr/bin/snmp.sh wait
ExecStop=/usr/bin/snmp.sh stop
Restart=always
RestartSec=30
admin@r-boxer-sw01:~$ cat /etc/systemd/system/snmp.timer
[Unit]
Description=Delays snmp container until SONiC has started

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

[Install]
WantedBy=timers.target
admin@r-boxer-sw01:~$ date
Fri Nov  1 11:59:21 UTC 2019
admin@r-boxer-sw01:~$ sudo systemctl restart swss
admin@r-boxer-sw01:~$ show log | grep systemd | grep 'Starting\|Stopping'
...
Nov  1 11:59:24.627053 r-boxer-sw01 INFO systemd[1]: Stopping DHCP relay container...
Nov  1 11:59:24.627749 r-boxer-sw01 INFO systemd[1]: Stopping SNMP container...
Nov  1 11:59:35.087238 r-boxer-sw01 INFO systemd[1]: Stopping switch state service...
Nov  1 11:59:38.222368 r-boxer-sw01 INFO systemd[1]: Stopping syncd service...
Nov  1 11:59:38.885745 r-boxer-sw01 INFO systemd[1]: Stopping Platform monitor container...
Nov  1 11:59:53.435216 r-boxer-sw01 INFO systemd[1]: Stopping TEAMD container...
Nov  1 11:59:53.940747 r-boxer-sw01 INFO systemd[1]: Stopping Router advertiser container...
Nov  1 11:59:56.403645 r-boxer-sw01 INFO systemd[1]: Starting switch state service...
Nov  1 12:00:01.941588 r-boxer-sw01 INFO systemd[1]: Starting SNMP container...
Nov  1 12:00:02.553401 r-boxer-sw01 INFO systemd[1]: Starting syncd service...
Nov  1 12:00:11.253117 r-boxer-sw01 INFO systemd[1]: Starting TEAMD container...
Nov  1 12:00:11.284966 r-boxer-sw01 INFO systemd[1]: Starting Platform monitor container...
Nov  1 12:00:14.590531 r-boxer-sw01 INFO systemd[1]: Starting DHCP relay container...
Nov  1 12:00:14.605838 r-boxer-sw01 INFO systemd[1]: Starting Router advertiser container...


@yxieca
Copy link
Contributor

yxieca commented Nov 5, 2019

Thanks @stepanblyschak , can you also test stop and start? systemctrld behaves differently between (stop/start) and (restart).

@stepanblyschak
Copy link
Collaborator Author

@yxieca Looks like working correctly as it was before 'WantedBy':

stop swss:

ov  5 17:25:06.097091 r-boxer-sw01 INFO systemd[1]: Stopping DHCP relay container...
Nov  5 17:25:06.100550 r-boxer-sw01 INFO systemd[1]: Stopping SNMP container...
Nov  5 17:25:16.672164 r-boxer-sw01 INFO systemd[1]: Stopping switch state service...
Nov  5 17:25:21.501945 r-boxer-sw01 INFO systemd[1]: Stopping syncd service...
Nov  5 17:25:34.517452 r-boxer-sw01 INFO systemd[1]: Stopping TEAMD container...
Nov  5 17:25:35.114649 r-boxer-sw01 INFO systemd[1]: Stopping Router advertiser container...

start swss:

Nov  5 17:26:33.436137 r-boxer-sw01 INFO systemd[1]: Starting switch state service...
Nov  5 17:26:38.556672 r-boxer-sw01 INFO systemd[1]: Starting TEAMD container...
Nov  5 17:26:38.558935 r-boxer-sw01 INFO systemd[1]: Starting SNMP container...
Nov  5 17:26:39.194342 r-boxer-sw01 INFO systemd[1]: Starting syncd service...
Nov  5 17:26:41.447298 r-boxer-sw01 INFO systemd[1]: Starting DHCP relay container...
Nov  5 17:26:48.537948 r-boxer-sw01 INFO systemd[1]: Starting Router advertiser container...

@yxieca
Copy link
Contributor

yxieca commented Nov 5, 2019

Thanks @stepanblyschak !

@yxieca
Copy link
Contributor

yxieca commented Nov 7, 2019

@stepanblyschak Please create an PR for this change against 201811 branch. There are merge conflicts. I suspect due to telemetry not in 201811.

@stepanblyschak
Copy link
Collaborator Author

@yxieca I just found that you were right, start of swss does not start snmp, even though I ensured that I test this behavior on changed image. This PR requires more work and I'll try what you suggested. You can revert for now.

yxieca added a commit that referenced this pull request Nov 8, 2019
lguohan pushed a commit that referenced this pull request Nov 9, 2019
zhenggen-xu pushed a commit to zhenggen-xu/sonic-buildimage that referenced this pull request Jan 10, 2020
zhenggen-xu pushed a commit to zhenggen-xu/sonic-buildimage that referenced this pull request Jan 10, 2020
prsunny pushed a commit that referenced this pull request Mar 18, 2020
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.

4 participants