From 1251149ee0ad3eff8289fd28babfaa5d5119c965 Mon Sep 17 00:00:00 2001 From: Vaibhav Hemant Dixit Date: Wed, 28 Oct 2020 12:29:02 -0700 Subject: [PATCH 1/2] Upgrade path fixes and improvements --- .../library/reduce_and_add_sonic_images.py | 19 ++++++++++++++----- .../test/files/ptftests/advanced-reboot.py | 8 ++++---- tests/common/fixtures/advanced_reboot.py | 2 ++ tests/upgrade_path/test_upgrade_path.py | 13 ++++++++++++- 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/ansible/library/reduce_and_add_sonic_images.py b/ansible/library/reduce_and_add_sonic_images.py index 9d8fd6c2f7..fcdd29b809 100644 --- a/ansible/library/reduce_and_add_sonic_images.py +++ b/ansible/library/reduce_and_add_sonic_images.py @@ -30,7 +30,9 @@ def exec_command(module, cmd, ignore_error=False, msg="executing command"): rc, out, err = module.run_command(cmd) - if not ignore_error and rc != 0: + if ignore_error: + return rc, out, err + if rc != 0: module.fail_json(msg="Failed %s: rc=%d, out=%s, err=%s" % (msg, rc, out, err)) return out @@ -68,10 +70,14 @@ def install_new_sonic_image(module, new_image_url): # There is enough space to install directly save_as = "/host/downloaded-sonic-image" download_new_sonic_image(module, new_image_url, save_as) - exec_command(module, + rc, out, err = exec_command(module, cmd="sonic_installer install {} -y".format(save_as), - msg="installing new image") + msg="installing new image", ignore_error=True) + # Always remove the downloaded temp image inside /host/ before proceeding exec_command(module, cmd="rm -f {}".format(save_as)) + if rc != 0: + module.fail_json(msg="Image installation failed: rc=%d, out=%s, err=%s" % + (rc, out, err)) else: # Create a tmpfs partition to download image to install exec_command(module, cmd="mkdir -p /tmp/tmpfs", ignore_error=True) @@ -82,13 +88,16 @@ def install_new_sonic_image(module, new_image_url): msg="mounting tmpfs") save_as = "/tmp/tmpfs/downloaded-sonic-image" download_new_sonic_image(module, new_image_url, save_as) - exec_command(module, + rc, out, err = exec_command(module, cmd="sonic_installer install {} -y".format(save_as), - msg="installing new image") + msg="installing new image", ignore_error=True) exec_command(module, cmd="sync", ignore_error=True) exec_command(module, cmd="umount /tmp/tmpfs", ignore_error=True) exec_command(module, cmd="rm -rf /tmp/tmpfs", ignore_error=True) + if rc != 0: + module.fail_json(msg="Image installation failed: rc=%d, out=%s, err=%s" % + (rc, out, err)) # If sonic device is configured with minigraph, remove config_db.json # to force next image to load minigraph. diff --git a/ansible/roles/test/files/ptftests/advanced-reboot.py b/ansible/roles/test/files/ptftests/advanced-reboot.py index 74fc58690a..89c3f4a1f6 100644 --- a/ansible/roles/test/files/ptftests/advanced-reboot.py +++ b/ansible/roles/test/files/ptftests/advanced-reboot.py @@ -489,7 +489,7 @@ def setUp(self): self.limit = datetime.timedelta(seconds=self.test_params['reboot_limit_in_seconds']) self.reboot_type = self.test_params['reboot_type'] - if self.reboot_type not in ['fast-reboot', 'warm-reboot']: + if self.reboot_type not in ['fast-reboot', 'warm-reboot', 'warm-reboot -f']: raise ValueError('Not supported reboot_type %s' % self.reboot_type) self.dut_mac = self.test_params['dut_mac'] @@ -537,7 +537,7 @@ def setUp(self): self.generate_ping_dut_lo() self.generate_arp_ping_packet() - if self.reboot_type == 'warm-reboot': + if 'warm-reboot' in self.reboot_type: self.log(self.get_sad_info()) # Pre-generate list of packets to be sent in send_in_background method. @@ -876,7 +876,7 @@ def wait_for_ssh_threads(): if self.reboot_type == 'fast-reboot' and self.no_cp_replies < 0.95 * self.nr_vl_pkts: self.fails['dut'].add("Dataplane didn't route to all servers, when control-plane was down: %d vs %d" % (self.no_cp_replies, self.nr_vl_pkts)) - if self.reboot_type == 'warm-reboot': + if 'warm-reboot' in self.reboot_type: if self.total_disrupt_time > self.limit.total_seconds(): self.fails['dut'].add("Total downtime period must be less then %s seconds. It was %s" \ % (str(self.limit), str(self.total_disrupt_time))) @@ -985,7 +985,7 @@ def runTest(self): self.wait_until_reboot() if self.reboot_type == 'fast-reboot': self.handle_fast_reboot_health_check() - if self.reboot_type == 'warm-reboot': + if 'warm-reboot' in self.reboot_type: self.handle_warm_reboot_health_check() self.handle_post_reboot_health_check() diff --git a/tests/common/fixtures/advanced_reboot.py b/tests/common/fixtures/advanced_reboot.py index f4a14232fb..62d342cb82 100644 --- a/tests/common/fixtures/advanced_reboot.py +++ b/tests/common/fixtures/advanced_reboot.py @@ -282,6 +282,8 @@ def __handleRebootImage(self): logger.info('Remove config_db.json so the new image will reload minigraph') self.duthost.shell('rm -f /host/old_config/config_db.json') + logger.info('Remove downloaded tempfile') + self.duthost.shell('rm -f {}'.format(tempfile)) def __setupTestbed(self): ''' diff --git a/tests/upgrade_path/test_upgrade_path.py b/tests/upgrade_path/test_upgrade_path.py index 33051673c4..e5d69ea5d6 100644 --- a/tests/upgrade_path/test_upgrade_path.py +++ b/tests/upgrade_path/test_upgrade_path.py @@ -118,6 +118,16 @@ def ptf_params(duthost, nbrhosts, creds): } return ptf_params +def get_reboot_type(duthost): + next_os_version = duthost.shell('sonic_installer list | grep Next | cut -f2 -d " "')['stdout'] + current_os_version = duthost.shell('sonic_installer list | grep Current | cut -f2 -d " "')['stdout'] + + # warm-reboot has to be forced for an upgrade from 201811 to 201911 to bypass ASIC config changed error + if 'SONiC-OS-201811' in current_os_version and 'SONiC-OS-201911' in next_os_version: + reboot_type = "warm-reboot -f" + else: + reboot_type = "warm-reboot" + return reboot_type def check_sonic_version(duthost, target_version): current_version = duthost.image_facts()['ansible_facts']['ansible_image_facts']['current'] @@ -134,7 +144,6 @@ def test_upgrade_path(localhost, duthost, ptfhost, upgrade_path_lists, ptf_param from_list = from_list_images.split(',') to_list = to_list_images.split(',') assert (from_list and to_list) - test_params = ptf_params for from_image in from_list: for to_image in to_list: logger.info("Test upgrade path from {} to {}".format(from_image, to_image)) @@ -148,7 +157,9 @@ def test_upgrade_path(localhost, duthost, ptfhost, upgrade_path_lists, ptf_param # Install target image logger.info("Upgrading to {}".format(to_image)) target_version = install_sonic(duthost, to_image) + test_params = ptf_params test_params['target_version'] = target_version + test_params['reboot_type'] = get_reboot_type(duthost) prepare_testbed_ssh_keys(duthost, ptfhost, test_params['dut_username']) log_file = "/tmp/advanced-reboot.ReloadTest.{}.log".format(datetime.now().strftime('%Y-%m-%d-%H:%M:%S')) From 8ec4d498d4a20b75f44f130e99e85dffb84cc3b1 Mon Sep 17 00:00:00 2001 From: Vaibhav Hemant Dixit Date: Mon, 2 Nov 2020 09:38:31 -0800 Subject: [PATCH 2/2] Review comments addressed - updated the return values for exec_command --- ansible/library/reduce_and_add_sonic_images.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/ansible/library/reduce_and_add_sonic_images.py b/ansible/library/reduce_and_add_sonic_images.py index fcdd29b809..7d0f0e1c96 100644 --- a/ansible/library/reduce_and_add_sonic_images.py +++ b/ansible/library/reduce_and_add_sonic_images.py @@ -30,16 +30,14 @@ def exec_command(module, cmd, ignore_error=False, msg="executing command"): rc, out, err = module.run_command(cmd) - if ignore_error: - return rc, out, err - if rc != 0: + if not ignore_error and rc != 0: module.fail_json(msg="Failed %s: rc=%d, out=%s, err=%s" % (msg, rc, out, err)) - return out + return rc, out, err def get_disk_free_size(module, partition): - out = exec_command(module, cmd="df -BM --output=avail %s" % partition, + _, out, _ = exec_command(module, cmd="df -BM --output=avail %s" % partition, msg="checking disk available size") avail = int(out.split('\n')[1][:-1]) @@ -57,9 +55,10 @@ def download_new_sonic_image(module, new_image_url, save_as): cmd="curl -o {} {}".format(save_as, new_image_url), msg="downloading new image") if path.exists(save_as): - results['downloaded_image_version'] = exec_command(module, + _, out, _ = exec_command(module, cmd="sonic_installer binary_version %s" % save_as - ).rstrip('\n') + ) + results['downloaded_image_version'] = out.rstrip('\n') def install_new_sonic_image(module, new_image_url): if not new_image_url: