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] introduce sonic.target #5705

Merged
merged 17 commits into from
Feb 25, 2021

Conversation

stepanblyschak
Copy link
Collaborator

@stepanblyschak stepanblyschak commented Oct 23, 2020

admin@r-tigon-11:~$ docker ps
CONTAINER ID        IMAGE                                COMMAND                  CREATED             STATUS              PORTS               NAMES
45cb06505f6a        docker-sonic-telemetry:latest        "/usr/bin/supervisord"   About an hour ago   Up 9 minutes                            telemetry
afb299c49d4c        docker-snmp:latest                   "/usr/bin/supervisord"   About an hour ago   Up 9 minutes                            snmp
5c75d20d71cf        docker-sonic-mgmt-framework:latest   "/usr/bin/supervisord"   About an hour ago   Up 9 minutes                            mgmt-framework
3e0563cd556f        docker-router-advertiser:latest      "/usr/bin/docker-ini…"   About an hour ago   Up 12 minutes                           radv
65f94b300db1        docker-platform-monitor:latest       "/usr/bin/docker_ini…"   About an hour ago   Up About a minute                       pmon
4ffadb5817aa        docker-dhcp-relay:latest             "/usr/bin/docker_ini…"   About an hour ago   Up 12 minutes                           dhcp_relay
65058396277c        docker-lldp:latest                   "/usr/bin/docker-lld…"   About an hour ago   Up 12 minutes                           lldp
f922b7a26b06        docker-syncd-mlnx:latest             "/usr/bin/supervisord"   About an hour ago   Up 12 minutes                           syncd
4f3aaddcacf9        docker-teamd:latest                  "/usr/bin/supervisord"   About an hour ago   Up 13 minutes                           teamd
c3622c5322b2        docker-orchagent:latest              "/usr/bin/docker-ini…"   About an hour ago   Up 13 minutes                           swss
370acc8b2289        docker-fpm-frr:latest                "/usr/bin/docker_ini…"   About an hour ago   Up 13 minutes                           bgp
16f359ad4057        docker-database:latest               "/usr/local/bin/dock…"   About an hour ago   Up 13 minutes                           database
admin@r-tigon-11:~$ sudo systemctl stop sonic.target
admin@r-tigon-11:~$ docker ps
CONTAINER ID        IMAGE                    COMMAND                  CREATED             STATUS              PORTS               NAMES
16f359ad4057        docker-database:latest   "/usr/local/bin/dock…"   About an hour ago   Up 14 minutes                           database
admin@r-tigon-11:~$ sudo systemctl restart sonic.target
admin@r-tigon-11:~$ docker ps
CONTAINER ID        IMAGE                                COMMAND                  CREATED             STATUS              PORTS               NAMES
45cb06505f6a        docker-sonic-telemetry:latest        "/usr/bin/supervisord"   About an hour ago   Up 11 seconds                           telemetry
afb299c49d4c        docker-snmp:latest                   "/usr/bin/supervisord"   About an hour ago   Up 6 seconds                            snmp
5c75d20d71cf        docker-sonic-mgmt-framework:latest   "/usr/bin/supervisord"   About an hour ago   Up 8 seconds                            mgmt-framework
3e0563cd556f        docker-router-advertiser:latest      "/usr/bin/docker-ini…"   About an hour ago   Up 16 seconds                           radv
65f94b300db1        docker-platform-monitor:latest       "/usr/bin/docker_ini…"   About an hour ago   Up 17 seconds                           pmon
4ffadb5817aa        docker-dhcp-relay:latest             "/usr/bin/docker_ini…"   About an hour ago   Up 17 seconds                           dhcp_relay
65058396277c        docker-lldp:latest                   "/usr/bin/docker-lld…"   About an hour ago   Up 17 seconds                           lldp
f922b7a26b06        docker-syncd-mlnx:latest             "/usr/bin/supervisord"   About an hour ago   Up 18 seconds                           syncd
4f3aaddcacf9        docker-teamd:latest                  "/usr/bin/supervisord"   About an hour ago   Up About a minute                       teamd
c3622c5322b2        docker-orchagent:latest              "/usr/bin/docker-ini…"   About an hour ago   Up About a minute                       swss
370acc8b2289        docker-fpm-frr:latest                "/usr/bin/docker_ini…"   About an hour ago   Up About a minute                       bgp
16f359ad4057        docker-database:latest               "/usr/local/bin/dock…"   About an hour ago   Up 17 minutes                           database

Signed-off-by: Stepan Blyschak [email protected]

- Why I did it
Group all SONiC services together and able to manage them together. Will be used in config reload command as much simpler and generic way to restart services.

- How I did it
Add services to sonic.target

