Fix the exit code return issue in shell scripts by wangxin · Pull Request #1107 · sonic-net/sonic-mgmt

wangxin · 2019-09-10T07:52:55Z

Description of PR

Summary:
Fixes # (issue)

In shell script, by default exit code of the last command
in commands connected by pipe is returned. If previous
command returned none zero exit code, the error is
not captured. If ansible run such script in playbook, the
script issue is ignored and ansible is not aware of it.

For example, block in fdb.yml:

  - name: Remove existing IPs from PTF host
    script: roles/test/files/helpers/remove_ip.sh
    delegate_to: "{{ptf_host}}"

fails to detect issue which looks in log like this:

02:09:48 TASK [test : Remove existing IPs from PTF host] ********************************
02:09:48 task path: /builds2/jenkins2/workspace/SONiC_Test-FDB/ansible/roles/test/tasks/fdb.yml:11
02:09:48 Thursday 25 July 2019  23:09:48 +0000 (0:00:00.297)       0:00:13.829 ********* 
02:09:48 <10.213.103.5> ESTABLISH SSH CONNECTION FOR USER: root
02:09:48 <10.213.103.5> SSH: ansible.cfg set ssh_args: (-o)(ControlMaster=auto)(-o)(ControlPersist=120s)(-o)(UserKnownHostsFile=/dev/null)
02:09:48 <10.213.103.5> SSH: ANSIBLE_HOST_KEY_CHECKING/host_key_checking disabled: (-o)(StrictHostKeyChecking=no)
02:09:48 <10.213.103.5> SSH: ANSIBLE_REMOTE_USER/remote_user/ansible_user/user/-u set: (-o)(User=root)
02:09:48 <10.213.103.5> SSH: ANSIBLE_TIMEOUT/timeout set: (-o)(ConnectTimeout=30)
02:09:48 <10.213.103.5> SSH: PlayContext set ssh_common_args: ()
02:09:48 <10.213.103.5> SSH: PlayContext set ssh_extra_args: ()
02:09:48 <10.213.103.5> SSH: found only ControlPersist; added ControlPath: (-o)(ControlPath=/root/.ansible/cp/ansible-ssh-%h-%p-%r)
02:09:48 <10.213.103.5> SSH: EXEC sshpass -d14 ssh -C -vvv -o ControlMaster=auto -o ControlPersist=120s -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o User=root -o ConnectTimeout=30 -o ControlPath=/root/.ansible/cp/ansible-ssh-%h-%p-%r -tt 10.213.103.5 'mkdir -p "$( echo $HOME/.ansible/tmp/ansible-tmp-1564096188.49-246685053741557 )" && echo "$( echo $HOME/.ansible/tmp/ansible-tmp-1564096188.49-246685053741557 )"'
02:10:11 <10.213.103.5> PUT /builds2/jenkins2/workspace/SONiC_Test-FDB/ansible/roles/test/files/helpers/remove_ip.sh TO /root/.ansible/tmp/ansible-tmp-1564096188.49-246685053741557/remove_ip.sh
02:10:11 <10.213.103.5> SSH: ansible.cfg set ssh_args: (-o)(ControlMaster=auto)(-o)(ControlPersist=120s)(-o)(UserKnownHostsFile=/dev/null)
02:10:11 <10.213.103.5> SSH: ANSIBLE_HOST_KEY_CHECKING/host_key_checking disabled: (-o)(StrictHostKeyChecking=no)
02:10:11 <10.213.103.5> SSH: ANSIBLE_REMOTE_USER/remote_user/ansible_user/user/-u set: (-o)(User=root)
02:10:11 <10.213.103.5> SSH: ANSIBLE_TIMEOUT/timeout set: (-o)(ConnectTimeout=30)
02:10:11 <10.213.103.5> SSH: PlayContext set ssh_common_args: ()
02:10:11 <10.213.103.5> SSH: PlayContext set sftp_extra_args: ()
02:10:11 <10.213.103.5> SSH: found only ControlPersist; added ControlPath: (-o)(ControlPath=/root/.ansible/cp/ansible-ssh-%h-%p-%r)
02:10:11 <10.213.103.5> SSH: EXEC sshpass -d14 sftp -b - -C -vvv -o ControlMaster=auto -o ControlPersist=120s -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o User=root -o ConnectTimeout=30 -o ControlPath=/root/.ansible/cp/ansible-ssh-%h-%p-%r '[10.213.103.5]'
02:10:11 <10.213.103.5> ESTABLISH SSH CONNECTION FOR USER: root
02:10:11 <10.213.103.5> SSH: ansible.cfg set ssh_args: (-o)(ControlMaster=auto)(-o)(ControlPersist=120s)(-o)(UserKnownHostsFile=/dev/null)
02:10:11 <10.213.103.5> SSH: ANSIBLE_HOST_KEY_CHECKING/host_key_checking disabled: (-o)(StrictHostKeyChecking=no)
02:10:11 <10.213.103.5> SSH: ANSIBLE_REMOTE_USER/remote_user/ansible_user/user/-u set: (-o)(User=root)
02:10:11 <10.213.103.5> SSH: ANSIBLE_TIMEOUT/timeout set: (-o)(ConnectTimeout=30)
02:10:11 <10.213.103.5> SSH: PlayContext set ssh_common_args: ()
02:10:11 <10.213.103.5> SSH: PlayContext set ssh_extra_args: ()
02:10:11 <10.213.103.5> SSH: found only ControlPersist; added ControlPath: (-o)(ControlPath=/root/.ansible/cp/ansible-ssh-%h-%p-%r)
02:10:11 <10.213.103.5> SSH: EXEC sshpass -d14 ssh -C -vvv -o ControlMaster=auto -o ControlPersist=120s -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o User=root -o ConnectTimeout=30 -o ControlPath=/root/.ansible/cp/ansible-ssh-%h-%p-%r -tt 10.213.103.5 'chmod +rx /root/.ansible/tmp/ansible-tmp-1564096188.49-246685053741557/remove_ip.sh'
02:10:11 <10.213.103.5> ESTABLISH SSH CONNECTION FOR USER: root
02:10:11 <10.213.103.5> SSH: ansible.cfg set ssh_args: (-o)(ControlMaster=auto)(-o)(ControlPersist=120s)(-o)(UserKnownHostsFile=/dev/null)
02:10:11 <10.213.103.5> SSH: ANSIBLE_HOST_KEY_CHECKING/host_key_checking disabled: (-o)(StrictHostKeyChecking=no)
02:10:11 <10.213.103.5> SSH: ANSIBLE_REMOTE_USER/remote_user/ansible_user/user/-u set: (-o)(User=root)
02:10:11 <10.213.103.5> SSH: ANSIBLE_TIMEOUT/timeout set: (-o)(ConnectTimeout=30)
02:10:11 <10.213.103.5> SSH: PlayContext set ssh_common_args: ()
02:10:11 <10.213.103.5> SSH: PlayContext set ssh_extra_args: ()
02:10:11 <10.213.103.5> SSH: found only ControlPersist; added ControlPath: (-o)(ControlPath=/root/.ansible/cp/ansible-ssh-%h-%p-%r)
02:10:11 <10.213.103.5> SSH: EXEC sshpass -d14 ssh -C -vvv -o ControlMaster=auto -o ControlPersist=120s -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o User=root -o ConnectTimeout=30 -o ControlPath=/root/.ansible/cp/ansible-ssh-%h-%p-%r -tt 10.213.103.5 'LANG=C LC_ALL=C LC_MESSAGES=C /root/.ansible/tmp/ansible-tmp-1564096188.49-246685053741557/remove_ip.sh '
02:10:11 <10.213.103.5> ESTABLISH SSH CONNECTION FOR USER: root
02:10:11 <10.213.103.5> SSH: ansible.cfg set ssh_args: (-o)(ControlMaster=auto)(-o)(ControlPersist=120s)(-o)(UserKnownHostsFile=/dev/null)
02:10:11 <10.213.103.5> SSH: ANSIBLE_HOST_KEY_CHECKING/host_key_checking disabled: (-o)(StrictHostKeyChecking=no)
02:10:11 <10.213.103.5> SSH: ANSIBLE_REMOTE_USER/remote_user/ansible_user/user/-u set: (-o)(User=root)
02:10:11 <10.213.103.5> SSH: ANSIBLE_TIMEOUT/timeout set: (-o)(ConnectTimeout=30)
02:10:11 <10.213.103.5> SSH: PlayContext set ssh_common_args: ()
02:10:11 <10.213.103.5> SSH: PlayContext set ssh_extra_args: ()
02:10:11 <10.213.103.5> SSH: found only ControlPersist; added ControlPath: (-o)(ControlPath=/root/.ansible/cp/ansible-ssh-%h-%p-%r)
02:10:11 <10.213.103.5> SSH: EXEC sshpass -d14 ssh -C -vvv -o ControlMaster=auto -o ControlPersist=120s -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o User=root -o ConnectTimeout=30 -o ControlPath=/root/.ansible/cp/ansible-ssh-%h-%p-%r -tt 10.213.103.5 'rm -f -r /root/.ansible/tmp/ansible-tmp-1564096188.49-246685053741557/ > /dev/null 2>&1'
02:10:11 changed: [r-anaconda-10-t0 -> 10.213.103.5] => {"changed": true, "invocation": {"module_args": {"_raw_params": "roles/test/files/helpers/remove_ip.sh"}, "module_name": "script"}, "rc": 0, "stderr": "OpenSSH_7.2p2 Ubuntu-4ubuntu2.4, OpenSSL 1.0.2g  1 Mar 2016\r\ndebug1: Reading configuration data /root/.ssh/config\r\ndebug1: /root/.ssh/config line 1: Applying options for *\r\ndebug1: Reading configuration data /etc/ssh/ssh_config\r\ndebug1: /etc/ssh/ssh_config line 19: Applying options for *\r\ndebug1: auto-mux: Trying existing master\r\ndebug2: fd 3 setting O_NONBLOCK\r\ndebug2: mux_client_hello_exchange: master version 4\r\ndebug3: mux_client_forwards: request forwardings: 0 local, 0 remote\r\ndebug3: mux_client_request_session: entering\r\ndebug3: mux_client_request_alive: entering\r\ndebug3: mux_client_request_alive: done pid = 43070\r\ndebug3: mux_client_request_session: session request sent\r\ndebug1: mux_client_request_session: master session id: 2\r\ndebug3: mux_client_read_packet: read header failed: Broken pipe\r\ndebug2: Received exit status from master 0\r\nShared connection to 10.213.103.5 closed.\r\n", "stdout": "Option \"-br\" is unknown, try \"ip -help\".\r\n", "stdout_lines": ["Option \"-br\" is unknown, try \"ip -help\"."]}
02:10:11 
02:10:11 TASK [test : Set unique MACs to PTF interfaces] ********************************
02:10:11 task path: /builds2/jenkins2/workspace/SONiC_Test-FDB/ansible/roles/test/tasks/fdb.yml:15
...

