[config reload] Fix config reload failure due to sonic.target job cancellation #1814

vivekrnv · 2021-09-11T04:09:42Z

Signed-off-by: Vivek Reddy Karri vkarri@nvidia.com

What I did

How I did it

How to verify it

With this change:

root@sonic:/home/admin# config reload -y -f
Running command: rm -rf /tmp/dropstat-*
Disabling container monitoring ...
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -j /etc/sonic/init_cfg.json -j /etc/sonic/config_db.json --write-to-db
Running command: /usr/local/bin/db_migrator.py -o migrate
Running command: /usr/local/bin/sonic-cfggen -d -y /etc/sonic/sonic_version.yml -t /usr/share/sonic/templates/sonic-environment.j2,/etc/sonic/sonic-environment
Restarting SONiC target ...
Enabling container monitoring ...
Reloading Monit configuration ...
Reinitializing monit daemon

In Paralell:
admin@sonic:~$ sudo systemctl start teamd.service
Failed to start teamd.service: Transaction for teamd.service/start is destructive (radv.service has 'stop' job queued, but 'start' is included in transaction).
See system logs and 'systemctl status teamd.service' for details.

Without this Change:

root@r-lionfish-16:/home/admin# config reload -y -f
Running command: rm -rf /tmp/dropstat-*
Disabling container monitoring ...
Stopping SONiC target ...
Job for sonic.target canceled.

In Paralell:
admin@sonic:~$ sudo systemctl start teamd.service.

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>

vivekrnv · 2021-09-11T04:11:18Z

@rajendra-dendukuri, please review

rajendra-dendukuri · 2021-09-11T04:53:17Z

config/main.py

@@ -686,7 +686,7 @@ def _stop_services():
        pass

    click.echo("Stopping SONiC target ...")
-    clicommon.run_command("sudo systemctl stop sonic.target")
+    clicommon.run_command("sudo systemctl stop sonic.target --job-mode replace-irreversibly")


Can we add this option also for systemctl restart sonic.target case at line#709

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>

liat-grozovik · 2021-09-12T11:27:22Z

@vivekreddynv could you please confirm this PR can be cherry picked to 202106 and 202012 cleanly? if not please create separated PRs.

vivekrnv · 2021-09-12T21:11:56Z

@vivekreddynv could you please confirm this PR can be cherry picked to 202106 and 202012 cleanly? if not please create separated PRs.

Verified Manually. The changes can be cleanly cherry-picked to 202012 & 202106