- How to verify it
Together with sonic-net/sonic-utilities#1199

config reload -y

- Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

admin@r-tigon-11:~$ docker ps
CONTAINER ID        IMAGE                                COMMAND                  CREATED             STATUS              PORTS               NAMES
45cb06505f6a        docker-sonic-telemetry:latest        "/usr/bin/supervisord"   About an hour ago   Up 9 minutes                            telemetry
afb299c49d4c        docker-snmp:latest                   "/usr/bin/supervisord"   About an hour ago   Up 9 minutes                            snmp
5c75d20d71cf        docker-sonic-mgmt-framework:latest   "/usr/bin/supervisord"   About an hour ago   Up 9 minutes                            mgmt-framework
3e0563cd556f        docker-router-advertiser:latest      "/usr/bin/docker-ini…"   About an hour ago   Up 12 minutes                           radv
65f94b300db1        docker-platform-monitor:latest       "/usr/bin/docker_ini…"   About an hour ago   Up About a minute                       pmon
4ffadb5817aa        docker-dhcp-relay:latest             "/usr/bin/docker_ini…"   About an hour ago   Up 12 minutes                           dhcp_relay
65058396277c        docker-lldp:latest                   "/usr/bin/docker-lld…"   About an hour ago   Up 12 minutes                           lldp
f922b7a26b06        docker-syncd-mlnx:latest             "/usr/bin/supervisord"   About an hour ago   Up 12 minutes                           syncd
4f3aaddcacf9        docker-teamd:latest                  "/usr/bin/supervisord"   About an hour ago   Up 13 minutes                           teamd
c3622c5322b2        docker-orchagent:latest              "/usr/bin/docker-ini…"   About an hour ago   Up 13 minutes                           swss
370acc8b2289        docker-fpm-frr:latest                "/usr/bin/docker_ini…"   About an hour ago   Up 13 minutes                           bgp
16f359ad4057        docker-database:latest               "/usr/local/bin/dock…"   About an hour ago   Up 13 minutes                           database
admin@r-tigon-11:~$ sudo systemctl stop sonic.target
admin@r-tigon-11:~$ docker ps
CONTAINER ID        IMAGE                    COMMAND                  CREATED             STATUS              PORTS               NAMES
16f359ad4057        docker-database:latest   "/usr/local/bin/dock…"   About an hour ago   Up 14 minutes                           database
admin@r-tigon-11:~$ sudo systemctl restart sonic.target
admin@r-tigon-11:~$ docker ps
CONTAINER ID        IMAGE                                COMMAND                  CREATED             STATUS              PORTS               NAMES
45cb06505f6a        docker-sonic-telemetry:latest        "/usr/bin/supervisord"   About an hour ago   Up 11 seconds                           telemetry
afb299c49d4c        docker-snmp:latest                   "/usr/bin/supervisord"   About an hour ago   Up 6 seconds                            snmp
5c75d20d71cf        docker-sonic-mgmt-framework:latest   "/usr/bin/supervisord"   About an hour ago   Up 8 seconds                            mgmt-framework
3e0563cd556f        docker-router-advertiser:latest      "/usr/bin/docker-ini…"   About an hour ago   Up 16 seconds                           radv
65f94b300db1        docker-platform-monitor:latest       "/usr/bin/docker_ini…"   About an hour ago   Up 17 seconds                           pmon
4ffadb5817aa        docker-dhcp-relay:latest             "/usr/bin/docker_ini…"   About an hour ago   Up 17 seconds                           dhcp_relay
65058396277c        docker-lldp:latest                   "/usr/bin/docker-lld…"   About an hour ago   Up 17 seconds                           lldp
f922b7a26b06        docker-syncd-mlnx:latest             "/usr/bin/supervisord"   About an hour ago   Up 18 seconds                           syncd
4f3aaddcacf9        docker-teamd:latest                  "/usr/bin/supervisord"   About an hour ago   Up About a minute                       teamd
c3622c5322b2        docker-orchagent:latest              "/usr/bin/docker-ini…"   About an hour ago   Up About a minute                       swss
370acc8b2289        docker-fpm-frr:latest                "/usr/bin/docker_ini…"   About an hour ago   Up About a minute                       bgp
16f359ad4057        docker-database:latest               "/usr/local/bin/dock…"   About an hour ago   Up 17 minutes                           database

Signed-off-by: Stepan Blyschak <[email protected]>
@yxieca
Copy link
Contributor

yxieca commented Oct 23, 2020

What is the behavior if we issue systemctl start sonic.target when it is already started?

Particularly, will it restart all 'wanted' services? What if the downstream service is started? What if downstream service is a oneshot timer which will start another service?

@stepanblyschak
Copy link
Collaborator Author