Error message is "Option "-br" is unknown, try "ip -help". But test continues because return code of the script is 0.

The fix is to add "set -o pipefail" in shell scripts. And put the commands embedded in for loop statement to a separate statement. Then the script can exit early and return none zero exit code if the "ip" command failed.

Type of change

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

Approach

How did you do it?

The fix is to add "set -o pipefail" in shell scripts. And
put the commands embedded in for loop statement
to a separate statement.

How did you verify/test it?

Tested on Mellanox platform.

Any platform specific information?

No

Supported testbed topology if it's a new test case?

NA

Documentation

In shell script, by default exit code of the last command in commands connected by pipe is returned. If previous command returned none zero exit code, the error is not captured. If ansible run such script in playbook, the script issue is ignored and ansible is not aware of it. The fix is to add "set -o pipefail" in shell scripts. And put the commands embedded in for loop statement to a separate statement. Signed-off-by: Xin Wang <xinw@mellanox.com>

tests/scripts/remove_ip.sh

keboliu

as comments

yxieca · 2019-09-17T00:57:59Z

@wangxin can you create a separate PR for 201811 branch or advise dependencies to be picked? This PR doesn't cherry-pick cleanly.

wangxin · 2019-09-17T03:37:56Z

@yxieca I have created a separate PR for 201811 branch: #1117

