Repository navigation
Enabling pre-submit by addressing the comments received from DevOps team - #119
Open
sandeeplocharla wants to merge 19 commits into
Open
sandeeplocharla wants to merge 19 commits into
sandeeplocharla wants to merge 19 commits into
Conversation
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 24.84% |
| Branch coverage | 19.00% |
Grade Scale
| Grade | Line Coverage | Meaning |
|---|---|---|
| 🟢 A | ≥ 80% | Excellent - this code sleeps well at night 😴 |
| 🟡 B | 60-79% | Good - almost there, don't stop now 😉 |
| 🟠 C | 40-59% | Acceptable - your code is wearing a seatbelt, but no airbags 😬 |
| 🔴 D | 20-39% | Marginal - boldly shipping where no test has gone before 🖖 |
| ⛔ F | < 20% | Failing - tests? what tests? 🔥 |
Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 24.84% |
| Branch coverage | 19.00% |
Grade Scale
| Grade | Line Coverage | Meaning |
|---|---|---|
| 🟢 A | ≥ 80% | Excellent - this code sleeps well at night 😴 |
| 🟡 B | 60-79% | Good - almost there, don't stop now 😉 |
| 🟠 C | 40-59% | Acceptable - your code is wearing a seatbelt, but no airbags 😬 |
| 🔴 D | 20-39% | Marginal - boldly shipping where no test has gone before 🖖 |
| ⛔ F | < 20% | Failing - tests? what tests? 🔥 |
Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 25.82% |
| Branch coverage | 20.11% |
Grade Scale
| Grade | Line Coverage | Meaning |
|---|---|---|
| 🟢 A | ≥ 80% | Excellent - this code sleeps well at night 😴 |
| 🟡 B | 60-79% | Good - almost there, don't stop now 😉 |
| 🟠 C | 40-59% | Acceptable - your code is wearing a seatbelt, but no airbags 😬 |
| 🔴 D | 20-39% | Marginal - boldly shipping where no test has gone before 🖖 |
| ⛔ F | < 20% | Failing - tests? what tests? 🔥 |
Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
The new pull-request script is not on main yet, so the PR-119 trial has to load it from feature/CSTACKEX-340. Change this back to main before the folder builds every pull request.
Jenkins warns that approving RunWrapper.getRawBuild lets a pipeline step outside the script sandbox. The existing job lookup already used for older builds can read the same revision.
A manual build does not copy the scan revision onto the build, so the prepare stage treated a real PR as unindexed.
The multibranch job never attaches a pull request revision, so refusing the build there skipped the GitHub eligibility check.
The environment block declared RESOLVED_SOURCE_ID and RESOLVED_SOURCE_SHA from build parameters. A multibranch pull request has no such parameters, and the block wraps every stage, so it shadowed the values set in prepareMultibranchPullRequest(). They read back empty, which caused a NullPointerException in presubmitSubjectBase and an empty SHA in the Maven and deploy shell steps. Set them in resolveSourceIdentity() instead. Also name the trusted CI branch in one place, trustedCiBranch(), and describe the multibranch folder configuration for the pilot.
Build 8 of PR-119 failed on the upstream, timing-sensitive ConfigKeyScheduledExecutionWrapperTest, which counts scheduled ticks in a fixed window and jitters in the CPU-limited agent pod. Add an opt-in RERUN_FAILING_TESTS setting to mvn-full.sh that passes -Dsurefire.rerunFailingTestsCount, and set it to 2 in the Jenkinsfile. The default stays 0 for local runs.
…out. The multibranch CI checkout fetched only private-cicd, so Phase 2 stopped with "Trusted ONTAP prerequisite checker is missing" right after the snapshot revert. run-phase2-remote.sh runs the trusted copy of test/integration/plugins/ontap/check_ontap_prereqs.py from the CI checkout, so add that single file to the sparse paths.
The HTML log link of a failed ONTAP test opened the suite stdout log, which only holds the pass/fail summary, so the reason for the failure was not visible. Parse the ERROR and FAIL blocks of the suite's results.txt, put them in a "Failure details" section above the suite log, and link a failed test straight to its block. Add an Error column to the mail table with a one-line reason, leading with the CloudStack job error text when there is one, and link the archived management and agent logs from the failure section.
test_06_start_vm_lun_remapped failed once with "Unable to apply dhcp entry on router": the guest network's router had reported failed health checks about a minute earlier, and the test started the VM with no check and no retry. Add to OntapTestBase a bounded router readiness gate, a deploy wrapper that waits before and after the deploy (the first deploy creates the router), and a start wrapper that retries a VM that rolled back to Stopped, at most three attempts, and raises every attempt's error if they all fail. A single wait timeout makes the class stop waiting, so a permanently flagged router cannot slow every VM operation. Use the wrappers in the iSCSI and NFS3 VM tests.
The stage table, the report's Started and Finished rows and the start mail showed UTC. Display them in IST (UTC+5:30). The stored timestamps and the stage durations stay UTC, so the arithmetic is unchanged.
sandeeplocharla
force-pushed
the
feature/CSTACKEX-340
branch
from
November 5, 2026 02:37
8fee48b to
7e3e850
Compare
sandeeplocharla
requested review from
piyush5netapp,
rajiv-jain-netapp,
sathvikaragi,
sp16743,
suryag1201 and
vallikns
November 5, 2026 04:05
Resolving the merge conflicts brought back a local _start_vm in the iSCSI and NFS3 VM volume tests. It used startVirtualMachineAPI, whose import had been removed, so test_08 and test_09 failed with a NameError and the snapshot tests were skipped. Remove the local copies and make the shared _start_vm in ontap_test_base.py skip a VM that is already running and wait for Running, as the local copies did.
When ONTAP rejects a createVMSnapshot that runs while a revert's clones exist, CloudStack keeps the snapshot record in Error. The retry in test_23 then succeeds, but the record has no id the test saw, so it was never deleted. Any Instance Snapshot blocks detach, resize and destroy, so test_24, test_25 and test_27 failed. Sweep Error-state VM snapshots of both test VMs in tearDown.
trustedCiBranch() named feature/CSTACKEX-340 for the PR-119 pilot. Return main so the presubmit uses the merged CI scripts for every pull request, and update the job guide to match. The Jenkins folder still has to switch its Remote Jenkinsfile Provider branch to main and its filter to PR-* after this merges.
suryag1201
approved these changes
Nov 6, 2026
vallikns
reviewed
Nov 12, 2026
| return | ||
| } | ||
| deleteDir() | ||
| checkout(scm) |
There was a problem hiding this comment.
checkoutCiScripts() now always clones this branch. Before, it used checkout(scm), so the manual branch/diagnose job ran the scripts from its own configured branch. Now it always runs main’s scripts, even when its Jenkinsfile comes from another branch.
The same applies before merge. main doesn’t have private-cicd/scripts/resolve-pull-request.py yet, so a build that loads this Jenkinsfile from the feature branch can’t find the script. It ends as NOT_BUILT with “pull request lookup failed”.
Could we take the branch from the job’s own SCM instead of a second hardcoded value that has to be kept in step with the Remote Jenkinsfile Provider setting by hand? If that isn’t possible, please add a note to the PR saying this value must be changed in the same step as the provider branch.
| f"base branch is '{decision['pr_base_branch']}' and change target " | ||
| f"is '{change_target}'." | ||
| ) | ||
| elif decision["pr_draft"]: |
There was a problem hiding this comment.
assess() rejects drafts, forks and wrong base branches, but never checks pull["state"]. The old flow stopped closed PRs (PR_ACTION == 'closed' in checkPresubmitEligibility). Now the Jenkinsfile hardcodes EFFECTIVE_PR_ACTION = 'synchronize', so that check can’t fire any more.
The folder keeps orphaned items for 14 days, according to the new docs. Anyone who hits Rebuild on a closed or merged PR would take a lab VM and run the full integration suite for nothing.
Please add a state != "open" rejection here, plus a test case in test_resolve_pull_request.py. The current tests only cover draft, fork, base branch and SHA mismatch.
| self.assertEqual(finish - start, report.timedelta(hours=1, minutes=30)) | ||
|
|
||
|
|
||
| class FailurePageTest(unittest.TestCase): |
There was a problem hiding this comment.
The regex stops at the first ' followed by a , or }. CloudStack error text often has its own quoted names. I ran the regex on this made-up message:
errortext : 'Unable to start VM 'i-2-5-VM', host unreachable'
It captures only Unable to start VM 'i-2-5-VM, so the Error column in the report cuts the useful part off. A message such as Volume doesn't exist is fine.
This is minor and only affects how the report looks. A fallback of “if the match ends before the closing }}, show the whole last line” would be enough.
- Choose the trusted CI branch from PRESUBMIT_TRUSTED_CI_BRANCH, then the manual job's own SCM, then main. A multibranch PR build never reads scm, since there it is the pull request's own code. Document the setting and that it must match the Remote Jenkinsfile Provider branch. - Reject a pull request that is not open in resolve-pull-request.py, because the closed-PR guard no longer fires in multibranch mode. Add tests. - End a CloudStack job error message at the quote that closes errortext, so quoted names inside the message are kept in the report. Add tests.
- Complete the Check after the report and archive exist, so its summary names the failed stage and links the Jenkins build and the HTML report. - Retry the completing PATCH three times; a Check left in progress blocks a pull request that requires it. - Publish an already completed Check when the one created at the start is missing, and complete a started Check as cancelled when the build is not built, so a result always reaches the pull request. - Document what each build result does to a required Check.
An ontap-coverage profile in the plugin pom, activated by -Dontap.coverage, attaches JaCoCo to the build that already runs the plugin's unit tests, so no second test run is needed. mvn-full.sh turns it on with ONTAP_COVERAGE=true and keeps the report. ontap-coverage.py summarises the unit and functional reports, with the combined figure computed per source line, and the report generator embeds the summary in the mail and report.html.
The plugin runs in the management server, so the JaCoCo agent is attached to its JVM before the first start. marvin-run.sh dumps the data after the iSCSI and the NFS3 suites. coverage-report.sh merges the dumps and builds the report against the plugin jar in the deployed management package. Every step degrades on its own and never fails the integration run.
The coverage summary is built before the report so the mail carries it, then added to the Check summary and posted as a single comment that later runs update in place. PRESUBMIT_COVERAGE=false turns the feature off and PRESUBMIT_COVERAGE_COMMENT=false keeps only the comment off. The comment step fails on its own if the App lacks write access, without changing the result.
sandeeplocharla
force-pushed
the
feature/CSTACKEX-340
branch
from
November 12, 2026 13:25
6674bc8 to
b5f778b
Compare
ONTAP storage plugin test coverage
Least covered files (by lines not covered by either run):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR has code to enable presubmits for NetApp ONTAP storage plugin with Multi-branch Jenkins pipeline
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?