@yxieca normal services are not touched, one-shot and timers get started, so delayed services restart as well as one-shot services.
I am still testing all the flows and need to fix few more issues with delayed services I found.

@jleveque
Copy link
Contributor

'Conficts=' in timer units caused a problem when
starting a timer causes restart of a service.
Instead, 'PartOf=' is a better choice.
'PartOf=' will make the timer stop and restart togather
with the service the timer is bound to, otherwise the timer
OnUnitActiveSec expires imidiatelly when the service stops
and starts it.

Signed-off-by: Stepan Blyshchak <[email protected]>
To prevent restarts on 'systemctl start <service>'.

Signed-off-by: Stepan Blyshchak <[email protected]>
admin@r-tigon-11:~$ docker ps
CONTAINER ID        IMAGE                                COMMAND                  CREATED             STATUS              PORTS               NAMES
45cb06505f6a        docker-sonic-telemetry:latest        "/usr/bin/supervisord"   About an hour ago   Up 9 minutes                            telemetry
afb299c49d4c        docker-snmp:latest                   "/usr/bin/supervisord"   About an hour ago   Up 9 minutes                            snmp
5c75d20d71cf        docker-sonic-mgmt-framework:latest   "/usr/bin/supervisord"   About an hour ago   Up 9 minutes                            mgmt-framework
3e0563cd556f        docker-router-advertiser:latest      "/usr/bin/docker-ini…"   About an hour ago   Up 12 minutes                           radv
65f94b300db1        docker-platform-monitor:latest       "/usr/bin/docker_ini…"   About an hour ago   Up About a minute                       pmon
4ffadb5817aa        docker-dhcp-relay:latest             "/usr/bin/docker_ini…"   About an hour ago   Up 12 minutes                           dhcp_relay
65058396277c        docker-lldp:latest                   "/usr/bin/docker-lld…"   About an hour ago   Up 12 minutes                           lldp
f922b7a26b06        docker-syncd-mlnx:latest             "/usr/bin/supervisord"   About an hour ago   Up 12 minutes                           syncd
4f3aaddcacf9        docker-teamd:latest                  "/usr/bin/supervisord"   About an hour ago   Up 13 minutes                           teamd
c3622c5322b2        docker-orchagent:latest              "/usr/bin/docker-ini…"   About an hour ago   Up 13 minutes                           swss
370acc8b2289        docker-fpm-frr:latest                "/usr/bin/docker_ini…"   About an hour ago   Up 13 minutes                           bgp
16f359ad4057        docker-database:latest               "/usr/local/bin/dock…"   About an hour ago   Up 13 minutes                           database
admin@r-tigon-11:~$ sudo systemctl stop sonic.target
admin@r-tigon-11:~$ docker ps
CONTAINER ID        IMAGE                    COMMAND                  CREATED             STATUS              PORTS               NAMES
16f359ad4057        docker-database:latest   "/usr/local/bin/dock…"   About an hour ago   Up 14 minutes                           database
admin@r-tigon-11:~$ sudo systemctl restart sonic.target
admin@r-tigon-11:~$ docker ps
CONTAINER ID        IMAGE                                COMMAND                  CREATED             STATUS              PORTS               NAMES
45cb06505f6a        docker-sonic-telemetry:latest        "/usr/bin/supervisord"   About an hour ago   Up 11 seconds                           telemetry
afb299c49d4c        docker-snmp:latest                   "/usr/bin/supervisord"   About an hour ago   Up 6 seconds                            snmp
5c75d20d71cf        docker-sonic-mgmt-framework:latest   "/usr/bin/supervisord"   About an hour ago   Up 8 seconds                            mgmt-framework
3e0563cd556f        docker-router-advertiser:latest      "/usr/bin/docker-ini…"   About an hour ago   Up 16 seconds                           radv
65f94b300db1        docker-platform-monitor:latest       "/usr/bin/docker_ini…"   About an hour ago   Up 17 seconds                           pmon
4ffadb5817aa        docker-dhcp-relay:latest             "/usr/bin/docker_ini…"   About an hour ago   Up 17 seconds                           dhcp_relay
65058396277c        docker-lldp:latest                   "/usr/bin/docker-lld…"   About an hour ago   Up 17 seconds                           lldp
f922b7a26b06        docker-syncd-mlnx:latest             "/usr/bin/supervisord"   About an hour ago   Up 18 seconds                           syncd
4f3aaddcacf9        docker-teamd:latest                  "/usr/bin/supervisord"   About an hour ago   Up About a minute                       teamd
c3622c5322b2        docker-orchagent:latest              "/usr/bin/docker-ini…"   About an hour ago   Up About a minute                       swss
370acc8b2289        docker-fpm-frr:latest                "/usr/bin/docker_ini…"   About an hour ago   Up About a minute                       bgp
16f359ad4057        docker-database:latest               "/usr/local/bin/dock…"   About an hour ago   Up 17 minutes                           database