yxieca · 2019-09-17T05:39:09Z

Thanks Xin! Appreciate the quick response! On Sep 16, 2019 8:38 PM, Xin Wang <notifications@github.com> wrote: @yxieca<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fyxieca&data=02%7C01%7Cying.xie%40microsoft.com%7C6d411fd3dbd84080685a08d73b2070da%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637042882830271000&sdata=VFaTeUCELSEmtjjwCxFS2idcUgTWKM1CIef6m7zMvjg%3D&reserved=0> I have created a separate PR for 201811 branch: #1117<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAzure%2Fsonic-mgmt%2Fpull%2F1117&data=02%7C01%7Cying.xie%40microsoft.com%7C6d411fd3dbd84080685a08d73b2070da%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637042882830280995&sdata=E16vnrd%2FxrCa6a6hIsl2%2Fmm8mV2XVF%2FSoS6mCg%2FbAgo%3D&reserved=0> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAzure%2Fsonic-mgmt%2Fpull%2F1107%3Femail_source%3Dnotifications%26email_token%3DAEPCO6MWVGNH6EVRLIGU3XDQKBGJRA5CNFSM4IVEGYVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD63FXFI%23issuecomment-532044693&data=02%7C01%7Cying.xie%40microsoft.com%7C6d411fd3dbd84080685a08d73b2070da%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637042882830280995&sdata=YEkKm5T6%2FfV4NETJ9bQFwrO5tA0ZmGuC1Bp2JKr5Ej4%3D&reserved=0>, or mute the thread<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAEPCO6PSQQIBTUJPKVXQHVTQKBGJRANCNFSM4IVEGYVA&data=02%7C01%7Cying.xie%40microsoft.com%7C6d411fd3dbd84080685a08d73b2070da%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637042882830280995&sdata=SYTg4SfX%2B2w4JUYIjsw7jnKzgR%2BWgAqwIbblFQh7fYU%3D&reserved=0>.

