${htmlEscape(env.RESOLVED_SOURCE_SHA)}")
- rows += presubmitInfoRow('Started', htmlEscape(env.PRESUBMIT_STARTED_AT))
+ rows += presubmitInfoRow(
+ 'Started', htmlEscape(env.PRESUBMIT_STARTED_AT_IST ?: env.PRESUBMIT_STARTED_AT))
def consoleUrl = "${env.BUILD_URL}console"
return """The API error above is often only a symptom. The cause is " + f"usually in the CloudStack logs: {links}
") + for number, failure in enumerate(failures, 1): + names = failure["names"] or (f"unmapped-{number}",) + anchors = "".join( + f'' for name in names) + title = ", ".join(failure["names"]) or failure["description"] + sections.append( + f'{anchors}'
+ f'{html.escape(chr(10).join(failure["lines"]))}')
+ return "'
+ "\n".join(rendered)
+ ""
@@ -175,6 +296,12 @@ def generate_log_artifacts(results, debian_log):
results / "phase2" / "configure-cloudstack.log",
results / "phase2" / "health-check.log",
]
+ runtime = results / "phase2" / "runtime-logs"
+ related_logs = (
+ ("Management server log",
+ runtime / "var-log-cloudstack-management" / "management-server.log"),
+ ("Agent log", runtime / "var-log-cloudstack-agent" / "agent.log"),
+ )
marvin = results / "phase2" / "marvin"
candidates.extend(marvin.glob("*.log"))
candidates.extend(
@@ -183,7 +310,15 @@ def generate_log_artifacts(results, debian_log):
for source in sorted(set(candidates)):
if not source.is_file():
continue
- log_to_html(source, Path(f"{source}.html"))
+ failures = (
+ parse_suite_failures(source.with_name("results.txt"))
+ if source.name == "stdout.log" else ())
+ destination = Path(f"{source}.html")
+ related = tuple(
+ (label, os.path.relpath(path, destination.parent))
+ for label, path in related_logs if path.is_file()
+ ) if failures else ()
+ log_to_html(source, destination, failures, related)
def parse_stage_events(stage_events, results):
@@ -263,8 +398,7 @@ def stage_rows(events, results, build_url):
log_link = (
f''
"Console")
- started_text = (
- started.strftime("%Y-%m-%d %H:%M:%S UTC") if started else "—")
+ started_text = format_ist(started)
rows.append(
"{html.escape(str(test.get('name', '')))}Failures are listed first. Select a log link to jump to the " "test.
" - + table(["Protocol", "Suite", "Test", "Status", "Details"], tests_html) + + table(["Protocol", "Suite", "Test", "Status", "Error", "Details"], + tests_html) if tests else "No Marvin test summary was available.
") body = ( diff --git a/private-cicd/scripts/mvn-full.sh b/private-cicd/scripts/mvn-full.sh index 7aa9821a1d24..605a71ee1ec6 100755 --- a/private-cicd/scripts/mvn-full.sh +++ b/private-cicd/scripts/mvn-full.sh @@ -25,7 +25,9 @@ # RESULT_DIR — if set, SOURCE_ID and SOURCE_SHA are required (Jenkins handoff); # PR_ID and PR_HEAD_SHA are accepted as fallbacks for pull-request builds. # Optional: SKIP_TESTS, ENABLE_NOREDIST, ENABLE_SIMULATOR (default true), -# MAVEN_THREADS (Jenkins uses 1C; empty disables -T; unset uses nproc). +# MAVEN_THREADS (Jenkins uses 1C; empty disables -T; unset uses nproc), +# RERUN_FAILING_TESTS (default 0; Jenkins uses 2 so a timing-flaky upstream +# test is retried before it fails the run). # Commands: mvn, java. Does not call other private-cicd scripts. # When RESULT_DIR is set, apply debian/rules packaging flags so build-debs.sh # can reuse target/ without a second mvn clean. @@ -43,6 +45,7 @@ SKIP_TESTS="${SKIP_TESTS:-false}" ENABLE_NOREDIST="${ENABLE_NOREDIST:-false}" ENABLE_SIMULATOR="${ENABLE_SIMULATOR:-true}" THREADS="${MAVEN_THREADS-$(nproc)}" +RERUN_FAILING_TESTS="${RERUN_FAILING_TESTS:-0}" RESULT_DIR="${RESULT_DIR:-}" mvn_args=(-B -P developer,systemvm) @@ -59,6 +62,9 @@ fi if [[ -n "$THREADS" ]]; then mvn_args+=(-T"$THREADS") fi +if [[ "$RERUN_FAILING_TESTS" != "0" ]]; then + mvn_args+=("-Dsurefire.rerunFailingTestsCount=$RERUN_FAILING_TESTS") +fi echo "==> Full build in $SOURCE_DIR" diff --git a/private-cicd/scripts/resolve-pull-request.py b/private-cicd/scripts/resolve-pull-request.py new file mode 100644 index 000000000000..01f90de0a50a --- /dev/null +++ b/private-cicd/scripts/resolve-pull-request.py @@ -0,0 +1,135 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Decide whether a multibranch build may test one origin pull request.""" + +import argparse +import json +import os +import sys +import urllib.error +import urllib.request + + +API_ROOT = "https://api.github.com" + + +def parse_args(): + parser = argparse.ArgumentParser() + parser.add_argument("--repository", required=True) + parser.add_argument("--pr-id", required=True) + parser.add_argument("--indexed-sha", required=True) + parser.add_argument("--change-target", required=True) + parser.add_argument("--output", required=True) + return parser.parse_args() + + +def github_get(url, token): + request = urllib.request.Request( + url, + headers={ + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {token}", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "cloudstack-ontap-presubmit", + }, + ) + try: + with urllib.request.urlopen(request, timeout=60) as response: + return json.loads(response.read().decode("utf-8")) + except urllib.error.HTTPError as error: + detail = error.read().decode("utf-8", errors="replace") + raise SystemExit(f"GitHub API {error.code} for {url}: {detail}") from error + + +def is_main(ref): + return ref in ("main", "refs/heads/main") + + +def assess(pull, pr_id, indexed_sha, change_target, expected_repository): + head = (pull or {}).get("head") or {} + base = (pull or {}).get("base") or {} + user = (pull or {}).get("user") or {} + repository = head.get("repo") or {} + decision = { + "eligible": False, + "reason": "", + "pr_id": str((pull or {}).get("number") or ""), + "pr_head_sha": head.get("sha") or "", + "pr_head_branch": head.get("ref") or "", + "pr_base_branch": base.get("ref") or "", + "pr_draft": bool((pull or {}).get("draft")), + "pr_title": (pull or {}).get("title") or "", + "pr_url": (pull or {}).get("html_url") or "", + "pr_author_login": user.get("login") or "", + "pr_author_email": user.get("email") or "", + "head_repository": repository.get("full_name") or "", + } + if not isinstance(pull, dict): + decision["reason"] = "GitHub pull request payload is missing." + elif decision["pr_id"] != str(pr_id): + decision["reason"] = "GitHub pull request number does not match the build." + elif decision["head_repository"] != expected_repository: + decision["reason"] = ( + f"head repository '{decision['head_repository']}' is not " + f"'{expected_repository}'." + ) + elif not is_main(decision["pr_base_branch"]) or not is_main(change_target): + decision["reason"] = ( + f"base branch is '{decision['pr_base_branch']}' and change target " + f"is '{change_target}'." + ) + elif decision["pr_draft"]: + decision["reason"] = "pull request is a draft." + elif not decision["pr_head_sha"] or len(decision["pr_head_sha"]) != 40: + decision["reason"] = "GitHub pull request head SHA is missing." + elif (indexed_sha or "").strip() and decision["pr_head_sha"].lower() != indexed_sha.strip().lower(): + decision["reason"] = "indexed revision does not match the pull request head SHA." + else: + decision["eligible"] = True + return decision + + +def main(): + args = parse_args() + token = os.environ.get("GITHUB_ACCESS_TOKEN", "").strip() + if not token: + raise SystemExit("GITHUB_ACCESS_TOKEN is empty.") + if not args.pr_id.isdigit(): + raise SystemExit("PR id must be numeric.") + pull = github_get( + f"{API_ROOT}/repos/{args.repository}/pulls/{args.pr_id}", + token, + ) + decision = assess( + pull, + args.pr_id, + args.indexed_sha, + args.change_target, + args.repository, + ) + with open(args.output, "w", encoding="utf-8") as handle: + json.dump(decision, handle) + handle.write("\n") + if not decision["eligible"]: + print(decision["reason"], file=sys.stderr) + raise SystemExit(2) + + +if __name__ == "__main__": + main() diff --git a/private-cicd/scripts/test_generate_presubmit_report.py b/private-cicd/scripts/test_generate_presubmit_report.py new file mode 100644 index 000000000000..f9cc443ff827 --- /dev/null +++ b/private-cicd/scripts/test_generate_presubmit_report.py @@ -0,0 +1,180 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import importlib.util +import tempfile +import unittest +from pathlib import Path + + +SPEC = importlib.util.spec_from_file_location( + "generate_presubmit_report", + Path(__file__).with_name("generate-presubmit-report.py")) +report = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(report) + +SEP = "=" * 70 +DASH = "-" * 70 +RESULTS = f"""Start the VM. ... === TestName: test_06_start | Status : EXCEPTION === +ERROR +Check the pool. ... === TestName: test_07_pool | Status : FAIL === +FAIL +Detach it. ... === TestName: test_08_detach | Status : SUCCESS === +ok + +{SEP} +ERROR: Start the VM. +{DASH} +Traceback (most recent call last): + File "test.py", line 750, in test_06_start + self.apiClient.startVirtualMachine(cmd) +Exception: Job failed: {{errortext : 'Unable to orchestrate the start of VM'}} +-------------------- >> begin captured logging << -------------------- +captured line that must not be shown +--------------------- >> end captured logging << --------------------- + +{SEP} +FAIL: Check the pool. +{DASH} +Traceback (most recent call last): + File "test.py", line 90, in test_07_pool + self.assertEqual(a, b) +AssertionError: 1 != 2 +{DASH} +Ran 3 tests in 1.0s + +FAILED (errors=1, failures=1) +""" +SETUP_ERROR = f"""{SEP} +ERROR: setUpClass (suite.Class) +{DASH} +Traceback (most recent call last): +RuntimeError: pool create failed +{DASH} +Ran 0 tests in 0.1s +""" + + +class ParseSuiteFailuresTest(unittest.TestCase): + def parse(self, text): + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "results.txt" + path.write_text(text, encoding="utf-8") + return report.parse_suite_failures(path) + + def test_error_and_fail_blocks_are_mapped_to_test_names(self): + failures = self.parse(RESULTS) + self.assertEqual([f["kind"] for f in failures], ["ERROR", "FAIL"]) + self.assertEqual(failures[0]["names"], ("test_06_start",)) + self.assertEqual(failures[1]["names"], ("test_07_pool",)) + + def test_error_is_the_exception_line_without_captured_logging(self): + failures = self.parse(RESULTS) + self.assertTrue(failures[0]["error"].startswith("Exception: Job failed")) + self.assertIn("Unable to orchestrate", failures[0]["error"]) + self.assertEqual(failures[1]["error"], "AssertionError: 1 != 2") + self.assertNotIn( + "must not be shown", "\n".join(failures[0]["lines"])) + + def test_cloudstack_job_failure_leads_with_its_message(self): + line = ("Exception: Job failed: {accountid : 'a', account : 'admin', " + "jobresult : {errorcode : 530, errortext : 'Unable to start " + "the VM'}}") + text = f"{SEP}\nERROR: d\n{DASH}\n{line}\n" + self.assertEqual( + self.parse(text)[0]["error"], + "Job failed (code 530): Unable to start the VM") + + def test_unmapped_block_is_kept_without_names(self): + failures = self.parse(SETUP_ERROR) + self.assertEqual(len(failures), 1) + self.assertEqual(failures[0]["names"], ()) + self.assertEqual(failures[0]["error"], "RuntimeError: pool create failed") + + def test_missing_file_or_clean_run_gives_no_failures(self): + self.assertEqual( + report.parse_suite_failures(Path("/nonexistent/results.txt")), ()) + self.assertEqual(self.parse("Detach. ... === TestName: t | Status : SUCCESS ===\nok\n"), ()) + + def test_long_error_is_truncated(self): + long_line = "Exception: " + "x" * 2000 + text = f"{SEP}\nERROR: d\n{DASH}\n{long_line}\n" + error = self.parse(text)[0]["error"] + self.assertEqual(len(error), report.MAX_ERROR_CHARS + 1) + + +class IstTimeTest(unittest.TestCase): + def test_utc_time_is_shown_in_ist(self): + value = report.parse_time("2026-11-05T01:54:07Z") + self.assertEqual(report.format_ist(value), "2026-11-05 07:24:07 IST") + + def test_offset_time_and_date_rollover(self): + value = report.parse_time("2026-11-05T22:00:00+00:00") + self.assertEqual(report.format_ist(value), "2026-11-06 03:30:00 IST") + + def test_missing_time_is_a_dash(self): + self.assertEqual(report.format_ist(None), "—") + + def test_durations_are_unaffected(self): + start = report.parse_time("2026-11-05T01:00:00Z") + finish = report.parse_time("2026-11-05T02:30:00Z") + self.assertEqual(finish - start, report.timedelta(hours=1, minutes=30)) + + +class FailurePageTest(unittest.TestCase): + def test_suite_page_shows_failure_before_the_log(self): + with tempfile.TemporaryDirectory() as directory: + suite = (Path(directory) / "phase2" / "marvin" / "ontap-results" + / "run-iscsi" / "suites" / "vm") + suite.mkdir(parents=True) + (suite / "results.txt").write_text(RESULTS, encoding="utf-8") + log = suite / "stdout.log" + log.write_text( + "=== TestName: test_06_start | Status : EXCEPTION ===\n" + "=== TestName: test_08_detach | Status : SUCCESS ===\n", + encoding="utf-8") + management = (Path(directory) / "phase2" / "runtime-logs" + / "var-log-cloudstack-management") + management.mkdir(parents=True) + (management / "management-server.log").write_text("log\n") + report.generate_log_artifacts(Path(directory), None) + page = (suite / "stdout.log.html").read_text(encoding="utf-8") + self.assertIn('id="fail-test_06_start"', page) + self.assertIn("Unable to orchestrate", page) + self.assertIn('id="test-test_08_detach"', page) + self.assertIn("Management server log", page) + self.assertIn( + "../../../../../runtime-logs/var-log-cloudstack-management/" + "management-server.log", page) + self.assertNotIn("Agent log", page) + self.assertLess(page.index("Failure details"), page.index("Suite log")) + self.assertLess(page.index("Suite log"), page.index("TestName")) + + def test_clean_suite_page_has_no_failure_section(self): + with tempfile.TemporaryDirectory() as directory: + log = Path(directory) / "stdout.log" + log.write_text("=== TestName: t | Status : SUCCESS ===\n", + encoding="utf-8") + destination = Path(directory) / "stdout.log.html" + report.log_to_html(log, destination) + self.assertNotIn( + "Failure details", destination.read_text(encoding="utf-8")) + + +if __name__ == "__main__": + unittest.main() diff --git a/private-cicd/scripts/test_resolve_pull_request.py b/private-cicd/scripts/test_resolve_pull_request.py new file mode 100644 index 000000000000..739fe10a89e0 --- /dev/null +++ b/private-cicd/scripts/test_resolve_pull_request.py @@ -0,0 +1,88 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Unit tests for resolve-pull-request.py.""" + +import importlib.util +import unittest +from pathlib import Path + + +SCRIPT = Path(__file__).with_name("resolve-pull-request.py") +SPEC = importlib.util.spec_from_file_location("resolve_pull_request", SCRIPT) +MODULE = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(MODULE) + +SHA = "a" * 40 + + +def pull(number="98", draft=False, base="main", repo="NetApp/cloudstack", sha=SHA): + return { + "number": int(number), + "title": "Test", + "html_url": f"https://github.com/NetApp/cloudstack/pull/{number}", + "draft": draft, + "base": {"ref": base}, + "head": { + "ref": "feature/test", + "sha": sha, + "repo": {"full_name": repo} if repo is not None else None, + }, + "user": {"login": "tester", "email": None}, + } + + +class AssessPullRequestTest(unittest.TestCase): + def test_accepts_matching_origin_pr(self): + decision = MODULE.assess(pull(), "98", SHA, "main", "NetApp/cloudstack") + + self.assertTrue(decision["eligible"]) + self.assertEqual("", decision["reason"]) + self.assertEqual("tester", decision["pr_author_login"]) + self.assertEqual("", decision["pr_author_email"]) + + def test_rejects_draft_fork_base_and_sha_mismatch(self): + cases = [ + pull(draft=True), + pull(repo="other/cloudstack"), + pull(repo=None), + pull(base="release"), + pull(sha="b" * 40), + ] + + for candidate in cases: + decision = MODULE.assess( + candidate, "98", SHA, "main", "NetApp/cloudstack") + self.assertFalse(decision["eligible"], candidate) + + def test_accepts_github_head_when_jenkins_sha_is_missing(self): + decision = MODULE.assess(pull(), "98", "", "main", "NetApp/cloudstack") + + self.assertTrue(decision["eligible"]) + self.assertEqual(SHA, decision["pr_head_sha"]) + + def test_rejects_change_target_other_than_main(self): + decision = MODULE.assess( + pull(), "98", SHA, "feature/other", "NetApp/cloudstack") + + self.assertFalse(decision["eligible"]) + self.assertIn("change target", decision["reason"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/integration/plugins/ontap/iscsi/instance/test_vm_volume_attach.py b/test/integration/plugins/ontap/iscsi/instance/test_vm_volume_attach.py index ab46b96e6a3b..706c272b71d7 100644 --- a/test/integration/plugins/ontap/iscsi/instance/test_vm_volume_attach.py +++ b/test/integration/plugins/ontap/iscsi/instance/test_vm_volume_attach.py @@ -125,7 +125,6 @@ listNetworks as listNetworksAPI, listVirtualMachines as listVirtualMachinesAPI, listVolumes as listVolumesAPI, - startVirtualMachine as startVirtualMachineAPI, stopVirtualMachine as stopVirtualMachineAPI, ) from marvin.lib.base import ServiceOffering, Snapshot, StoragePool, VmSnapshot, Volume @@ -618,6 +617,7 @@ def tearDown(self): VmSnapshot.deleteVMSnapshot(self.apiClient, snap_id) except Exception as exc: logger.warning("tearDown: VM snapshot %s: %s" % (snap_id, exc)) + self._delete_failed_vm_snapshots() for snap_id in self._volume_snapshot_ids: try: Snapshot({"id": snap_id}).delete(self.apiClient) @@ -798,14 +798,6 @@ def _stop_vm(self, vm_id): self.apiClient.stopVirtualMachine(cmd) self._poll_vm_state(vm_id, "Stopped", timeout=600) - def _start_vm(self, vm_id): - if (self._vm_state(vm_id) or "").lower() == "running": - return - cmd = startVirtualMachineAPI.startVirtualMachineCmd() - cmd.id = vm_id - self.apiClient.startVirtualMachine(cmd) - self._poll_vm_state(vm_id, "Running", timeout=900) - def _root_volume(self, vm_id): cmd = listVolumesAPI.listVolumesCmd() cmd.virtualmachineid = vm_id @@ -902,6 +894,30 @@ def _take_vm_snapshot(self, vm_id): def _vm_snapshots_of(self, vm_id): return VmSnapshot.list(self.apiClient, virtualmachineid=vm_id, listall=True) or [] + def _delete_failed_vm_snapshots(self): + """Delete VM snapshots left in Error by a rejected create. + + A rejected createVMSnapshot (ONTAP refusing while a revert's clones + exist) leaves its record behind with no id the test ever saw. Any + Instance Snapshot, even in Error, blocks detach, resize and destroy. + """ + for vm in (self.__class__.vm, self.__class__.vm2): + if vm is None: + continue + try: + stale = [s for s in self._vm_snapshots_of(vm.id) + if (getattr(s, "state", "") or "").lower() == "error"] + except Exception as exc: + logger.warning("tearDown: could not list VM snapshots of %s: %s" + % (vm.id, exc)) + continue + for snap in stale: + try: + VmSnapshot.deleteVMSnapshot(self.apiClient, snap.id) + except Exception as exc: + logger.warning("tearDown: failed VM snapshot %s: %s" + % (snap.id, exc)) + def _volume_snapshot_listed(self, snap_id): listed = Snapshot.list(self.apiClient, id=snap_id, listall=True) or [] return any(getattr(item, "id", None) == snap_id for item in listed) @@ -1043,7 +1059,7 @@ def test_03_deploy_vm(self): if self.__class__.network_id: cmd.networkids = self.__class__.network_id - vm = self.apiClient.deployVirtualMachine(cmd) + vm = self._deploy_vm(cmd) self.__class__.vm = vm result = self._poll_vm_state(vm.id, "Running") @@ -1094,7 +1110,7 @@ def test_03a_deploy_second_vm_reuses_template_cache(self): if self.__class__.network_id: cmd.networkids = self.__class__.network_id - vm2 = self.apiClient.deployVirtualMachine(cmd) + vm2 = self._deploy_vm(cmd) self.assertIsNotNone(vm2, "deployVirtualMachine returned None") self.__class__.vm2 = vm2 self._poll_vm_state(vm2.id, "Running") @@ -1336,9 +1352,7 @@ def test_08_start_vm_lun_remapped(self): if self.__class__.vm is None: self.skipTest("VM not deployed — test_03 was skipped") - cmd = startVirtualMachineAPI.startVirtualMachineCmd() - cmd.id = self.__class__.vm.id - self.apiClient.startVirtualMachine(cmd) + self._start_vm(self.__class__.vm.id) result = self._poll_vm_state(self.__class__.vm.id, "Running", timeout=300) diff --git a/test/integration/plugins/ontap/nfs3/instance/test_vm_volume_attach.py b/test/integration/plugins/ontap/nfs3/instance/test_vm_volume_attach.py index d3450949baf2..c3b7e36a7316 100644 --- a/test/integration/plugins/ontap/nfs3/instance/test_vm_volume_attach.py +++ b/test/integration/plugins/ontap/nfs3/instance/test_vm_volume_attach.py @@ -110,7 +110,6 @@ listNetworks as listNetworksAPI, listVirtualMachines as listVirtualMachinesAPI, listVolumes as listVolumesAPI, - startVirtualMachine as startVirtualMachineAPI, stopVirtualMachine as stopVirtualMachineAPI, updateStoragePool as updateStoragePoolAPI, ) @@ -450,6 +449,7 @@ def tearDown(self): VmSnapshot.deleteVMSnapshot(self.apiClient, snap_id) except Exception as exc: logger.warning("tearDown: VM snapshot %s: %s" % (snap_id, exc)) + self._delete_failed_vm_snapshots() for snap_id in self._volume_snapshot_ids: try: Snapshot({"id": snap_id}).delete(self.apiClient) @@ -630,14 +630,6 @@ def _stop_vm(self, vm_id): self.apiClient.stopVirtualMachine(cmd) self._poll_vm_state(vm_id, "Stopped", timeout=600) - def _start_vm(self, vm_id): - if (self._vm_state(vm_id) or "").lower() == "running": - return - cmd = startVirtualMachineAPI.startVirtualMachineCmd() - cmd.id = vm_id - self.apiClient.startVirtualMachine(cmd) - self._poll_vm_state(vm_id, "Running", timeout=900) - def _root_volume(self, vm_id): cmd = listVolumesAPI.listVolumesCmd() cmd.virtualmachineid = vm_id @@ -734,6 +726,30 @@ def _take_vm_snapshot(self, vm_id): def _vm_snapshots_of(self, vm_id): return VmSnapshot.list(self.apiClient, virtualmachineid=vm_id, listall=True) or [] + def _delete_failed_vm_snapshots(self): + """Delete VM snapshots left in Error by a rejected create. + + A rejected createVMSnapshot (ONTAP refusing while a revert's clones + exist) leaves its record behind with no id the test ever saw. Any + Instance Snapshot, even in Error, blocks detach, resize and destroy. + """ + for vm in (self.__class__.vm, self.__class__.vm2): + if vm is None: + continue + try: + stale = [s for s in self._vm_snapshots_of(vm.id) + if (getattr(s, "state", "") or "").lower() == "error"] + except Exception as exc: + logger.warning("tearDown: could not list VM snapshots of %s: %s" + % (vm.id, exc)) + continue + for snap in stale: + try: + VmSnapshot.deleteVMSnapshot(self.apiClient, snap.id) + except Exception as exc: + logger.warning("tearDown: failed VM snapshot %s: %s" + % (snap.id, exc)) + def _volume_snapshot_listed(self, snap_id): listed = Snapshot.list(self.apiClient, id=snap_id, listall=True) or [] return any(getattr(item, "id", None) == snap_id for item in listed) @@ -872,7 +888,7 @@ def test_03_deploy_vm(self): if self.__class__.network_id: cmd.networkids = self.__class__.network_id - vm = self.apiClient.deployVirtualMachine(cmd) + vm = self._deploy_vm(cmd) self.assertIsNotNone(vm, "deployVirtualMachine returned None") self.__class__.vm = vm @@ -912,7 +928,7 @@ def test_03a_deploy_second_vm_reuses_template_cache(self): if self.__class__.network_id: cmd.networkids = self.__class__.network_id - vm2 = self.apiClient.deployVirtualMachine(cmd) + vm2 = self._deploy_vm(cmd) self.assertIsNotNone(vm2, "deployVirtualMachine returned None") self.__class__.vm2 = vm2 self._poll_vm_state(vm2.id, "Running") @@ -1213,9 +1229,7 @@ def test_08_start_vm_volume_accessible(self): vm = self.__class__.vm vol = self.__class__.volume - cmd = startVirtualMachineAPI.startVirtualMachineCmd() - cmd.id = vm.id - self.apiClient.startVirtualMachine(cmd) + self._start_vm(vm.id) result = self._poll_vm_state(vm.id, "Running", timeout=300) self.assertEqual( diff --git a/test/integration/plugins/ontap/ontap_test_base.py b/test/integration/plugins/ontap/ontap_test_base.py index e528aba2b25f..eb36929651a8 100644 --- a/test/integration/plugins/ontap/ontap_test_base.py +++ b/test/integration/plugins/ontap/ontap_test_base.py @@ -46,6 +46,7 @@ deleteNetwork as deleteNetworkAPI, deleteStoragePool as deleteStoragePoolAPI, deleteVolume as deleteVolumeAPI, + deployVirtualMachine as deployVirtualMachineAPI, destroyVirtualMachine as destroyVirtualMachineAPI, destroyVolume as destroyVolumeAPI, detachVolume as detachVolumeAPI, @@ -53,10 +54,12 @@ listDiskOfferings as listDiskOfferingsAPI, listNetworkOfferings as listNetworkOfferingsAPI, listNetworks as listNetworksAPI, + listRouters as listRoutersAPI, listServiceOfferings as listServiceOfferingsAPI, listTemplates as listTemplatesAPI, listVirtualMachines as listVirtualMachinesAPI, listVolumes as listVolumesAPI, + startVirtualMachine as startVirtualMachineAPI, stopVirtualMachine as stopVirtualMachineAPI, resizeVolume as resizeVolumeAPI, updateStoragePool as updateStoragePoolAPI, @@ -897,6 +900,17 @@ class OntapTestBase(cloudstackTestCase): # VM states from which the VM will never reach any other target state. _VM_TERMINAL_STATES = ("error", "destroyed", "expunging") + # The guest network's virtual router is created by the first VM deploy + # and can be briefly unreachable afterwards, which makes a VM start fail + # with "Unable to apply dhcp entry on router". Every wait and retry below + # is bounded so a genuine failure is reported, never waited out forever. + ROUTER_HEALTHY_TIMEOUT = 180 # seconds, per wait + ROUTER_POLL_INTERVAL = 10 # seconds + VM_START_ATTEMPTS = 3 # total tries, including the first + VM_START_RETRY_DELAY = 15 # seconds between tries + # Set once a router wait times out; see _wait_for_routers_healthy(). + _router_wait_exhausted = False + # ---- zone guard ---------------------------------------------------- @classmethod @@ -1618,6 +1632,118 @@ def _other_ontap_pools_on_svm(self, current_pool_id): others.append(pool) return others + # ---- virtual router readiness and VM operations -------------------- + + def _unhealthy_routers(self): + """Describe the test account's routers that are not ready. + + A router is not ready when it is not Running or reports failed health + checks. Returns [] when every router is ready, or when none exists + yet (the first VM deploy creates it). + """ + cmd = listRoutersAPI.listRoutersCmd() + cmd.listall = True + if self.zone is not None: + cmd.zoneid = self.zone.id + if self.account is not None: + cmd.account = self.account.name + cmd.domainid = self.account.domainid + unready = [] + for router in self.apiClient.listRouters(cmd) or []: + state = getattr(router, "state", "") or "" + failed = bool(getattr(router, "healthchecksfailed", False)) + if state.lower() != "running" or failed: + unready.append( + "%s (state=%s, healthchecksfailed=%s)" + % (getattr(router, "name", "router"), state, failed)) + return unready + + def _wait_for_routers_healthy(self, timeout=None): + """Wait, for a bounded time, until the guest router is ready. + + Returns True when ready (or when readiness cannot be determined) and + False when it is still not ready after the timeout. It never raises: + a router that stays flagged is reported in the log, and the VM + operation that follows then decides the test. After one timeout the + class stops waiting, so a router with a permanent health warning + costs one wait, not one per VM operation. + """ + timeout = self.ROUTER_HEALTHY_TIMEOUT if timeout is None else timeout + if self.__class__._router_wait_exhausted: + timeout = 0 + deadline = time.time() + timeout + while True: + try: + unready = self._unhealthy_routers() + except Exception as error: + log_progress( + logger, "warning", + "Cannot read router health, continuing: %s", error) + return True + if not unready: + return True + if time.time() >= deadline: + break + log_progress( + logger, "info", "Waiting for router to be ready: %s", + "; ".join(unready)) + time.sleep(self.ROUTER_POLL_INTERVAL) + if timeout: + self.__class__._router_wait_exhausted = True + log_progress( + logger, "warning", + "Router still not ready after %ss: %s", timeout, + "; ".join(unready)) + return False + + def _deploy_vm(self, cmd): + """Deploy a VM once the router is ready, and let a new one settle.""" + self._wait_for_routers_healthy() + vm = self.apiClient.deployVirtualMachine(cmd) + # The first deploy creates the guest network's router. + self._wait_for_routers_healthy() + return vm + + def _start_vm(self, vm_id): + """Start a stopped VM, retrying a bounded number of times. + + A start can fail only because the router was briefly unreachable + ("Unable to apply dhcp entry on router"); CloudStack then rolls the + VM back to Stopped. That, and only that, is retried. A VM in any + other state, or the last attempt failing, raises with every attempt's + error so the cause is not hidden. Returns once the VM is Running; a + VM that is already Running is left alone. + """ + vms = self.apiClient.listVirtualMachines(_list_vms_cmd(vm_id)) or [] + if vms and (vms[0].state or "").lower() == "running": + return + cmd = startVirtualMachineAPI.startVirtualMachineCmd() + cmd.id = vm_id + attempts = self.VM_START_ATTEMPTS + errors = [] + for attempt in range(1, attempts + 1): + self._wait_for_routers_healthy() + try: + self.apiClient.startVirtualMachine(cmd) + self._poll_vm_state(vm_id, "Running", timeout=900) + return + except Exception as error: + errors.append("attempt %d/%d: %s" % (attempt, attempts, error)) + vms = self.apiClient.listVirtualMachines( + _list_vms_cmd(vm_id)) or [] + state = (vms[0].state if vms else "") or "" + if attempt == attempts or state.lower() != "stopped": + raise Exception( + "startVirtualMachine failed after %d attempt(s), " + "VM state '%s':\n%s" + % (attempt, state, "\n".join(errors))) from error + log_progress( + logger, "warning", + "startVirtualMachine attempt %d/%d failed, retrying in " + "%ss: %s", attempt, attempts, self.VM_START_RETRY_DELAY, + error) + time.sleep(self.VM_START_RETRY_DELAY) + def _poll_vm_state(self, vm_id, target_state, timeout=300, interval=10): """ Poll listVirtualMachines until the VM reaches target_state.