Signed-off-by: Stepan Blyschak <[email protected]>
'Conficts=' in timer units caused a problem when
starting a timer causes restart of a service.
Instead, 'PartOf=' is a better choice.
'PartOf=' will make the timer stop and restart togather
with the service the timer is bound to, otherwise the timer
OnUnitActiveSec expires imidiatelly when the service stops
and starts it.

Signed-off-by: Stepan Blyshchak <[email protected]>
To prevent restarts on 'systemctl start <service>'.

Signed-off-by: Stepan Blyshchak <[email protected]>
@stepanblyschak stepanblyschak marked this pull request as ready for review October 28, 2020 16:12
yxieca
yxieca previously approved these changes Oct 29, 2020
@jleveque
Copy link
Contributor

Please fix new conflicts, as hostcfgd has moved. Also please apply necessary changes to the other services in the new sonic-host-services package.

@jleveque
Copy link
Contributor

@stepanblyschak: Has this been tested thoroughly?

@stepanblyschak
Copy link
Collaborator Author

This was tested in different scenarios:

  • config reload
  • individual service restarts, feature state toggling
  • reboot, warm-reboot, fast-reboot

However, on recent master this change became broken because of #6115 wich introduced a cycle between ntp-config and ntp service. As a result config reload hangs forever. I am investigating further for a fix.

@jleveque
Copy link
Contributor

Retest vsimage please

@jleveque
Copy link
Contributor

Retest vsimage please

@lguohan lguohan added the appext Application Extension label Jan 22, 2021
@stepanblyschak
Copy link
Collaborator Author

Retest this please

@stepanblyschak
Copy link
Collaborator Author

/AzurePipelines run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 5705 in repo Azure/sonic-buildimage

@lguohan
Copy link
Collaborator

lguohan commented Jan 26, 2021

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

@stepanblyschak can you resolve conflicts so we can have clean build and move forward with mergint it? this is an enhancement we would like to have in 202012 as well

@liat-grozovik
Copy link
Collaborator

@rajendra-dendukuri and @yxieca i am sure i saw the approval before but for some reason it is deleted. Can you please add it again?
@lguohan do you understand why it happens?

@jleveque
Copy link
Contributor

@rajendra-dendukuri and @yxieca i am sure i saw the approval before but for some reason it is deleted. Can you please add it again?
@lguohan do you understand why it happens?

@liat-grozovik: Repo settings have changed such that when new commits are pushed to the repo any existing approvals are reset. This prevents unreviewed commits from sneaking in after an approval.

@yxieca
Copy link
Contributor

yxieca commented Feb 21, 2021

@rajendra-dendukuri and @yxieca i am sure i saw the approval before but for some reason it is deleted. Can you please add it again?
@lguohan do you understand why it happens?

@liat-grozovik: Repo settings have changed such that when new commits are pushed to the repo any existing approvals are reset. This prevents unreviewed commits from sneaking in after an approval.

I also noticed this. Seems that when new change is pushed in. github dismisses old approvals. But this doesn't happen consistently. I also see new changes added and approval sticks. I didn't figure out what was the difference.

@liat-grozovik liat-grozovik merged commit e179ec2 into sonic-net:master Feb 25, 2021
yxieca pushed a commit that referenced this pull request Mar 4, 2021
- Why I did it
Group all SONiC services together and able to manage them together. Will be used in config reload command as much simpler and generic way to restart services.

- How I did it
Add services to sonic.target

- How to verify it
Together with sonic-net/sonic-utilities#1199
config reload -y

Signed-off-by: Stepan Blyshchak <[email protected]>
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
- Why I did it
Group all SONiC services together and able to manage them together. Will be used in config reload command as much simpler and generic way to restart services.

- How I did it
Add services to sonic.target

- How to verify it
Together with sonic-net/sonic-utilities#1199
config reload -y

Signed-off-by: Stepan Blyshchak <[email protected]>
lolyu pushed a commit to lolyu/sonic-buildimage that referenced this pull request Sep 13, 2021
- Why I did it
Group all SONiC services together and able to manage them together. Will be used in config reload command as much simpler and generic way to restart services.

- How I did it
Add services to sonic.target

- How to verify it
Together with sonic-net/sonic-utilities#1199
config reload -y

Signed-off-by: Stepan Blyshchak <[email protected]>
@stepanblyschak stepanblyschak deleted the sonic_target branch September 23, 2022 13:33
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.

6 participants