Repository: Azure/WALinuxAgent
Pull request #2283 (2021-06-25 00:20:47)

Fix if hangs
<!-- DO NOT DELETE THIS TEMPLATE -->

## Description

Issue # <!-- if any -->
<!--
Please add an informative description that covers that changes made by the pull request. 
This checklist is used to make sure that common issues in a pull request are addressed.
This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.
-->

---

### PR information
- [ ] The title of the PR is clear and informative.
- [ ] There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, [see this page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md).
- [ ] If applicable, the PR references the bug/issue that it fixes in the description.
- [ ] New Unit tests were added for the changes made

### Quality of Code and Contribution Guidelines
- [ ] I have read the [contribution guidelines](https://github.com/Azure/WALinuxAgent/blob/master/.github/CONTRIBUTING.md).

codecov[bot]: # [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2283?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) Report
> Merging [#2283](https://codecov.io/gh/Azure/WALinuxAgent/pull/2283?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) (accd4d5) into [hotfix-2.3.1](https://codecov.io/gh/Azure/WALinuxAgent/commit/b9b2f1b172825d62987176ad5beafe9a41e09e02?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) (b9b2f1b) will **decrease** coverage by `0.03%`.
> The diff coverage is `25.00%`.

[![Impacted file tree graph](https://codecov.io/gh/Azure/WALinuxAgent/pull/2283/graphs/tree.svg?width=650&height=150&src=pr&token=ae90E2KYCw&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure)](https://codecov.io/gh/Azure/WALinuxAgent/pull/2283?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure)

```diff
@@               Coverage Diff                @@
##           hotfix-2.3.1    #2283      +/-   ##
================================================
- Coverage         70.62%   70.59%   -0.04%     
================================================
  Files                96       96              
  Lines             13652    13646       -6     
  Branches           1934     1935       +1     
================================================
- Hits               9642     9633       -9     
+ Misses             3591     3590       -1     
- Partials            419      423       +4     
```


| [Impacted Files](https://codecov.io/gh/Azure/WALinuxAgent/pull/2283?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) | Coverage Δ | |
|---|---|---|
| [azurelinuxagent/common/osutil/ubuntu.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9vc3V0aWwvdWJ1bnR1LnB5) | `55.43% <0.00%> (+0.59%)` | :arrow_up: |
| [azurelinuxagent/common/osutil/default.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9vc3V0aWwvZGVmYXVsdC5weQ==) | `60.12% <57.14%> (+<0.01%)` | :arrow_up: |
| [azurelinuxagent/ga/collect\_telemetry\_events.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2dhL2NvbGxlY3RfdGVsZW1ldHJ5X2V2ZW50cy5weQ==) | `89.72% <0.00%> (-1.72%)` | :arrow_down: |
| [azurelinuxagent/ga/exthandlers.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2dhL2V4dGhhbmRsZXJzLnB5) | `86.60% <0.00%> (-0.09%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2283?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2283?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure). Last update [b9b2f1b...accd4d5](https://codecov.io/gh/Azure/WALinuxAgent/pull/2283?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure).


Patch:
diff --git a/azurelinuxagent/common/osutil/default.py b/azurelinuxagent/common/osutil/default.py
index 066e1431..820016c1 100644
--- a/azurelinuxagent/common/osutil/default.py
+++ b/azurelinuxagent/common/osutil/default.py
@@ -1163,25 +1163,15 @@ class DefaultOSUtil(object):
     def restart_if(self, ifname, retries=3, wait=5):
         retry_limit = retries + 1
         for attempt in range(1, retry_limit):
-            try:
-                shellutil.run_command(["ifdown", ifname])
-                shellutil.run_command(["ifup", ifname])
+            return_code = shellutil.run("ifdown {0} && ifup {0}".format(ifname), expected_errors=[1] if attempt < retries else [])
+            if return_code == 0:
                 return
-            except shellutil.CommandError as cmd_err:
-                
-                msg = "failed to restart {0}: returncode={1}\n[stdout]{2}\n\n[stderr]{3}\n"\
-                    .format(ifname, cmd_err.returncode, cmd_err.stdout, cmd_err.stderr)
-                
-                if cmd_err.returncode == 1:
-                    logger.info(msg)
-                else:
-                    logger.warn(msg)
-
-                if attempt < retry_limit:
-                    logger.info("retrying in {0} seconds".format(wait))
-                    time.sleep(wait)
-                else:
-                    logger.warn("exceeded restart retries")
+            logger.warn("failed to restart {0}: return code {1}".format(ifname, return_code))
+            if attempt < retry_limit:
+                logger.info("retrying in {0} seconds".format(wait))
+                time.sleep(wait)
+            else:
+                logger.warn("exceeded restart retries")
 
     def publish_hostname(self, hostname):
         self.set_dhcp_hostname(hostname)
diff --git a/azurelinuxagent/common/osutil/ubuntu.py b/azurelinuxagent/common/osutil/ubuntu.py
index 249e1120..5a21511c 100644
--- a/azurelinuxagent/common/osutil/ubuntu.py
+++ b/azurelinuxagent/common/osutil/ubuntu.py
@@ -142,19 +142,17 @@ class UbuntuOSUtil(Ubuntu16OSUtil):
         Restart an interface by bouncing the link. systemd-networkd observes
         this event, and forces a renew of DHCP.
         """
-        retry_limit=retries+1
+        retry_limit = retries+1
         for attempt in range(1, retry_limit):
-            try:
-                shellutil.run_command(["ip", "link", "set", ifname, "down"])
-                shellutil.run_command(["ip", "link", "set", ifname, "up"])
-
-            except shellutil.CommandError as cmd_err:
-                logger.warn("failed to restart {0}: return code {1}".format(ifname, cmd_err.returncode))
-                if attempt < retry_limit:
-                    logger.info("retrying in {0} seconds".format(wait))
-                    time.sleep(wait)
-                else:
-                    logger.warn("exceeded restart retries")
+            return_code = shellutil.run("ip link set {0} down && ip link set {0} up".format(ifname))
+            if return_code == 0:
+                return
+            logger.warn("failed to restart {0}: return code {1}".format(ifname, return_code))
+            if attempt < retry_limit:
+                logger.info("retrying in {0} seconds".format(wait))
+                time.sleep(wait)
+            else:
+                logger.warn("exceeded restart retries")
 
 
 class UbuntuSnappyOSUtil(Ubuntu14OSUtil):


Test patch:
diff --git a/tests/common/osutil/test_default.py b/tests/common/osutil/test_default.py
index 65d7ae0f..d6eae68f 100644
--- a/tests/common/osutil/test_default.py
+++ b/tests/common/osutil/test_default.py
@@ -49,20 +49,15 @@ class TestOSUtil(AgentTestCase):
         # setup
         retries = 3
         ifname = 'dummy'
-        with patch.object(shellutil, "run_command") as run_patch:
-            run_patch.side_effect = shellutil.CommandError("ifupdown dummy", 1, "", "")
+        with patch.object(shellutil, "run") as run_patch:
+            run_patch.return_value = 1
 
             # execute
             osutil.DefaultOSUtil.restart_if(osutil.DefaultOSUtil(), ifname=ifname, retries=retries, wait=0)
 
             # assert
             self.assertEqual(run_patch.call_count, retries)
-            cmd_queue = list(args[0] for (args, _) in run_patch.call_args_list)            
-            while cmd_queue:
-                self.assertEqual(cmd_queue.pop(0), ["ifdown", ifname])
-                # We don't expect the following command to be called because 'dummy' does
-                # not exist.
-                self.assertNotEqual(cmd_queue[0] if cmd_queue else None, ["ifup", ifname])
+            self.assertEqual(run_patch.call_args_list[0][0][0], 'ifdown {0} && ifup {0}'.format(ifname))
                 
     def test_get_dvd_device_success(self):
         with patch.object(os, 'listdir', return_value=['cpu', 'cdrom0']):