…cellation (#1814) #### What I did Fixes sonic-net/sonic-buildimage#7508

nazariig · 2021-09-13T16:15:55Z

sonic-net/sonic-buildimage#8657

v-wfarris · 2021-09-13T23:14:43Z

Where can I get the updated bin install file that comes with this fix? I would like to try it out on the Mellanox 2700 I was working with and having issues configuring due to this bug.
Thanks
Wes

vivekrnv · 2021-09-13T23:30:12Z

The submodule update for this repo has been raised here: sonic-net/sonic-buildimage#8741.

Once this gets merged you can get the image from the artifacts available here. https://dev.azure.com/mssonic/build/_build

or once the CI is completed, you can use those artifacts here: https://dev.azure.com/mssonic/build/_build/results?buildId=<buildId>&view=artifacts&type=publishedArtifacts

v-wfarris · 2021-09-13T23:33:13Z

Wonderful thanks for the info. May I ask what the eta is on the merger? No hurry I am just curious. Thanks again for your response.
Wes

nazariig · 2021-09-14T11:27:41Z

@v-wfarris there are might be some issues with that fix...so probably we will need yet another PR

nazariig · 2021-09-14T11:30:25Z

config/main.py

@@ -706,7 +706,7 @@ def _reset_failed_services():

 def _restart_services():
    click.echo("Restarting SONiC target ...")
-    clicommon.run_command("sudo systemctl restart sonic.target")
+    clicommon.run_command("sudo systemctl restart sonic.target --job-mode replace-irreversibly")


@vivekreddynv please double check this change
@stepanblyschak please elaborate on the consequences

@vivekreddynv It looks like if restart is executed with --job-mode replace-irreversibly we will still have the same issue, because the start job will be placed in the in systemd's job queue as "replace-irreverisbly" and the next config reload stop job will be discarded by systemd due to "replace-irreverisbly" of the start job leading to an error that looks smth like this:

Transaction for sonic.target/stop is destructive

Could you please double check? I think we only need to stop services with the guaranty that it will be successfully executed.

when the restart job (and all the other dependent jobs) are finished and is taken out of systemd's job queue, i don't think the next stop job will be cancelled.

However, simultaneous config reloads in the quick succession, can lead to the behavior you have said.

admin@sonic:~$ sudo systemctl restart sonic.target --job-mode replace-irreversibly admin@sonic:~$ sudo systemctl stop sonic.target --job-mode replace-irreversibly (Ran Immediately) Failed to stop sonic.target: Transaction for sonic.target/stop is destructive (ntp-config.service has 'start' job queued, but 'stop' is included in transaction). See system logs and 'systemctl status sonic.target' for details. (After all the dependent jobs of restart sonic.target are done, it works) admin@sonic:~$ sudo systemctl stop sonic.target --job-mode replace-irreversibly admin@sonic:

The problem here is that the sonic.target start is not a blocking call unlike sonic.target stop.

Nevertheless, the idea of this PR is that config reload should not fail and thus it makes sense to remove this. I'll raise a separate PR

…cellation (#1814) #### What I did Fixes sonic-net/sonic-buildimage#7508

#### What I did As discussed in this PR #1814 (comment), only the stop.job should have job-mode set to replace irreversibly. Otherwise, simultaneous config reloads in the quick succession, can lead to the behavior. Although ,when the restart job (and all the other dependent jobs) are finished and is taken out of systemd's job queue, the next stop job will not be cancelled.

* d03ba4f [202012] [portstat, intfstat] added rates and utilization (sonic-net#1812) * 499ad3f [config reload] Fix config reload failure due to sonic.target job cancellation (sonic-net#1814) * 96d658c [202012][sonic installer] Add swap setup support (sonic-net#1815) * a9c6970 platform pre-check for reboot in 202012 branch (sonic-net#1788) * 0e0478b Unify the number format in the ourput of portstat and pfcstat in all cases (sonic-net#1795) * 2d1e00e [ecnconfig] Fix exception seen during display and add unit tests (sonic-net#1784) (sonic-net#1789) Signed-off-by: Volodymyr Samotiy <volodymyrs@nvidia.com>

#### What I did As discussed in this PR sonic-net/sonic-utilities#1814 (comment), only the stop.job should have job-mode set to replace irreversibly. Otherwise, simultaneous config reloads in the quick succession, can lead to the behavior. Although ,when the restart job (and all the other dependent jobs) are finished and is taken out of systemd's job queue, the next stop job will not be cancelled.

Job Cancellation Issue fixed

ef367ab

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>

vivekrnv mentioned this pull request Sep 11, 2021

[config] config reload needs to handle exceptions while stopping sonic target #1694

Closed

rajendra-dendukuri reviewed Sep 11, 2021

View reviewed changes

Added job mode for restart sonic.target

29f9647

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>

rajendra-dendukuri approved these changes Sep 11, 2021

View reviewed changes

liat-grozovik requested a review from qiluo-msft September 12, 2021 11:26

liat-grozovik added Request for 202012 Branch Request for 202106 Branch labels Sep 13, 2021

qiluo-msft approved these changes Sep 13, 2021

View reviewed changes

qiluo-msft merged commit 2416175 into sonic-net:master Sep 13, 2021

qiluo-msft pushed a commit that referenced this pull request Sep 13, 2021

[config reload] Fix config reload failure due to sonic.target job can…

499ad3f

…cellation (#1814) #### What I did Fixes sonic-net/sonic-buildimage#7508

qiluo-msft added the Included in 202012 Branch label Sep 13, 2021

nazariig mentioned this pull request Sep 13, 2021

Mellanox 2700 Unable to configure mgmt ip and hostname info sonic-net/sonic-buildimage#8657

Closed

nazariig reviewed Sep 14, 2021

View reviewed changes

vivekrnv mentioned this pull request Sep 14, 2021

[config reload] Removed replace irreversibly job-mode for sonic.target restart #1820

Merged

judyjoseph pushed a commit that referenced this pull request Sep 14, 2021

[config reload] Fix config reload failure due to sonic.target job can…

c33e3a8

…cellation (#1814) #### What I did Fixes sonic-net/sonic-buildimage#7508

judyjoseph added the Included in 202106 Branch label Sep 14, 2021

CharlieChenEC mentioned this pull request Mar 28, 2022

Config Reload Enhancement - Introduce the Transaction Mechanism sonic-net/SONiC#964

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[config reload] Fix config reload failure due to sonic.target job cancellation #1814

[config reload] Fix config reload failure due to sonic.target job cancellation #1814

vivekrnv commented Sep 11, 2021

vivekrnv commented Sep 11, 2021

rajendra-dendukuri Sep 11, 2021

vivekrnv Sep 11, 2021

liat-grozovik commented Sep 12, 2021

vivekrnv commented Sep 12, 2021

nazariig commented Sep 13, 2021

v-wfarris commented Sep 13, 2021

vivekrnv commented Sep 13, 2021

v-wfarris commented Sep 13, 2021

nazariig commented Sep 14, 2021 •

edited

Loading

nazariig Sep 14, 2021

stepanblyschak Sep 14, 2021

vivekrnv Sep 14, 2021

[config reload] Fix config reload failure due to sonic.target job cancellation #1814

[config reload] Fix config reload failure due to sonic.target job cancellation #1814

Conversation

vivekrnv commented Sep 11, 2021

What I did

How I did it

How to verify it

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

vivekrnv commented Sep 11, 2021

rajendra-dendukuri Sep 11, 2021

Choose a reason for hiding this comment

vivekrnv Sep 11, 2021

Choose a reason for hiding this comment

liat-grozovik commented Sep 12, 2021

vivekrnv commented Sep 12, 2021

nazariig commented Sep 13, 2021

v-wfarris commented Sep 13, 2021

vivekrnv commented Sep 13, 2021

v-wfarris commented Sep 13, 2021

nazariig commented Sep 14, 2021 • edited Loading

nazariig Sep 14, 2021

Choose a reason for hiding this comment

stepanblyschak Sep 14, 2021

Choose a reason for hiding this comment

vivekrnv Sep 14, 2021

Choose a reason for hiding this comment

nazariig commented Sep 14, 2021 •

edited

Loading