Skip to content

[reload config] redirect reload config output to /dev/null#1687

Merged
yxieca merged 1 commit intosonic-net:masterfrom
yxieca:recover
May 20, 2020
Merged

[reload config] redirect reload config output to /dev/null#1687
yxieca merged 1 commit intosonic-net:masterfrom
yxieca:recover

Conversation

@yxieca
Copy link
Collaborator

@yxieca yxieca commented May 19, 2020 •
edited
Loading

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Approach

How did you do it?

config reload command will restart interface-config service, which
will reset eth0. Which will reset the connection issues config
reload if the output is coming to stdio/stderr. It in turn could
cause the pytest to stuck for long time.

change .command to .shell so that we could redirect output to /dev/null (command doesn't allow &> operation).

Signed-off-by: Ying Xie [email protected]

How did you verify/test it?

This is a first step enhancement of config reload/load_minigraph operations. This change greatly reduced the chance of these commands getting stuck. However, I believe they still get stuck from time to time. Some further enhancement is needed to fully address the issue.

@yxieca
[reload config] redirect reload config output to /dev/null
1a0ae55 
config reload command will restart interface-config service, which
will reset eth0. Which will reset the connection issues config
reload if the output is coming to stdio/stderr. It in turn could
cause the pytest to stuck for long time.

Signed-off-by: Ying Xie <[email protected]>
@yxieca yxieca merged commit bd6e135 into sonic-net:master May 20, 2020
@yxieca yxieca deleted the recover branch May 20, 2020 15:12
stepanblyschak pushed a commit to stepanblyschak/sonic-mgmt that referenced this pull request May 27, 2020
[reload config] use /bin/bash as shell for config reload
dfb36d8 
This change was introduced in
sonic-net#1687 to fix connection reset
caused by 'config reload'. The fix was to redirect output to /dev/null.
Shell will interpret 'config reload -y &> /dev/null' as request to run
the command in the background instead of redirecting stdout/stderr to
/dev/null. It looks like not the redirection fixed the issue but the
fact that it was running in background. However, it introduced another
issue because config reload takes ~1.5m so that 90/120 sec wait time is
not enough time waiting for system to become ready.

Change-Id: I9d44c0a55e4fa016bbe177524891c9b14302c761
Signed-off-by: Stepan Blyschak <[email protected]>
@stepanblyschak stepanblyschak mentioned this pull request May 27, 2020
Merged
3 tasks
yxieca pushed a commit that referenced this pull request Jun 1, 2020
@stepanblyschak
[reload config] use /bin/bash as shell for config reload (#1705)
031dd2a 
This change was introduced in
#1687 to fix connection reset
caused by 'config reload'. The fix was to redirect output to /dev/null.
Shell will interpret 'config reload -y &> /dev/null' as request to run
the command in the background instead of redirecting stdout/stderr to
/dev/null. It looks like not the redirection fixed the issue but the
fact that it was running in background. However, it introduced another
issue because config reload takes ~1.5m so that 90/120 sec wait time is
not enough time waiting for system to become ready.

Change-Id: I9d44c0a55e4fa016bbe177524891c9b14302c761
Signed-off-by: Stepan Blyschak <[email protected]>
shlomibitton pushed a commit to Junchao-Mellanox/sonic-mgmt that referenced this pull request Jul 12, 2020
Squashed commit of the following:
5e7f7a8 
commit d79d500
Author: Xin Wang <[email protected]>
Date:   Sat May 23 11:31:48 2020 +0800

    Fix acl testing for new t1 routes (sonic-net#1688)

    The t1 and t1-lag routes have been changed after replacing static
    routes on VM with exabgp announced routes. This caused the acl
    testing failed. This PR is to fix this issue reported in sonic-net#1603.

    Changes:
    1. Update the acl rules template to use new IP addresses
    2. Update the acl script to use new IP addresses

    Signed-off-by: Xin Wang <[email protected]>

commit c84e699
Author: yvolynets-mlnx <[email protected]>
Date:   Sat May 23 04:41:19 2020 +0300

    [pytest/ntp] Minor fix for NTP test case. (sonic-net#1698)

    Added verification of 'wait_until' result, to easier understand test case failure reason.

    Signed-off-by: Yuriy Volynets <[email protected]>

commit 441ddfa
Author: Vitaliy Senchyshyn <[email protected]>
Date:   Sat May 23 00:40:02 2020 +0300

    [wr_arp][pytest] Fixed wr_arp test (sonic-net#1695)

    * [wr_arp] Fixed wr_arp test

    Signed-off-by: Vitaliy Senchyshyn <[email protected]>

    * Make show ip protocol command working for different sonic images

    * Fixed review comments

    * Made records variable a class memeber

    * Fixed DIP taking from ip route show for sonic master

commit abdb746
Author: Xin Wang <[email protected]>
Date:   Fri May 22 18:45:56 2020 +0800

    [pytest/fib]: Fix the issue of using wrong port announcing IPv6 routes (sonic-net#1697)

    I used wrong port for announcing IPv6 routes in PR sonic-net#1692. This PR is to fix that issue.

    Signed-off-by: Xin Wang <[email protected]>

commit 9e52bba
Author: lguohan <[email protected]>
Date:   Fri May 22 03:45:14 2020 -0700

    [pytest/snmp_cpu]: query nproc when ansible facts does not have vcpu info (sonic-net#1693)

    sometimes, ansible facts does not gather vcpu information. in this case, use nproc to query the cpu number directly.

    Signed-off-by: Guohan Lu <[email protected]>

commit b175f8f
Author: yvolynets-mlnx <[email protected]>
Date:   Fri May 22 02:49:30 2020 +0300

    [pytest/test_drop_counters.py] Fixed IGMP test case (sonic-net#1624)

    Signed-off-by: Yuriy Volynets <[email protected]>

commit 208d02a
Author: Xin Wang <[email protected]>
Date:   Fri May 22 00:23:19 2020 +0800

    Announce vips routes for bgp_multipath_relax testing (sonic-net#1692)

    Previously the vips routes were statically configured in the VMs for
    bgp_multipath_relax testing. After the change of announcing
    routes from exabgp in PTF to VMs, the vips routes were missing.
    The bgp_multipath_relax testing depends on the vips routes failed.

    This change is to announce the vips routes to VMs  that have the
    vips routes configuration. The functions in the fib fixture definition
    file were refactored a little bit.

    Signed-off-by: Xin Wang <[email protected]>

commit 6d7498f
Author: yvolynets-mlnx <[email protected]>
Date:   Thu May 21 11:38:08 2020 +0300

    Fixed fib tests: test_hash_ipv4/test_hash_ipv6 (sonic-net#1683)

    Fixes sonic-net#1679

    Signed-off-by: Yuriy Volynets <[email protected]>

commit f17471c
Author: Neetha John <[email protected]>
Date:   Wed May 20 15:53:44 2020 -0700

    Qos SAI test restructure (sonic-net#1682)

    * Qos SAI test restructure

    Signed-off-by: Neetha John <[email protected]>

commit 87f3a23
Author: Joe LeVeque <[email protected]>
Date:   Wed May 20 10:42:22 2020 -0700

    [pytest][cacl] Remove unused 'testbed_devices' from parameters (sonic-net#1689)

commit bd6e135
Author: Ying Xie <[email protected]>
Date:   Wed May 20 08:11:55 2020 -0700

    [reload config] redirect reload config output to /dev/null (sonic-net#1687)

    config reload command will restart interface-config service, which
    will reset eth0. Which will reset the connection issues config
    reload if the output is coming to stdio/stderr. It in turn could
    cause the pytest to stuck for long time.

    Signed-off-by: Ying Xie <[email protected]>

Signed-off-by: Stepan Blyschak <[email protected]>
shlomibitton pushed a commit to Junchao-Mellanox/sonic-mgmt that referenced this pull request Jul 12, 2020
@stepanblyschak @keboliu
[reload config] use /bin/bash as shell for config reload (sonic-net#1705
eeda925 
)

This change was introduced in
sonic-net#1687 to fix connection reset
caused by 'config reload'. The fix was to redirect output to /dev/null.
Shell will interpret 'config reload -y &> /dev/null' as request to run
the command in the background instead of redirecting stdout/stderr to
/dev/null. It looks like not the redirection fixed the issue but the
fact that it was running in background. However, it introduced another
issue because config reload takes ~1.5m so that 90/120 sec wait time is
not enough time waiting for system to become ready.

Change-Id: I9d44c0a55e4fa016bbe177524891c9b14302c761
Signed-off-by: Stepan Blyschak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wangxin wangxin Awaiting requested review from wangxin

@neethajohn neethajohn Awaiting requested review from neethajohn

2 more reviewers

@daall daall daall approved these changes

@tahmed-dev tahmed-dev tahmed-dev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Enhancement Pytest 🐍

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yxieca @daall @tahmed-dev