…omatically (#24670) #### Why I did it src/sonic-swss-common ``` * 36f40a1 - (HEAD -> 202405, origin/202405) Automated agent pool migration for branch 202405 (sonic-net#1107) (21 hours ago) [yijingyan2] ``` #### How I did it #### How to verify it #### Description for the changelog

Revert "Revert " [201911]show interface counters for multi ASIC devices (sonic-net#1104)"" Revert "Revert "Pfcstat (sonic-net#1097)"" [show] Fix 'show int neighbor expected' (sonic-net#1106) Update argument for docker exec it->i (sonic-net#1118) Update to make config load/reload backward compatible. (sonic-net#1115) Handling deletion of Port Channel before deletion of its members (sonic-net#1062) Skip default route present in ASIC-DB but not in APP-DB. (sonic-net#1107) [CLI][PFCWD][Multi-ASIC] Added multi ASIC support to 'pfcwd' CLI (sonic-net#1102) [201911] Multi asic platform config interface portchannel, show transceiver (sonic-net#1087) [drop counters] Fix configuration for counters with lowercase names (sonic-net#1103) Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>

wangxin changed the title ~~Improve exit code return in shell scripts~~ Fix the exit code return issue in shell scripts Sep 11, 2019

keboliu reviewed Sep 11, 2019

View reviewed changes

tests/scripts/remove_ip.sh Outdated Show resolved Hide resolved

keboliu requested changes Sep 11, 2019

View reviewed changes

Restore code that was overwritten by copy and paste

4a8e75c

keboliu approved these changes Sep 11, 2019

View reviewed changes

liat-grozovik added the Request for 201811 branch label Sep 16, 2019

liat-grozovik approved these changes Sep 16, 2019

View reviewed changes

liat-grozovik merged commit ec67257 into sonic-net:master Sep 16, 2019

wangxin mentioned this pull request Sep 17, 2019

Fix the exit code return issue in shell scripts - 201811 #1117

Merged

1 task

yxieca added Included in 201811 branch Request for 201811 branch and removed Request for 201811 branch labels Sep 17, 2019

wangxin deleted the capture-shell-err branch March 28, 2020 03:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the exit code return issue in shell scripts#1107

Fix the exit code return issue in shell scripts#1107
liat-grozovik merged 2 commits intosonic-net:masterfrom
wangxin:capture-shell-err

wangxin commented Sep 10, 2019

Uh oh!

Uh oh!

keboliu left a comment

Uh oh!

yxieca commented Sep 17, 2019

Uh oh!

wangxin commented Sep 17, 2019

Uh oh!

yxieca commented Sep 17, 2019 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

wangxin commented Sep 10, 2019

Description of PR

Type of change

Approach

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Uh oh!

Uh oh!

keboliu left a comment

Choose a reason for hiding this comment

Uh oh!

yxieca commented Sep 17, 2019

Uh oh!

wangxin commented Sep 17, 2019

Uh oh!

yxieca commented Sep 17, 2019 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants