CloudStack ONTAP presubmit
@@ -524,12 +733,13 @@ results follows when this run finishes.
// GitHub App diagnostic skip them, as does a request that `Check eligibility`
// rejected by setting NOT_BUILT.
boolean isWorkerStageEnabled() {
- return params.SOURCE_MODE in ['pull_request', 'branch'] &&
- currentBuild.result != 'NOT_BUILT'
+ return sourceValue('SOURCE_MODE') in ['pull_request', 'branch'] &&
+ currentBuild.result != 'NOT_BUILT' &&
+ currentBuild.currentResult != 'NOT_BUILT'
}
boolean isDiagnoseRun() {
- return params.SOURCE_MODE == 'diagnose'
+ return sourceValue('SOURCE_MODE') == 'diagnose'
}
void discoverPullRequests() {
@@ -660,18 +870,44 @@ void discoverPullRequests() {
}
}
+// Branch mode leaves PR_ID and PR_HEAD_SHA empty, so scripts receive a
+// mode-independent identity. The branch is sanitised because it reaches
+// a filesystem path on the target VM. A multibranch pull request has already
+// set both values in prepareMultibranchPullRequest().
+void resolveSourceIdentity() {
+ if (isMultibranchPullRequest()) {
+ return
+ }
+ if (params.SOURCE_MODE == 'branch') {
+ env.RESOLVED_SOURCE_ID =
+ 'branch-' + params.SOURCE_BRANCH.replaceAll('[^A-Za-z0-9._-]', '-')
+ env.RESOLVED_SOURCE_SHA = params.SOURCE_SHA
+ } else {
+ env.RESOLVED_SOURCE_ID = 'pr-' + params.PR_ID
+ env.RESOLVED_SOURCE_SHA = params.PR_HEAD_SHA
+ }
+}
+
void validateSourceRequest() {
+ resolveSourceIdentity()
sh 'rm -f "${WORKSPACE}@tmp/presubmit-stage-events.tsv"'
env.PRESUBMIT_INTEGRATION_STARTED = 'false'
- env.PRESUBMIT_STARTED_AT = new Date().format(
+ def startedAt = new Date()
+ // The UTC value feeds the report script and durations. The IST value is
+ // only what the start mail displays.
+ env.PRESUBMIT_STARTED_AT = startedAt.format(
"yyyy-MM-dd'T'HH:mm:ssXXX",
TimeZone.getTimeZone('UTC')
)
+ env.PRESUBMIT_STARTED_AT_IST = startedAt.format(
+ "yyyy-MM-dd HH:mm:ss 'IST'",
+ TimeZone.getTimeZone('Asia/Kolkata')
+ )
recordStageEvent('Validate source request', 'STARTED')
- if (!(params.SOURCE_MODE in ['pull_request', 'branch'])) {
- error("Unsupported SOURCE_MODE '${params.SOURCE_MODE}'.")
+ if (!(sourceValue('SOURCE_MODE') in ['pull_request', 'branch'])) {
+ error("Unsupported SOURCE_MODE '${sourceValue('SOURCE_MODE')}'.")
}
- if (params.SOURCE_MODE == 'branch') {
+ if (sourceValue('SOURCE_MODE') == 'branch') {
if (!isValidBranchName(params.SOURCE_BRANCH)) {
error('SOURCE_BRANCH is not a valid Git branch name.')
}
@@ -679,16 +915,16 @@ void validateSourceRequest() {
error('SOURCE_SHA must be a full 40-character Git SHA.')
}
} else {
- if (!(params.PR_ID ==~ /^[0-9]+$/)) {
+ if (!(sourceValue('PR_ID') ==~ /^[0-9]+$/)) {
error('PR_ID must be the numeric GitHub pull request number.')
}
- if (params.PR_REPOSITORY != params.EXPECTED_REPOSITORY) {
- error("Repository '${params.PR_REPOSITORY}' is not '${params.EXPECTED_REPOSITORY}'.")
+ if (sourceValue('PR_REPOSITORY') != params.EXPECTED_REPOSITORY) {
+ error("Repository '${sourceValue('PR_REPOSITORY')}' is not '${params.EXPECTED_REPOSITORY}'.")
}
- if (!(params.PR_HEAD_BRANCH ==~ /^[A-Za-z0-9._\/-]+$/)) {
+ if (!(sourceValue('PR_HEAD_BRANCH') ==~ /^[A-Za-z0-9._\/-]+$/)) {
error('PR_HEAD_BRANCH contains unsupported characters.')
}
- if (!(params.PR_HEAD_SHA ==~ /^[0-9a-fA-F]{40}$/) && params.PR_ACTION != 'closed') {
+ if (!(sourceValue('PR_HEAD_SHA') ==~ /^[0-9a-fA-F]{40}$/) && sourceValue('PR_ACTION') != 'closed') {
error('PR_HEAD_SHA must be a full 40-character Git SHA.')
}
}
@@ -718,15 +954,23 @@ void validateSourceRequest() {
}
}
try {
- def sourceKey = params.SOURCE_MODE == 'branch' ?
- "branch:${params.SOURCE_BRANCH}" :
- "pull_request:${params.PR_ID}"
- env.PRESUBMIT_DIFF_NUMBER = diffNumberForSource(
- env.JOB_NAME,
- env.BUILD_NUMBER.toInteger(),
- sourceKey,
- env.RESOLVED_SOURCE_SHA
- ).toString()
+ if (isMultibranchPullRequest()) {
+ env.PRESUBMIT_DIFF_NUMBER = diffNumberForResolvedSha(
+ env.JOB_NAME,
+ env.BUILD_NUMBER.toInteger(),
+ env.RESOLVED_SOURCE_SHA
+ ).toString()
+ } else {
+ def sourceKey = sourceValue('SOURCE_MODE') == 'branch' ?
+ "branch:${params.SOURCE_BRANCH}" :
+ "pull_request:${sourceValue('PR_ID')}"
+ env.PRESUBMIT_DIFF_NUMBER = diffNumberForSource(
+ env.JOB_NAME,
+ env.BUILD_NUMBER.toInteger(),
+ sourceKey,
+ env.RESOLVED_SOURCE_SHA
+ ).toString()
+ }
} catch (Exception e) {
env.PRESUBMIT_DIFF_NUMBER = ''
echo "Unable to determine the diff number: ${e}"
@@ -736,37 +980,51 @@ void validateSourceRequest() {
// five-minute discovery runs.
def diffLabel = env.PRESUBMIT_DIFF_NUMBER ?
" diff #${env.PRESUBMIT_DIFF_NUMBER}" : ''
- if (params.SOURCE_MODE == 'branch') {
+ if (sourceValue('SOURCE_MODE') == 'branch') {
currentBuild.displayName =
"#${env.BUILD_NUMBER} branch ${params.SOURCE_BRANCH}${diffLabel}"
currentBuild.description = "Branch ${params.SOURCE_BRANCH} @ ${params.SOURCE_SHA.take(12)}"
} else {
currentBuild.displayName =
- "#${env.BUILD_NUMBER} PR-${params.PR_ID}${diffLabel} ${params.PR_ACTION}"
+ "#${env.BUILD_NUMBER} PR-${sourceValue('PR_ID')}${diffLabel} ${sourceValue('PR_ACTION')}"
currentBuild.description =
- "${env.RESOLVED_SOURCE_SHA?.take(12)} ${params.PR_TITLE ?: params.PR_URL}"
+ "${env.RESOLVED_SOURCE_SHA?.take(12)} ${sourceValue('PR_TITLE') ?: sourceValue('PR_URL')}"
}
}
void checkPresubmitEligibility() {
recordStageEvent('Check eligibility', 'STARTED')
- if (params.SOURCE_MODE == 'pull_request') {
+ if (sourceValue('SOURCE_MODE') == 'pull_request') {
def eligibleActions = ['opened', 'synchronize', 'reopened', 'ready_for_review']
- if (params.PR_ACTION == 'closed' ||
- params.PR_DRAFT.toBoolean() ||
- params.PR_BASE_BRANCH != 'main' ||
- !(params.PR_ACTION in eligibleActions)) {
+ if (sourceValue('PR_ACTION') == 'closed' ||
+ sourceValue('PR_DRAFT').toBoolean() ||
+ sourceValue('PR_BASE_BRANCH') != 'main' ||
+ !(sourceValue('PR_ACTION') in eligibleActions)) {
currentBuild.result = 'NOT_BUILT'
- echo "Ignoring PR-${params.PR_ID}: action=${params.PR_ACTION}, draft=${params.PR_DRAFT}, base=${params.PR_BASE_BRANCH}"
+ echo "Ignoring PR-${sourceValue('PR_ID')}: action=${sourceValue('PR_ACTION')}, draft=${sourceValue('PR_DRAFT')}, base=${sourceValue('PR_BASE_BRANCH')}"
}
}
}
void abortSupersededRun() {
recordStageEvent('Abort superseded run', 'STARTED')
- def sourceKey = params.SOURCE_MODE == 'branch' ?
+ if (isMultibranchPullRequest()) {
+ try {
+ supersedeMultibranchBuilds(
+ env.JOB_NAME,
+ env.BUILD_NUMBER.toInteger(),
+ env.RESOLVED_SOURCE_SHA
+ ).each { result ->
+ echo result
+ }
+ } catch (Exception e) {
+ echo "Unable to inspect older builds; no build was aborted: ${e}"
+ }
+ return
+ }
+ def sourceKey = sourceValue('SOURCE_MODE') == 'branch' ?
"branch:${params.SOURCE_BRANCH}" :
- "pull_request:${params.PR_ID}"
+ "pull_request:${sourceValue('PR_ID')}"
try {
abortOlderBuildsForSource(
env.JOB_NAME,
@@ -780,10 +1038,157 @@ void abortSupersededRun() {
}
}
+// Branch name from a Git SCM branch spec such as '*/main' or 'refs/heads/main'.
+// Returns '' for a wildcard or any name Git would not accept.
+String branchFromSpec(String spec) {
+ def name = (spec ?: '').trim()
+ if (name.startsWith('*/')) {
+ name = name.substring(2)
+ }
+ if (name.startsWith('refs/heads/')) {
+ name = name.substring('refs/heads/'.length())
+ }
+ return name.contains('*') || !isValidBranchName(name) ? '' : name
+}
+
+// Branch that supplies the trusted CI scripts, in this order:
+// 1. PRESUBMIT_TRUSTED_CI_BRANCH, a job or folder environment variable. Set it
+// to the Remote Jenkinsfile Provider branch when that is not 'main', for
+// example while a Jenkinsfile change is piloted from a feature branch.
+// 2. For the manual job, the branch of the job's own Pipeline SCM.
+// 3. 'main'.
+// A multibranch pull request build never reads scm: there it is the pull
+// request's own code, and the trusted scripts must not come from the PR.
+String trustedCiBranch() {
+ def override = branchFromSpec(env.PRESUBMIT_TRUSTED_CI_BRANCH)
+ if (override) {
+ return override
+ }
+ if (!isMultibranchPullRequest()) {
+ try {
+ // A BranchSpec prints as its name; this avoids a new sandbox approval.
+ def fromJob = scm?.branches ? branchFromSpec("${scm.branches[0]}") : ''
+ if (fromJob) {
+ return fromJob
+ }
+ } catch (Exception e) {
+ echo "Could not read the job's SCM branch, using main: ${e}"
+ }
+ }
+ return 'main'
+}
+
void checkoutCiScripts() {
recordStageEvent('Checkout CI scripts', 'STARTED')
+ def ciBranch = trustedCiBranch()
+ if (env.PRESUBMIT_CI_SHA?.trim() && fileExists('private-cicd/scripts/resolve-pull-request.py')) {
+ echo "CI scripts already checked out from ${ciBranch} at ${env.PRESUBMIT_CI_SHA}."
+ return
+ }
deleteDir()
- checkout(scm)
+ def remote = [
+ url: params.CLOUDSTACK_GIT_URL,
+ refspec: "+refs/heads/${ciBranch}:refs/remotes/origin/${ciBranch}"
+ ]
+ def credentialId = params.GIT_CREDENTIALS_ID?.trim()
+ if (!credentialId) {
+ credentialId = params.GITHUB_APP_CREDENTIALS_ID?.trim()
+ }
+ if (credentialId && !credentialId.startsWith('PLACEHOLDER-')) {
+ remote.credentialsId = credentialId
+ }
+ def cloneOptions = [[
+ $class: 'CloneOption',
+ honorRefspec: true,
+ noTags: true,
+ shallow: true,
+ depth: 1
+ ]]
+ try {
+ checkout([
+ $class: 'GitSCM',
+ branches: [[name: "*/${ciBranch}"]],
+ doGenerateSubmoduleConfigurations: false,
+ extensions: cloneOptions + [[
+ $class: 'SparseCheckoutPaths',
+ // run-phase2-remote.sh runs the trusted copy of the ONTAP
+ // prerequisite checker from this checkout, never the PR's copy.
+ sparseCheckoutPaths: [
+ [path: 'private-cicd'],
+ [path: 'test/integration/plugins/ontap/check_ontap_prereqs.py']
+ ]
+ ]],
+ userRemoteConfigs: [remote]
+ ])
+ } catch (Exception e) {
+ echo "Sparse CI checkout unavailable, using a full ${ciBranch} checkout: ${e}"
+ deleteDir()
+ checkout([
+ $class: 'GitSCM',
+ branches: [[name: "*/${ciBranch}"]],
+ doGenerateSubmoduleConfigurations: false,
+ extensions: cloneOptions,
+ userRemoteConfigs: [remote]
+ ])
+ }
+ env.PRESUBMIT_CI_SHA = sh(script: 'git rev-parse HEAD', returnStdout: true).trim()
+ echo "Checked out CI scripts from ${ciBranch} at ${env.PRESUBMIT_CI_SHA}."
+}
+
+void prepareMultibranchPullRequest() {
+ checkoutCiScripts()
+ def indexed = indexedHeadSha(env.JOB_NAME, env.BUILD_NUMBER.toInteger())
+ if (!(indexed ==~ /^[0-9a-fA-F]{40}$/)) {
+ indexed = ''
+ echo "Jenkins did not expose an indexed head SHA for PR-${env.CHANGE_ID}. Using the GitHub pull request head."
+ }
+ def status = 1
+ withCredentials([
+ usernamePassword(
+ credentialsId: params.GITHUB_APP_CREDENTIALS_ID,
+ usernameVariable: 'GITHUB_APP',
+ passwordVariable: 'GITHUB_ACCESS_TOKEN'
+ )
+ ]) {
+ withEnv([
+ "PRESUBMIT_REPOSITORY=${params.EXPECTED_REPOSITORY}",
+ "PRESUBMIT_PR_ID=${env.CHANGE_ID}",
+ "PRESUBMIT_INDEXED_SHA=${indexed}",
+ "PRESUBMIT_CHANGE_TARGET=${env.CHANGE_TARGET ?: ''}"
+ ]) {
+ status = sh(
+ script: '''python3 private-cicd/scripts/resolve-pull-request.py \
+ --repository "$PRESUBMIT_REPOSITORY" \
+ --pr-id "$PRESUBMIT_PR_ID" \
+ --indexed-sha "$PRESUBMIT_INDEXED_SHA" \
+ --change-target "$PRESUBMIT_CHANGE_TARGET" \
+ --output multibranch-source.json''',
+ returnStatus: true
+ )
+ }
+ }
+ def decision = fileExists('multibranch-source.json') ? readJSON(file: 'multibranch-source.json') : [:]
+ if (status != 0 || !decision.eligible) {
+ currentBuild.result = 'NOT_BUILT'
+ echo "Not building PR-${env.CHANGE_ID}: ${decision.reason ?: 'pull request lookup failed.'}"
+ return
+ }
+ env.PRESUBMIT_MULTIBRANCH_READY = 'true'
+ env.EFFECTIVE_SOURCE_MODE = 'pull_request'
+ env.EFFECTIVE_PR_ID = decision.pr_id.toString()
+ env.EFFECTIVE_PR_ACTION = 'synchronize'
+ env.EFFECTIVE_PR_DRAFT = 'false'
+ env.EFFECTIVE_PR_REPOSITORY = params.EXPECTED_REPOSITORY
+ env.EFFECTIVE_PR_BASE_BRANCH = decision.pr_base_branch.toString()
+ env.EFFECTIVE_PR_HEAD_BRANCH = decision.pr_head_branch.toString()
+ env.EFFECTIVE_PR_HEAD_SHA = decision.pr_head_sha.toString()
+ env.EFFECTIVE_PR_AUTHOR_LOGIN = decision.pr_author_login?.toString() ?: ''
+ env.EFFECTIVE_PR_AUTHOR_EMAIL = decision.pr_author_email?.toString() ?: ''
+ env.EFFECTIVE_PR_URL = decision.pr_url?.toString() ?: ''
+ env.EFFECTIVE_PR_TITLE = decision.pr_title?.toString() ?: ''
+ env.RESOLVED_SOURCE_SHA = env.EFFECTIVE_PR_HEAD_SHA
+ env.RESOLVED_SOURCE_ID = "pr-${env.EFFECTIVE_PR_ID}"
+ echo "Accepted PR-${env.EFFECTIVE_PR_ID} at ${env.RESOLVED_SOURCE_SHA}."
}
// A full worker run takes over an hour, so reporting problems are diagnosed
@@ -793,11 +1198,11 @@ void diagnoseGithubApp() {
if (!hasGithubAppCredential()) {
error('GITHUB_APP_CREDENTIALS_ID is unset or still a placeholder.')
}
- def headSha = params.PR_HEAD_SHA?.trim() ?: params.SOURCE_SHA?.trim()
+ def headSha = sourceValue('PR_HEAD_SHA')?.trim() ?: params.SOURCE_SHA?.trim()
if (!(headSha ==~ /^[0-9a-f]{40}$/)) {
error('Set PR_HEAD_SHA to a full 40-character commit SHA to diagnose.')
}
- def repository = params.PR_REPOSITORY?.trim() ?: params.EXPECTED_REPOSITORY
+ def repository = sourceValue('PR_REPOSITORY')?.trim() ?: params.EXPECTED_REPOSITORY
withCredentials([
usernamePassword(
credentialsId: params.GITHUB_APP_CREDENTIALS_ID,
@@ -808,7 +1213,7 @@ void diagnoseGithubApp() {
withEnv([
"GITHUB_REPOSITORY=${repository}",
"GITHUB_HEAD_SHA=${headSha}",
- "PR_ID=${params.PR_ID ?: ''}"
+ "PR_ID=${sourceValue('PR_ID') ?: ''}"
]) {
sh 'private-cicd/scripts/github-check-run.sh diagnose'
}
@@ -863,18 +1268,18 @@ void checkoutSource() {
recordStageEvent('Checkout source', 'STARTED')
dir(env.SOURCE_DIR) {
deleteDir()
- def isBranch = params.SOURCE_MODE == 'branch'
- def sourceBranch = isBranch ? params.SOURCE_BRANCH : params.PR_HEAD_BRANCH
- def sourceSha = isBranch ? params.SOURCE_SHA : params.PR_HEAD_SHA
- def sourceLabel = isBranch ? "branch:${sourceBranch}" : "pull_request:${params.PR_ID}"
+ def isBranch = sourceValue('SOURCE_MODE') == 'branch'
+ def sourceBranch = isBranch ? params.SOURCE_BRANCH : sourceValue('PR_HEAD_BRANCH')
+ def sourceSha = isBranch ? params.SOURCE_SHA : sourceValue('PR_HEAD_SHA')
+ def sourceLabel = isBranch ? "branch:${sourceBranch}" : "pull_request:${sourceValue('PR_ID')}"
def remoteRef = isBranch ?
"refs/remotes/origin/source/${sourceBranch}" :
- "refs/remotes/origin/pr/${params.PR_ID}"
+ "refs/remotes/origin/pr/${sourceValue('PR_ID')}"
def remote = [
url: params.CLOUDSTACK_GIT_URL,
refspec: isBranch ?
"+refs/heads/${sourceBranch}:${remoteRef}" :
- "+refs/pull/${params.PR_ID}/head:${remoteRef}"
+ "+refs/pull/${sourceValue('PR_ID')}/head:${remoteRef}"
]
if (params.GIT_CREDENTIALS_ID?.trim()) {
remote.credentialsId = params.GIT_CREDENTIALS_ID
@@ -932,6 +1337,8 @@ void runUnitTests() {
SOURCE_SHA="$RESOLVED_SOURCE_SHA" \
ENABLE_SIMULATOR=false \
MAVEN_THREADS=1C \
+ RERUN_FAILING_TESTS=2 \
+ ONTAP_COVERAGE="${PRESUBMIT_COVERAGE:-true}" \
MAVEN_OPTS="-Xms1g -Xmx8g" \
private-cicd/scripts/mvn-full.sh
'''
@@ -987,9 +1394,9 @@ void deployAndRunOntapIntegration() {
env.VM_NAME = vm.vcenter_vm as String
env.VM_SNAPSHOT = vm.snapshot as String
env.VM_BRIDGE = vm.bridge as String
- def sourceDescription = params.SOURCE_MODE == 'branch' ?
+ def sourceDescription = sourceValue('SOURCE_MODE') == 'branch' ?
"branch=${params.SOURCE_BRANCH}" :
- "PR-${params.PR_ID}"
+ "PR-${sourceValue('PR_ID')}"
currentBuild.description = "${sourceDescription} VM=${env.VM_ID}"
echo "Reserved compatible VM '${env.VM_ID}' via '${env.LOCKED_VM}'."
@@ -1054,6 +1461,7 @@ void deployAndRunOntapIntegration() {
SOURCE_ID="$RESOLVED_SOURCE_ID" \
SOURCE_SHA="$RESOLVED_SOURCE_SHA" \
BRIDGE="$VM_BRIDGE" \
+ FUNCTIONAL_COVERAGE="${PRESUBMIT_COVERAGE:-true}" \
private-cicd/scripts/run-phase2-remote.sh
'''
}
@@ -1063,7 +1471,15 @@ void deployAndRunOntapIntegration() {
}
void publishPresubmitResults() {
- if (params.SOURCE_MODE == 'discover') {
+ if (isMultibranchPullRequest() && currentBuild.currentResult == 'NOT_BUILT') {
+ echo "Multibranch PR-${env.CHANGE_ID} was not built."
+ // A check created before the build stopped must not stay in progress.
+ if (env.GITHUB_CHECK_RUN_ID?.trim()) {
+ completeGithubCheck('cancelled')
+ }
+ return
+ }
+ if (sourceValue('SOURCE_MODE') == 'discover') {
echo 'Discovery build completed; worker reporting was skipped.'
return
}
@@ -1084,19 +1500,20 @@ void publishPresubmitResults() {
"yyyy-MM-dd'T'HH:mm:ssXXX",
TimeZone.getTimeZone('UTC')
)
- completeGithubCheck(conclusion)
if (!env.PRESUBMIT_MAIL_TO?.trim()) {
resolvePresubmitMailRecipient()
}
- def sourceName = params.SOURCE_MODE == 'branch' ?
- "branch ${params.SOURCE_BRANCH}" : "PR-${params.PR_ID}"
+ def sourceName = sourceValue('SOURCE_MODE') == 'branch' ?
+ "branch ${params.SOURCE_BRANCH}" : "PR-${sourceValue('PR_ID')}"
+ // Before the report, which embeds the coverage summary.
+ def coverageFile = buildCoverageSummary()
try {
if (fileExists('private-cicd/scripts/generate-presubmit-report.py')) {
withEnv([
"PRESUBMIT_REPORT_RESULT=${result}",
"PRESUBMIT_REPORT_SOURCE=${sourceName}",
- "PRESUBMIT_REPORT_SOURCE_URL=${params.PR_URL ?: ''}",
- "PRESUBMIT_REPORT_TITLE=${params.PR_TITLE ?: ''}",
+ "PRESUBMIT_REPORT_SOURCE_URL=${sourceValue('PR_URL') ?: ''}",
+ "PRESUBMIT_REPORT_TITLE=${sourceValue('PR_TITLE') ?: ''}",
"PRESUBMIT_REPORT_SHA=${env.RESOLVED_SOURCE_SHA}",
"PRESUBMIT_REPORT_DIFF=${env.PRESUBMIT_DIFF_NUMBER ?: ''}",
"PRESUBMIT_REPORT_VM=${env.VM_ID ?: ''}"
@@ -1135,6 +1552,12 @@ void publishPresubmitResults() {
}
def failedStage = fileExists("${env.PRESUBMIT_RESULTS}/failed-stage.txt") ?
readFile("${env.PRESUBMIT_RESULTS}/failed-stage.txt").trim() : ''
+ // Completed after the report and the archive exist, so the check can
+ // name the failed stage and link the report.
+ def reportUrl = fileExists("${env.PRESUBMIT_RESULTS}/report.html") ?
+ "${env.BUILD_URL}artifact/${env.PRESUBMIT_RESULTS}/report.html" : ''
+ completeGithubCheck(conclusion, failedStage, reportUrl, coverageFile)
+ postCoverageComment(coverageFile)
def finalSubject = failedStage ?
"${presubmitSubjectBase()}: ${result} in ${failedStage}" :
"${presubmitSubjectBase()}: ${result}"
@@ -1193,15 +1616,11 @@ spec:
timeout(time: 8, unit: 'HOURS')
}
- triggers {
- cron('H/5 * * * *')
- }
-
parameters {
choice(
name: 'SOURCE_MODE',
choices: ['discover', 'pull_request', 'branch', 'diagnose'],
- description: 'Timer builds use discover. Self-queued workers use pull_request. Use branch for a manual exact-SHA build, or diagnose to probe GitHub App Checks access using PR_HEAD_SHA.'
+ description: 'Legacy discover mode is not scheduled. Multibranch pull requests ignore this default. Use pull_request for a manual PR build, branch for an exact SHA, or diagnose to probe GitHub App Checks access.'
)
string(name: 'SOURCE_BRANCH', defaultValue: '', description: 'Manual branch mode: remote branch name, for example feature/CSTACKEX-223.')
string(name: 'SOURCE_SHA', defaultValue: '', description: 'Manual branch mode: exact full 40-character commit SHA.')
@@ -1249,18 +1668,17 @@ spec:
environment {
SOURCE_DIR = 'cloudstack-src'
PRESUBMIT_RESULTS = 'presubmit-results'
- // Branch mode leaves PR_ID and PR_HEAD_SHA empty, so scripts receive a
- // mode-independent identity. The branch is sanitised because it reaches
- // a filesystem path on the target VM.
- RESOLVED_SOURCE_ID = "${params.SOURCE_MODE == 'branch' ? 'branch-' + params.SOURCE_BRANCH.replaceAll('[^A-Za-z0-9._-]', '-') : 'pr-' + params.PR_ID}"
- RESOLVED_SOURCE_SHA = "${params.SOURCE_MODE == 'branch' ? params.SOURCE_SHA : params.PR_HEAD_SHA}"
+ // RESOLVED_SOURCE_ID and RESOLVED_SOURCE_SHA are deliberately not
+ // declared here. This block wraps every stage in withEnv, so a value
+ // declared here shadows the multibranch assignment made later and
+ // reads back empty. See resolveSourceIdentity().
}
stages {
stage('Discover PR revisions') {
when {
expression {
- return params.SOURCE_MODE == 'discover'
+ return sourceValue('SOURCE_MODE') == 'discover' && !isMultibranchPullRequest()
}
}
steps {
@@ -1270,6 +1688,19 @@ spec:
}
}
+ stage('Prepare multibranch pull request') {
+ when {
+ expression {
+ return isMultibranchPullRequest()
+ }
+ }
+ steps {
+ script {
+ prepareMultibranchPullRequest()
+ }
+ }
+ }
+
stage('Validate source request') {
when {
expression {
diff --git a/private-cicd/README.md b/private-cicd/README.md
index ae48b35a9a75..df978fd69ddf 100644
--- a/private-cicd/README.md
+++ b/private-cicd/README.md
@@ -28,10 +28,11 @@ current implementation contract, see
## How it works
-The production Jenkins job loads [`Jenkinsfile`](Jenkinsfile) and helper scripts
-from the protected NetApp `main` branch. Every five minutes a short discovery
-build polls GitHub and self-queues a worker for each unseen PR head SHA. Each
-worker checks out the exact commit into a separate `cloudstack-src` directory,
+The production job is the multibranch folder `cloudstack-ontap-presubmit-mb`.
+Jenkins scans GitHub about once a minute. That scan is not a build. Each
+non-draft origin pull request into `main` gets its own child job. The child
+loads [`Jenkinsfile`](Jenkinsfile) and helper scripts from protected `main`,
+then checks the exact pull-request head SHA out into `cloudstack-src`. A child
then:
1. validates the request, builder, credentials, and VM inventory;
@@ -48,10 +49,10 @@ replacing the underlying build or test result.
## Entry points
-- **Pull request:** `SOURCE_MODE=discover` notices an open non-draft PR update
- to `main` and self-queues at most five `SOURCE_MODE=pull_request` workers in
- the same job. Extra SHAs wait for a later poll. Workers then wait on the
- `cloudstack-presubmit-vm` lock for a lab VM.
+- **Pull request:** a folder scan creates one child job per open non-draft
+ origin pull request into `main`. Drafts get no job. Compiles run together.
+ Integration waits on the `cloudstack-presubmit-vm` lock. There is no
+ five-worker cap. GitHub cannot call this Jenkins, so there is no webhook.
- **Manual branch:** a separate triggerless job uses `SOURCE_MODE=branch` with
a remote branch and exact 40-character commit SHA.
- **Local validation:** `scripts/validate-local.sh` checks the CI files without
diff --git a/private-cicd/docs/CREATE-PRESUBMIT-JOB.md b/private-cicd/docs/CREATE-PRESUBMIT-JOB.md
index 18fc9053d38a..37291f682777 100644
--- a/private-cicd/docs/CREATE-PRESUBMIT-JOB.md
+++ b/private-cicd/docs/CREATE-PRESUBMIT-JOB.md
@@ -19,12 +19,73 @@
# Create the auto-triggered ONTAP presubmit job
-This guide creates **one** Pipeline job, `cloudstack-ontap-presubmit`. Timer
-builds poll GitHub every five minutes and self-queue one worker build for each
-new eligible PR head SHA. Worker builds compile and package concurrently, then
-wait for an available `cloudstack-presubmit-vm` Lockable Resource before
-integration. Until `feature/CSTACKEX-223` is merged, the job loads trusted CI
-from that branch. After merge, change only **Branches to build** to `*/main`.
+The current job is the multibranch folder `cloudstack-ontap-presubmit-mb`.
+It scans GitHub about once a minute and creates one child job for every open
+non-draft origin pull request into `main`. Remote Jenkinsfile Provider loads
+`private-cicd/Jenkinsfile` from the trusted CI branch. Match branches stays off.
+Draft pull requests get no job. Do not add a webhook.
+
+The trusted CI branch is `main`. The Jenkinsfile and the CI scripts of every pull
+request build come from the trusted branch, never from the pull request, so a PR
+that changes `private-cicd/` is built with the unchanged trusted copy. Its
+changes apply only after they are merged. To try a Jenkinsfile or script change
+before the merge, run the manual job (`SOURCE_MODE=pull_request` or `branch`)
+against the feature branch.
+
+The CI scripts branch is chosen by `trustedCiBranch()` in the Jenkinsfile:
+
+1. the `PRESUBMIT_TRUSTED_CI_BRANCH` environment variable, if the job or folder
+ defines it;
+2. for the manual job, the branch of its own Pipeline SCM;
+3. otherwise `main`.
+
+A multibranch pull request build cannot read its branch from `scm`, because
+there `scm` is the pull request's own code. The value must therefore equal the
+Remote Jenkinsfile Provider branch. When the provider branch is not `main`, set
+`PRESUBMIT_TRUSTED_CI_BRANCH` to the same branch in the same step. A mismatch
+shows as `NOT_BUILT` with "pull request lookup failed", because the scripts
+are missing on the other branch.
+
+During the PR-119 pilot the provider branch was `feature/CSTACKEX-340`, the
+folder include filter was `PR-119`, and `PRESUBMIT_TRUSTED_CI_BRANCH` had to name
+that branch. Do not switch the provider branch to `main` until PR-119 is merged:
+the previous `main` Jenkinsfile knows nothing about `CHANGE_ID` and defaults to
+`SOURCE_MODE=discover`. After the merge, change the provider branch to `main`,
+remove `PRESUBMIT_TRUSTED_CI_BRANCH` and change the filter to `PR-*`. Keep
+`feature/CSTACKEX-340` until then.
+
+A pull request that is closed or merged is not built, even if someone selects
+Rebuild on an orphaned item: the build ends `NOT_BUILT` with "pull request is not
+open".
+
+The numbered sections below that create a single Pipeline job with a five-minute
+`SOURCE_MODE=discover` timer (11 to 16) are retired. Sections on plugins,
+credentials, the inventory, Lockable Resources, and the GitHub App still apply.
+Keep `cloudstack-ontap-presubmit` disabled after the folder works. Do not delete
+the GitHub App, credentials, or the VM lock until a multibranch pull request has
+passed.
+
+## Multibranch folder configuration
+
+Open `cloudstack-ontap-presubmit-mb` > **Configure**:
+
+| Setting | Value |
+| --- | --- |
+| Branch Source | GitHub, credential `cloudstack-presubmit-github-app`, repository `NetApp/cloudstack` |
+| Behaviours | Discover pull requests from origin only, strategy **The current pull request revision**; remove branch and fork discovery |
+| Filter by name | `PR-*` (`PR-119` during the pilot) |
+| Build Configuration | **by Remote Jenkinsfile Provider**, repository `https://github.com/NetApp/cloudstack.git`, branch `main` (`feature/CSTACKEX-340` during the pilot), file `private-cicd/Jenkinsfile`, **Match branches off** |
+| Scan Repository Triggers | Periodically, 1 to 5 minutes |
+| Orphaned items | Keep 14 days, 50 items |
+
+Run **Scan Repository Now** and expect one child for each open non-draft pull
+request into `main` (only `PR-119` during the pilot). The first build may
+stop on Script Security. Approve only signatures requested by this Jenkinsfile
+and rebuild.
+
+The single-branch Pipeline job is now only the manual job for
+`SOURCE_MODE=branch` and `diagnose`. Point it at the same trusted branch, add no
+triggers, and never run `SOURCE_MODE=discover`.
Do not create a GitHub repository webhook. OpenLab Jenkins is not reachable
from GitHub.com, so inbound `pull_request` deliveries fail with
@@ -36,8 +97,8 @@ item for this presubmit: that would run `Jenkinsfile` from the PR branch. This
design loads trusted CI from a fixed branch and checks the PR SHA out
separately.
-Do not create a separate poller, Multibranch Pipeline, or second production
-copy of this job.
+The multibranch folder above is the production job. Do not create another
+poller or a second production copy.
This guide starts before the Jenkins item exists and ends after polling and
required Check have been verified. Follow sections 1 through 21 in order. Do
diff --git a/private-cicd/docs/PRIVATE-CICD-GUIDE.md b/private-cicd/docs/PRIVATE-CICD-GUIDE.md
index 54d4dae56a6f..e7576b09241f 100644
--- a/private-cicd/docs/PRIVATE-CICD-GUIDE.md
+++ b/private-cicd/docs/PRIVATE-CICD-GUIDE.md
@@ -30,9 +30,9 @@ This is the consolidated operator and developer reference. The current [`Jenkins
The guide covers:
-- the trusted production Pipeline-from-SCM job with five-minute GitHub API
- discovery, including first-PR cutover from `feature/CSTACKEX-223` to `main`
- ([`CREATE-PRESUBMIT-JOB.md`](CREATE-PRESUBMIT-JOB.md));
+- the multibranch folder `cloudstack-ontap-presubmit-mb`, which scans about
+ once a minute and runs trusted CI from `main` for every non-draft origin
+ pull request into `main` ([`CREATE-PRESUBMIT-JOB.md`](CREATE-PRESUBMIT-JOB.md));
- the separate triggerless manual branch job;
- local CI-file validation and direct disposable-VM validation;
- Stage 1 build, unit-test, and Debian package handoff;
@@ -68,12 +68,10 @@ VM lock.
## 3. Architecture and data flow
```text
-Jenkins timer -> discover mode (every 5 minutes)
- -> GitHub API -> unseen PR head SHAs
- -> self-queue at most MAX_ACTIVE_WORKERS pull_request builds
- -> extra SHAs deferred with watermark held
- -> self-queued worker builds in the same job
- -> trusted Jenkinsfile/scripts from protected main
+Folder scan about once a minute (not a pipeline build)
+ -> one child job per open non-draft origin PR into main
+ -> trusted Jenkinsfile/scripts from protected main
+ -> exact indexed PR head SHA, confirmed with the pull request API
-> request, eligibility, credential, inventory, and builder checks
-> exact PR or branch SHA checkout into cloudstack-src
-> Maven clean install and unit tests
@@ -156,12 +154,10 @@ Lightweight checkout: off
PR code cannot replace trusted CI helpers before credentials are used. The source under test is checked out
separately.
-Until `feature/CSTACKEX-223` is merged, that Jenkinsfile is not on `main`. Create one job named
-`cloudstack-ontap-presubmit` that loads CI from `*/feature/CSTACKEX-223`, using
-[`CREATE-PRESUBMIT-JOB.md`](CREATE-PRESUBMIT-JOB.md). Its timer builds discover work and self-queue workers in the
-same item. After merge, change only **Branches to build** to `*/main`. Keep the same job, GitHub App, credentials,
-resources, and Check. Do not create a GitHub repository webhook: OpenLab Jenkins is not reachable from GitHub.com.
-Do not create a GitHub Organization folder for this presubmit.
+Production is the multibranch folder `cloudstack-ontap-presubmit-mb`. Remote Jenkinsfile
+Provider loads this Jenkinsfile from `main` with Match branches off. A folder scan,
+not a webhook, finds pull requests. Keep the old `cloudstack-ontap-presubmit` job
+disabled. Do not create a GitHub repository webhook or a GitHub Organization folder.
### Manual-job trust boundary
@@ -550,6 +546,60 @@ cloudstack-ontap-presubmit
Without protection it is informational. Verify pending/failed results block merge and each new SHA needs a new Check.
+How the Check behaves once it is required:
+
+| Build result | Check conclusion | Merge |
+| --- | --- | --- |
+| `SUCCESS` | `success` | allowed |
+| `FAILURE`, `UNSTABLE` | `failure` | blocked |
+| `ABORTED`, superseded or timed out | `cancelled` | blocked until a new run passes |
+| not built (draft, fork, closed, wrong base) | no Check, or `cancelled` if one was started | blocked until a run passes |
+
+- The Check is created `in_progress` at the start of the run and completed after the report is archived. The
+ completed Check names the failed stage and links the Jenkins build and the HTML report.
+- If creating the Check failed at the start, the final step publishes an already completed Check instead, so the
+ result still reaches the pull request. If that also fails, the job description says
+ `GitHub Check result not published`, and the Check stays missing or in progress, which blocks the merge.
+ Fix the cause (run `SOURCE_MODE=diagnose`), then push a commit or rebuild the PR job.
+- Completion is retried three times. A Jenkins crash or restart mid-run still leaves the Check in progress. Rebuild
+ the PR job, or push a commit, to replace it. A GitHub **Re-run** button does nothing, because Jenkins receives no
+ webhooks.
+- In the branch rule, select the Check produced by the presubmit GitHub App as its source, not **Any source**.
+- Do not require the `continuous-integration/jenkins/pr-head` status that GitHub Branch Source can also post. The
+ Check above is the only gate.
+- A pull request from a fork never gets a Check, so it cannot merge while the Check is required.
+
+### ONTAP plugin test coverage
+
+Every run reports the coverage of the ONTAP storage plugin
+(`plugins/storage/volume/ontap`) from two sources, in the mail, in `report.html`, in the summary of the
+`cloudstack-ontap-presubmit` Check, and in one PR comment that later runs update in place.
+
+| Source | How it is produced |
+| --- | --- |
+| Unit tests | `mvn-full.sh` passes `-Dontap.coverage=true`, which activates the `ontap-coverage` profile in the plugin's `pom.xml`. The same Maven build that runs the tests writes `target/site/jacoco/jacoco.xml`. `mvn-full.sh` copies it to `presubmit-results/unit-tests/coverage/unit/`. |
+| Functional (Marvin) tests | `run-phase2-remote.sh` fetches the JaCoCo agent and command line (`fetch-jacoco.sh`, version 0.8.11) and ships them to the VM. `configure-cloudstack-vm.sh` adds `-javaagent:...,output=tcpserver,address=127.0.0.1,port=36320` to the management server `JAVA_OPTS` before its first start. `marvin-run.sh` dumps and resets the data after the iSCSI suites and again after the NFS3 suites into `phase2/coverage/*.exec`. |
+| Summary | `coverage-report.sh` merges the dumps and builds a report from the plugin jar inside the deployed management package, so the report matches the code that ran. `ontap-coverage.py` then writes `coverage/summary.{json,md,html}`. |
+
+Notes:
+
+- The **combined** figure is line coverage over the union of both runs, taken line by line from the two reports, so
+ it does not depend on the two reports agreeing on class IDs. Branch and method figures are shown per source only.
+- Only the management server is instrumented, which is where the ONTAP plugin runs. The KVM agent is not.
+- The coverage is informational. It never changes the build result or the Check conclusion.
+- Every part degrades on its own. If the JaCoCo jars cannot be fetched, the agent cannot be attached, a dump fails or
+ the report cannot be built, the run goes on and the summary says which source is missing.
+- A management server restart between two dumps loses the data collected since the last dump.
+- The JaCoCo HTML under `coverage/functional/html` and `unit-tests/coverage/unit` is linked for detail, but Jenkins
+ blocks the scripts and styles of archived HTML, so those pages look plain. The tables in the mail and the PR come
+ from the summary instead.
+- `PRESUBMIT_COVERAGE=false` (a job, folder or global environment variable) turns the whole feature off.
+ `PRESUBMIT_COVERAGE_COMMENT=false` keeps the summary in the mail and the Check but stops the PR comment.
+- The PR comment needs **Pull requests: Read and write** (or **Issues: Read and write**) on the GitHub App. Without
+ it only the comment step fails; the log says so and the build is unchanged. The comment is found by the marker
+ `` among comments written by a bot, so it does not touch the `coverage-grade-bot` comment
+ that the repository's own workflow posts.
+
## 13. Manual branch job and exact SHA
Create a separate Pipeline-from-SCM job:
diff --git a/private-cicd/scripts/configure-cloudstack-vm.sh b/private-cicd/scripts/configure-cloudstack-vm.sh
index 24d9746568c8..7aeeee85881d 100755
--- a/private-cicd/scripts/configure-cloudstack-vm.sh
+++ b/private-cicd/scripts/configure-cloudstack-vm.sh
@@ -428,6 +428,25 @@ until systemctl is-active --quiet libvirtd; do
sleep 1
libvirtd_wait=$((libvirtd_wait + 1))
done
+# Functional-test coverage: the ONTAP plugin runs inside the management server
+# JVM, so the JaCoCo agent is attached to it before its first start. A problem
+# here only loses the coverage data, it never fails the integration run.
+if [[ -n "${JACOCO_AGENT_JAR:-}" ]]; then
+ defaults_file=/etc/default/cloudstack-management
+ if [[ ! -f "$JACOCO_AGENT_JAR" ]]; then
+ echo "JACOCO_AGENT_JAR $JACOCO_AGENT_JAR does not exist; functional coverage is off." >&2
+ elif ! grep -q '^JAVA_OPTS="' "$defaults_file"; then
+ echo "No JAVA_OPTS line in $defaults_file; functional coverage is off." >&2
+ elif grep -q 'jacocoagent' "$defaults_file"; then
+ echo "The JaCoCo agent is already configured in $defaults_file."
+ else
+ install -d -m 755 /opt/jacoco
+ install -m 644 "$JACOCO_AGENT_JAR" /opt/jacoco/jacocoagent.jar
+ agent_option="-javaagent:/opt/jacoco/jacocoagent.jar=output=tcpserver,address=127.0.0.1,port=${COVERAGE_PORT:-36320},includes=org.apache.cloudstack.storage.*"
+ sed -i "s|^JAVA_OPTS=\"\(.*\)\"\$|JAVA_OPTS=\"\1 ${agent_option}\"|" "$defaults_file"
+ echo "JaCoCo agent added to the management server JAVA_OPTS."
+ fi
+fi
db_trace_state "before first start of cloudstack-management"
systemctl start cloudstack-management
systemctl restart cloudstack-agent
diff --git a/private-cicd/scripts/coverage-report.sh b/private-cicd/scripts/coverage-report.sh
new file mode 100755
index 000000000000..65757c3fdea3
--- /dev/null
+++ b/private-cicd/scripts/coverage-report.sh
@@ -0,0 +1,105 @@
+#!/usr/bin/env bash
+# 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.
+
+# Purpose:
+# Builds the ONTAP storage plugin coverage summary of one presubmit run. The
+# unit-test JaCoCo report already exists, written by the Maven build. The
+# functional report is made here from the management server dumps that
+# marvin-run.sh collected on the lab VM, using the plugin jar from the
+# management package that was actually deployed. ontap-coverage.py then
+# writes summary.json, summary.md and summary.html.
+# This script never fails the presubmit: every problem is reported and the
+# summary says which part of the coverage is missing.
+# Dependencies:
+# RESULTS_DIR — the presubmit results directory (required). Reads
+# unit-tests/coverage/unit/jacoco.xml and phase2/coverage/*.exec, writes
+# coverage/.
+# SOURCE_DIR — CloudStack source checkout (required): ONTAP sources.
+# Optional: DEB_DIR (default $SOURCE_DIR/dist/deb-all) for the deployed jar,
+# BUILD_URL to link the reports.
+# Commands: java, python3, dpkg-deb; fetch-jacoco.sh for
+# the JaCoCo command line.
+# Calls: fetch-jacoco.sh, ontap-coverage.py.
+
+set -uo pipefail
+
+RESULTS_DIR="${RESULTS_DIR:?Set RESULTS_DIR}"
+SOURCE_DIR="${SOURCE_DIR:?Set SOURCE_DIR}"
+DEB_DIR="${DEB_DIR:-$SOURCE_DIR/dist/deb-all}"
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+
+ontap_module="plugins/storage/volume/ontap"
+out_dir="$RESULTS_DIR/coverage"
+unit_xml="$RESULTS_DIR/unit-tests/coverage/unit/jacoco.xml"
+exec_dir="$RESULTS_DIR/phase2/coverage"
+work_dir="$(mktemp -d)"
+trap 'rm -rf "$work_dir"' EXIT
+mkdir -p "$out_dir"
+
+functional_xml=""
+shopt -s nullglob
+exec_files=("$exec_dir"/*.exec)
+shopt -u nullglob
+
+if [[ "${#exec_files[@]}" -eq 0 ]]; then
+ echo "No functional coverage dumps under $exec_dir."
+elif ! "$SCRIPT_DIR/fetch-jacoco.sh" "$work_dir/jacoco" >/dev/null 2>&1; then
+ echo "Could not fetch the JaCoCo command line; functional coverage is skipped." >&2
+else
+ cli="$work_dir/jacoco/jacococli.jar"
+ classfiles=""
+ # The jar inside the deployed management package is the code that ran.
+ management_deb="$(find "$DEB_DIR" -maxdepth 1 -name 'cloudstack-management_*.deb' -print -quit 2>/dev/null)"
+ if [[ -n "$management_deb" ]] && dpkg-deb -x "$management_deb" "$work_dir/deployed" 2>/dev/null; then
+ classfiles="$(find "$work_dir/deployed" -name 'cloud-plugin-storage-volume-ontap-*.jar' \
+ ! -name '*-sources.jar' ! -name '*-tests.jar' -print -quit)"
+ fi
+ if [[ -z "$classfiles" && -d "$SOURCE_DIR/$ontap_module/target/classes" ]]; then
+ classfiles="$SOURCE_DIR/$ontap_module/target/classes"
+ echo "Deployed plugin jar not found; using $classfiles."
+ fi
+ if [[ -z "$classfiles" ]]; then
+ echo "No ONTAP plugin classes found; functional coverage is skipped." >&2
+ elif mkdir -p "$out_dir/functional" \
+ && java -jar "$cli" merge "${exec_files[@]}" --destfile "$work_dir/functional.exec" >/dev/null \
+ && java -jar "$cli" report "$work_dir/functional.exec" \
+ --classfiles "$classfiles" \
+ --sourcefiles "$SOURCE_DIR/$ontap_module/src/main/java" \
+ --name "ONTAP functional tests" \
+ --xml "$out_dir/functional/jacoco.xml" \
+ --csv "$out_dir/functional/jacoco.csv" \
+ --html "$out_dir/functional/html" >/dev/null; then
+ functional_xml="$out_dir/functional/jacoco.xml"
+ echo "Functional coverage report: $functional_xml"
+ else
+ echo "Could not build the functional coverage report." >&2
+ fi
+fi
+
+summary_args=(--output-dir "$out_dir")
+[[ -f "$unit_xml" ]] && summary_args+=(--unit "$unit_xml")
+[[ -n "$functional_xml" ]] && summary_args+=(--functional "$functional_xml")
+if [[ -n "${BUILD_URL:-}" ]]; then
+ [[ -f "$unit_xml" ]] && summary_args+=(
+ --unit-link "${BUILD_URL}artifact/presubmit-results/unit-tests/coverage/unit/index.html")
+ [[ -n "$functional_xml" ]] && summary_args+=(
+ --functional-link "${BUILD_URL}artifact/presubmit-results/coverage/functional/html/index.html")
+fi
+python3 "$SCRIPT_DIR/ontap-coverage.py" "${summary_args[@]}" \
+ || echo "Could not write the coverage summary." >&2
+exit 0
diff --git a/private-cicd/scripts/fetch-jacoco.sh b/private-cicd/scripts/fetch-jacoco.sh
new file mode 100755
index 000000000000..7f73d049ea91
--- /dev/null
+++ b/private-cicd/scripts/fetch-jacoco.sh
@@ -0,0 +1,62 @@
+#!/usr/bin/env bash
+# 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.
+
+# Purpose:
+# Copies the JaCoCo agent and command-line jars into a directory as
+# jacocoagent.jar and jacococli.jar. The agent is attached to the management
+# server on the lab VM, and the command line dumps and reports coverage.
+# Jars already present in the directory are kept, so a second call is free.
+# Dependencies:
+# Usage: fetch-jacoco.sh DEST_DIR
+# Optional: JACOCO_VERSION (default 0.8.11, the version pinned in the root pom).
+# Commands: mvn with access to Maven Central or a mirror.
+# Called by: run-phase2-remote.sh, coverage-report.sh.
+
+set -euo pipefail
+
+DEST_DIR="${1:?Usage: fetch-jacoco.sh DEST_DIR}"
+JACOCO_VERSION="${JACOCO_VERSION:-0.8.11}"
+DEPENDENCY_PLUGIN="org.apache.maven.plugins:maven-dependency-plugin:3.6.1:copy"
+
+mkdir -p "$DEST_DIR"
+DEST_DIR="$(cd "$DEST_DIR" && pwd)"
+if [[ -f "$DEST_DIR/jacocoagent.jar" && -f "$DEST_DIR/jacococli.jar" ]]; then
+ exit 0
+fi
+
+work_dir="$(mktemp -d)"
+trap 'rm -rf "$work_dir"' EXIT
+
+# The agent is the "runtime" classifier of org.jacoco.agent; the command line is
+# the self-contained "nodeps" jar of org.jacoco.cli.
+mvn -B -q "$DEPENDENCY_PLUGIN" \
+ "-Dartifact=org.jacoco:org.jacoco.agent:${JACOCO_VERSION}:jar:runtime" \
+ "-DoutputDirectory=${work_dir}/agent" -Dmdep.stripVersion=true
+mvn -B -q "$DEPENDENCY_PLUGIN" \
+ "-Dartifact=org.jacoco:org.jacoco.cli:${JACOCO_VERSION}:jar:nodeps" \
+ "-DoutputDirectory=${work_dir}/cli" -Dmdep.stripVersion=true
+
+agent_jar="$(find "$work_dir/agent" -name '*.jar' -print -quit)"
+cli_jar="$(find "$work_dir/cli" -name '*.jar' -print -quit)"
+if [[ -z "$agent_jar" || -z "$cli_jar" ]]; then
+ echo "Could not fetch the JaCoCo ${JACOCO_VERSION} jars" >&2
+ exit 1
+fi
+cp "$agent_jar" "$DEST_DIR/jacocoagent.jar"
+cp "$cli_jar" "$DEST_DIR/jacococli.jar"
+echo "JaCoCo ${JACOCO_VERSION} jars are in $DEST_DIR"
diff --git a/private-cicd/scripts/generate-presubmit-report.py b/private-cicd/scripts/generate-presubmit-report.py
index 3067cfa26724..0799bb8ba41f 100644
--- a/private-cicd/scripts/generate-presubmit-report.py
+++ b/private-cicd/scripts/generate-presubmit-report.py
@@ -21,9 +21,11 @@
import argparse
import html
import json
+import os
import re
import shutil
-from datetime import datetime, timezone
+from datetime import datetime, timedelta, timezone
+from functools import lru_cache
from pathlib import Path
from urllib.parse import quote
@@ -71,6 +73,16 @@ def parse_time(value):
return datetime.fromisoformat(value.replace("Z", "+00:00"))
+IST = timezone(timedelta(hours=5, minutes=30), "IST")
+
+
+def format_ist(value):
+ """Show a parsed time in IST. Stored timestamps and durations stay UTC."""
+ if value is None:
+ return "—"
+ return value.astimezone(IST).strftime("%Y-%m-%d %H:%M:%S IST")
+
+
def format_duration(start, finish):
if not start or not finish:
return "—"
@@ -121,7 +133,120 @@ def html_document(title, body):
"""
-def log_to_html(source, destination):
+FAILURE_HEADER = re.compile(r"^(ERROR|FAIL):\s*(.*)$")
+DESCRIBED_TEST = re.compile(
+ r"^(.*?)\s*\.\.\.\s*=== TestName:\s+(\S+)\s+\|\s+Status\s+:\s+(\S+)")
+# The message ends at the quote that closes the errortext value: one followed by
+# the closing brace or by the next "key :" pair. A quote inside the message,
+# such as a quoted VM name, is kept. A message with no closing quote runs to the
+# end of the line.
+JOB_ERROR = re.compile(
+ r"errorcode\s*:\s*(\d+)\s*,\s*errortext\s*:\s*'"
+ r"(.*?)(?:'\s*(?:\}|,\s*\w+\s*:)|$)")
+MAX_FAILURE_LINES = 80
+MAX_ERROR_CHARS = 400
+
+
+@lru_cache(maxsize=None)
+def parse_suite_failures(results_txt):
+ """Return the ERROR and FAIL blocks of one Marvin suite results.txt.
+
+ Marvin prints each block with the test's docstring as its title, while the
+ summary lines pair the same docstring with the test name, so the two are
+ joined on that text. A block that cannot be joined (for example a
+ setUpClass error) keeps an empty name list and is still shown.
+ """
+ try:
+ text = ANSI_ESCAPE.sub(
+ "", results_txt.read_text(encoding="utf-8", errors="replace"))
+ except OSError:
+ return ()
+ lines = text.splitlines()
+ names = {}
+ for line in lines:
+ described = DESCRIBED_TEST.match(line)
+ if described:
+ names.setdefault(described.group(1).strip(), []).append(
+ described.group(2))
+ failures = []
+ index = 0
+ while index < len(lines):
+ header = FAILURE_HEADER.match(lines[index])
+ if not (header and index > 0 and lines[index - 1].startswith("=====")):
+ index += 1
+ continue
+ block = []
+ index += 1
+ while index < len(lines):
+ line = lines[index]
+ if (line.startswith("=====") or line.startswith("Ran ")
+ or ">> begin captured" in line):
+ break
+ block.append(line)
+ index += 1
+ while block and (not block[0].strip()
+ or set(block[0].strip()) == {"-"}):
+ block.pop(0)
+ while block and (not block[-1].strip()
+ or set(block[-1].strip()) == {"-"}):
+ block.pop()
+ description = header.group(2).strip()
+ error = block[-1].strip() if block else ""
+ # A failed CloudStack async job prints a long object of account and
+ # domain IDs before the useful text, so lead with the job's own message.
+ job = JOB_ERROR.search(error)
+ if job:
+ error = f"Job failed (code {job.group(1)}): {job.group(2)}"
+ if len(error) > MAX_ERROR_CHARS:
+ error = error[:MAX_ERROR_CHARS] + "…"
+ failures.append({
+ "kind": header.group(1),
+ "description": description,
+ "names": tuple(names.get(description, ())),
+ "lines": tuple(block[:MAX_FAILURE_LINES]),
+ "error": error,
+ })
+ return tuple(failures)
+
+
+def failure_anchor(test_name):
+ return f"fail-{slug(test_name)}"
+
+
+def render_failures(failures, related=()):
+ if not failures:
+ return ""
+ sections = []
+ if related:
+ links = " · ".join(
+ f'
{html.escape(label)}'
+ for label, path in related)
+ sections.append(
+ "
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(failure["kind"])}: {html.escape(title)}
'
+ f'
'
+ f'{html.escape(chr(10).join(failure["lines"]))}')
+ return "
Failure details
" + "".join(sections) + "
Suite log
"
+
+
+def failure_for_test(summary_path, test):
+ suite_dir = summary_path.parent / "suites" / test.get("tag", "")
+ for failure in parse_suite_failures(suite_dir / "results.txt"):
+ if test.get("name") in failure["names"]:
+ return failure
+ return None
+
+
+def log_to_html(source, destination, failures=(), related=()):
lines = source.read_text(encoding="utf-8", errors="replace").splitlines()
rendered = []
test_pattern = re.compile(r"TestName:\s+(\S+)\s+\|\s+Status\s+:\s+(\S+)")
@@ -154,7 +279,8 @@ def log_to_html(source, destination):
rendered.append(f"{anchor}{escaped}")
body = (
f"
{html.escape(source.name)}
"
- f'
'
+ "\n".join(rendered)
+ ""
@@ -175,6 +301,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 +315,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 +403,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(
"
"
f"| {html.escape(stage)} | "
@@ -339,13 +478,26 @@ def test_rows(tests, results, build_url):
for test in tests:
status = display_test_status(test.get("status"))
log_path = test_log_for_summary(test["_summary"], test)
+ failure = None
+ if status not in ("PASS", "SKIP"):
+ failure = failure_for_test(test["_summary"], test)
if log_path:
relative = log_path.relative_to(results.parent)
- link = artifact_url(
- build_url, relative, f"test-{slug(test.get('name', 'test'))}")
+ name = test.get("name", "test")
+ # The failure section sits at the top of the suite page, so a
+ # failed test links straight to its exception.
+ fragment = (
+ failure_anchor(name)
+ if failure and log_path.name == "stdout.log.html"
+ else f"test-{slug(name)}")
+ link = artifact_url(build_url, relative, fragment)
log_cell = f'HTML log'
else:
log_cell = "—"
+ error_cell = (
+ f''
+ f'{html.escape(failure["error"])}'
+ if failure and failure["error"] else "")
suite = str(test.get("label") or test.get("tag") or "")
rows.append(
"
"
@@ -353,6 +505,7 @@ def test_rows(tests, results, build_url):
f"| {html.escape(suite)} | "
f"{html.escape(str(test.get('name', '')))} | "
f'{html.escape(status)} | '
+ f"{error_cell} | "
f"{log_cell} | "
"
")
return "\n".join(rows)
@@ -374,6 +527,14 @@ def table(headers, rows):
f"
{cells}
{styled_rows}")
+def coverage_section(results):
+ """The ONTAP coverage summary written by coverage-report.sh, if any."""
+ try:
+ return (results / "coverage" / "summary.html").read_text(encoding="utf-8")
+ except OSError:
+ return ""
+
+
def main():
args = parse_args()
args.results.mkdir(parents=True, exist_ok=True)
@@ -415,9 +576,9 @@ def main():
| VM |
{html.escape(args.vm or 'not assigned')} |
| Started |
-{html.escape(args.started_at)} |
+
{html.escape(format_ist(started))} |
| Finished |
-{html.escape(args.finished_at)} |
+
{html.escape(format_ist(finished))} |
| Total duration |
{html.escape(format_duration(started, finished))} |
@@ -437,12 +598,13 @@ def main():
"
ONTAP tests
"
"
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
"
ONTAP tests
No Marvin test summary was available.
")
body = (
- header + failure + "
Pipeline stages
" +
- stage_table + test_section)
+ header + failure + coverage_section(args.results) +
+ "
Pipeline stages
" + stage_table + test_section)
document = html_document("CloudStack ONTAP presubmit report", body)
(args.results / "report.html").write_text(document, encoding="utf-8")
(args.results / "email-body.html").write_text(body, encoding="utf-8")
diff --git a/private-cicd/scripts/github-check-run.sh b/private-cicd/scripts/github-check-run.sh
index e00821cede8c..b5161b0fc4cb 100755
--- a/private-cicd/scripts/github-check-run.sh
+++ b/private-cicd/scripts/github-check-run.sh
@@ -22,6 +22,8 @@
# that run with a conclusion, commit-email prints the head commit author
# address used for presubmit mail, and diagnose probes the App's token type,
# repository access and Checks permissions to explain a refused check run.
+# coverage-comment creates or updates the single pull-request comment that
+# shows the ONTAP plugin coverage (needs Pull requests or Issues write).
# Dependencies:
# Commands: curl, python3 (stdlib), mktemp.
# GITHUB_ACCESS_TOKEN, GITHUB_REPOSITORY — required by every subcommand; the
@@ -29,7 +31,14 @@
# process list.
# GITHUB_HEAD_SHA — create, commit-email and diagnose. BUILD_URL — create and
# complete. GITHUB_CHECK_RUN_ID, GITHUB_CHECK_CONCLUSION — complete.
-# Optional: PR_ID and BUILD_TAG label the check run.
+# Optional: PR_ID and BUILD_TAG label the check run. For create, setting
+# GITHUB_CHECK_CONCLUSION publishes an already completed check run, which is
+# how a result is still reported when the run could not be created at the
+# start. GITHUB_CHECK_FAILED_STAGE and GITHUB_CHECK_REPORT_URL add the failed
+# stage and the report link to the check's summary, and GITHUB_CHECK_COVERAGE_FILE
+# appends the coverage summary (markdown) to it. coverage-comment needs PR_ID
+# and GITHUB_COVERAGE_FILE, the markdown file whose first line is the marker
+# that identifies the comment.
# Called by the Jenkinsfile. Does not call other private-cicd scripts.
set -euo pipefail
@@ -129,35 +138,81 @@ with open(sys.argv[1], encoding="utf-8") as handle:
PY
}
-create_check() {
- require_env GITHUB_ACCESS_TOKEN GITHUB_REPOSITORY GITHUB_HEAD_SHA BUILD_URL
- local payload response
- payload="$(mktemp)"
- response="$(mktemp)"
- trap "rm -f '$payload' '$response'" EXIT
-
- python3 - "$payload" <<'PY'
+# Builds the check-run payload for the "create" and "complete" subcommands from
+# the environment. GITHUB_CHECK_CONCLUSION makes it a completed run.
+write_payload() {
+ python3 - "$1" "$2" <<'PY'
import json
import os
import sys
+kind = sys.argv[2]
+env = os.environ.get
+conclusion = env("GITHUB_CHECK_CONCLUSION", "")
+build_url = env("BUILD_URL", "")
+sha = env("GITHUB_HEAD_SHA", "")
+pr = env("PR_ID", "unknown")
+
payload = {
"name": "cloudstack-ontap-presubmit",
- "head_sha": os.environ["GITHUB_HEAD_SHA"],
- "status": "in_progress",
- "details_url": os.environ["BUILD_URL"],
- "external_id": os.environ.get("BUILD_TAG", ""),
- "output": {
- "title": f"CloudStack ONTAP presubmit for PR-{os.environ.get('PR_ID', 'unknown')}",
+ "details_url": build_url,
+}
+if kind == "create":
+ payload["head_sha"] = sha
+ payload["external_id"] = env("BUILD_TAG", "")
+
+if kind == "create" and not conclusion:
+ payload["status"] = "in_progress"
+ payload["output"] = {
+ "title": f"CloudStack ONTAP presubmit for PR-{pr}",
"summary": (
- f"Jenkins build started for {os.environ['GITHUB_HEAD_SHA'][:12]}. "
- f"Details: {os.environ['BUILD_URL']}"
+ f"Jenkins build started for {sha[:12]}. "
+ f"Details: {build_url}"
),
- },
-}
+ }
+else:
+ verdict = {
+ "success": "passed",
+ "failure": "failed",
+ "cancelled": "was cancelled",
+ "timed_out": "timed out",
+ "neutral": "finished with a neutral result",
+ }.get(conclusion, conclusion)
+ lines = [f"The CloudStack ONTAP presubmit {verdict}."]
+ if env("GITHUB_CHECK_FAILED_STAGE"):
+ lines.append(f"Failed stage: {env('GITHUB_CHECK_FAILED_STAGE')}")
+ if sha:
+ lines.append(f"Commit: {sha[:12]}")
+ lines.append(f"Jenkins build and logs: {build_url}")
+ if env("GITHUB_CHECK_REPORT_URL"):
+ lines.append(f"Test report: {env('GITHUB_CHECK_REPORT_URL')}")
+ summary = "\n\n".join(lines)
+ coverage_file = env("GITHUB_CHECK_COVERAGE_FILE")
+ if coverage_file and os.path.isfile(coverage_file):
+ with open(coverage_file, encoding="utf-8") as handle:
+ coverage = handle.read().replace("", "").strip()
+ if coverage:
+ summary += "\n\n" + coverage
+ # GitHub rejects a summary above 65535 characters.
+ payload["status"] = "completed"
+ payload["conclusion"] = conclusion
+ payload["output"] = {
+ "title": f"CloudStack ONTAP presubmit {verdict}",
+ "summary": summary[:60000],
+ }
with open(sys.argv[1], "w", encoding="utf-8") as output:
json.dump(payload, output)
PY
+}
+
+create_check() {
+ require_env GITHUB_ACCESS_TOKEN GITHUB_REPOSITORY GITHUB_HEAD_SHA BUILD_URL
+ local payload response
+ payload="$(mktemp)"
+ response="$(mktemp)"
+ trap "rm -f '$payload' '$response'" EXIT
+
+ write_payload "$payload" create
github_api POST "/repos/${GITHUB_REPOSITORY}/check-runs" "$payload" > "$response" || {
local status=$?
@@ -178,41 +233,28 @@ PY
complete_check() {
require_env GITHUB_ACCESS_TOKEN GITHUB_REPOSITORY GITHUB_CHECK_RUN_ID \
GITHUB_CHECK_CONCLUSION BUILD_URL
- local payload response
+ local payload response attempt
payload="$(mktemp)"
response="$(mktemp)"
trap "rm -f '$payload' '$response'" EXIT
- python3 - "$payload" <<'PY'
-import json
-import os
-import sys
+ write_payload "$payload" complete
-conclusion = os.environ["GITHUB_CHECK_CONCLUSION"]
-payload = {
- "name": "cloudstack-ontap-presubmit",
- "status": "completed",
- "conclusion": conclusion,
- "details_url": os.environ["BUILD_URL"],
- "output": {
- "title": f"CloudStack ONTAP presubmit: {conclusion}",
- "summary": (
- f"Jenkins build completed with conclusion {conclusion}. "
- f"Details and logs: {os.environ['BUILD_URL']}"
- ),
- },
-}
-with open(sys.argv[1], "w", encoding="utf-8") as output:
- json.dump(payload, output)
-PY
-
- if ! github_api PATCH \
- "/repos/${GITHUB_REPOSITORY}/check-runs/${GITHUB_CHECK_RUN_ID}" \
- "$payload" > "$response"; then
- echo "GitHub refused to complete the check run:" >&2
+ # A check left in progress blocks a pull request that requires it, so a
+ # transient GitHub error gets a few retries before the failure is reported.
+ for attempt in 1 2 3; do
+ if github_api PATCH \
+ "/repos/${GITHUB_REPOSITORY}/check-runs/${GITHUB_CHECK_RUN_ID}" \
+ "$payload" > "$response"; then
+ return 0
+ fi
+ echo "GitHub refused to complete the check run (attempt ${attempt}/3):" >&2
print_github_error "$response"
- return 1
- fi
+ if (( attempt < 3 )); then
+ sleep "${CHECK_RETRY_SECONDS:-10}"
+ fi
+ done
+ return 1
}
# Read-only probes plus one write probe, ordered so that the first failure
@@ -357,6 +399,78 @@ print(payload.get("commit", {}).get("author", {}).get("email") or "")
PY
}
+# Creates the coverage comment on the pull request, or updates the one a
+# previous run left, so a PR carries a single coverage comment. The comment is
+# found by the marker on its first line, among comments written by a bot.
+coverage_comment() {
+ require_env GITHUB_ACCESS_TOKEN GITHUB_REPOSITORY PR_ID GITHUB_COVERAGE_FILE
+ local comments payload response comment_id page
+ comments="$(mktemp)"
+ payload="$(mktemp)"
+ response="$(mktemp)"
+ trap "rm -f '$comments' '$payload' '$response'" EXIT
+
+ python3 - "$GITHUB_COVERAGE_FILE" "$payload" <<'PY'
+import json
+import sys
+
+with open(sys.argv[1], encoding="utf-8") as source:
+ body = source.read()
+with open(sys.argv[2], "w", encoding="utf-8") as output:
+ json.dump({"body": body[:60000]}, output)
+PY
+
+ comment_id=""
+ for page in 1 2 3 4 5; do
+ github_api GET \
+ "/repos/${GITHUB_REPOSITORY}/issues/${PR_ID}/comments?per_page=100&page=${page}" \
+ > "$comments" || {
+ echo "GitHub refused to list the pull request comments:" >&2
+ print_github_error "$comments"
+ return 1
+ }
+ comment_id="$(python3 - "$comments" <<'PY'
+import json
+import sys
+
+marker = ""
+with open(sys.argv[1], encoding="utf-8") as handle:
+ items = json.load(handle)
+for item in items:
+ if (item.get("user") or {}).get("type") == "Bot" and marker in (item.get("body") or ""):
+ print(item["id"])
+ break
+PY
+)"
+ if [[ -n "$comment_id" ]]; then
+ break
+ fi
+ # A full page means there may be more comments to search.
+ if [[ "$(python3 -c 'import json,sys; print(len(json.load(open(sys.argv[1]))))' "$comments")" -lt 100 ]]; then
+ break
+ fi
+ done
+
+ if [[ -n "$comment_id" ]]; then
+ github_api PATCH "/repos/${GITHUB_REPOSITORY}/issues/comments/${comment_id}" \
+ "$payload" > "$response" || {
+ echo "GitHub refused to update the coverage comment:" >&2
+ print_github_error "$response"
+ return 1
+ }
+ echo "Updated coverage comment ${comment_id} on PR-${PR_ID}."
+ else
+ github_api POST "/repos/${GITHUB_REPOSITORY}/issues/${PR_ID}/comments" \
+ "$payload" > "$response" || {
+ echo "GitHub refused to create the coverage comment:" >&2
+ print_github_error "$response"
+ echo " The App needs Pull requests or Issues: Read and write." >&2
+ return 1
+ }
+ echo "Created coverage comment on PR-${PR_ID}."
+ fi
+}
+
case "${1:-}" in
create)
create_check
@@ -370,8 +484,11 @@ case "${1:-}" in
diagnose)
diagnose_app
;;
+ coverage-comment)
+ coverage_comment
+ ;;
*)
- echo "Usage: $0 {create|complete|commit-email|diagnose}" >&2
+ echo "Usage: $0 {create|complete|commit-email|diagnose|coverage-comment}" >&2
exit 2
;;
esac
diff --git a/private-cicd/scripts/marvin-run.sh b/private-cicd/scripts/marvin-run.sh
index 9e2996a9c74d..8c375ebdc5a5 100755
--- a/private-cicd/scripts/marvin-run.sh
+++ b/private-cicd/scripts/marvin-run.sh
@@ -58,6 +58,24 @@ run_suite() {
return "$status"
}
+# Functional-test coverage: dump (and reset) the JaCoCo data of the management
+# server after a suite, so a later restart cannot lose it. The data is only
+# collected when the agent was attached; a failed dump never fails the run.
+dump_coverage() {
+ local name="$1"
+ if [[ -z "${JACOCO_CLI:-}" || ! -f "$JACOCO_CLI" || -z "${COVERAGE_DIR:-}" ]]; then
+ return 0
+ fi
+ mkdir -p "$COVERAGE_DIR"
+ if java -jar "$JACOCO_CLI" dump --address 127.0.0.1 \
+ --port "${COVERAGE_PORT:-36320}" --reset \
+ --destfile "$COVERAGE_DIR/${name}.exec"; then
+ echo "Functional coverage for ${name}: $COVERAGE_DIR/${name}.exec"
+ else
+ echo "Could not dump functional coverage for ${name}" >&2
+ fi
+}
+
echo "==> Creating and validating the CloudStack zone"
if ! run_suite setup_zone; then
echo "Zone setup failed; protocol suites cannot run" >&2
@@ -67,9 +85,11 @@ fi
suite_status=0
echo "==> Running ONTAP iSCSI suites"
run_suite iscsi || suite_status=1
+dump_coverage iscsi
echo "==> Running ONTAP NFS3 suites"
run_suite nfs3 || suite_status=1
+dump_coverage nfs3
ontap_results=test/integration/plugins/ontap/results
if [[ -d "$ontap_results" ]]; then
diff --git a/private-cicd/scripts/mvn-full.sh b/private-cicd/scripts/mvn-full.sh
index 7aa9821a1d24..c9bb3add4715 100755
--- a/private-cicd/scripts/mvn-full.sh
+++ b/private-cicd/scripts/mvn-full.sh
@@ -25,7 +25,11 @@
# 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),
+# ONTAP_COVERAGE=true also writes the ONTAP plugin JaCoCo report to
+# $RESULT_DIR/coverage/unit,
+# 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 +47,8 @@ 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}"
+ONTAP_COVERAGE="${ONTAP_COVERAGE:-false}"
RESULT_DIR="${RESULT_DIR:-}"
mvn_args=(-B -P developer,systemvm)
@@ -59,6 +65,14 @@ fi
if [[ -n "$THREADS" ]]; then
mvn_args+=(-T"$THREADS")
fi
+if [[ "$RERUN_FAILING_TESTS" != "0" ]]; then
+ mvn_args+=("-Dsurefire.rerunFailingTestsCount=$RERUN_FAILING_TESTS")
+fi
+if [[ "$ONTAP_COVERAGE" == "true" ]]; then
+ # Activates the ontap-coverage profile of the ONTAP plugin module, so the
+ # same build that runs its unit tests also writes the JaCoCo report.
+ mvn_args+=(-Dontap.coverage=true)
+fi
echo "==> Full build in $SOURCE_DIR"
@@ -112,6 +126,18 @@ if [[ "$report_count" -gt 0 ]]; then
| tar --null -czf "$RESULT_DIR/surefire-reports.tar.gz" --files-from -
fi
+# JaCoCo report written by the ontap-coverage profile of the ONTAP plugin.
+ontap_coverage_dir="plugins/storage/volume/ontap/target/site/jacoco"
+if [[ "$ONTAP_COVERAGE" == "true" ]]; then
+ if [[ -f "$ontap_coverage_dir/jacoco.xml" ]]; then
+ mkdir -p "$RESULT_DIR/coverage"
+ cp -a "$ontap_coverage_dir" "$RESULT_DIR/coverage/unit"
+ echo "ONTAP unit-test coverage report: $RESULT_DIR/coverage/unit/jacoco.xml"
+ else
+ echo "ONTAP unit-test coverage was requested but $ontap_coverage_dir/jacoco.xml was not produced" >&2
+ fi
+fi
+
if [[ "$maven_status" -eq 0 ]]; then
result="SUCCESS"
else
diff --git a/private-cicd/scripts/ontap-coverage.py b/private-cicd/scripts/ontap-coverage.py
new file mode 100644
index 000000000000..1e711d3e52c6
--- /dev/null
+++ b/private-cicd/scripts/ontap-coverage.py
@@ -0,0 +1,279 @@
+#!/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.
+
+# Purpose:
+# Summarise ONTAP storage plugin coverage for one presubmit run. Reads the
+# JaCoCo XML reports of the unit tests (produced by the Maven build) and of
+# the functional Marvin tests (produced from the management server's JaCoCo
+# dump), and writes summary.json, summary.md and summary.html to --output-dir.
+# The combined figure is computed per source line, so it does not depend on
+# the two reports agreeing on class IDs: a line counts as covered when either
+# run executed it.
+# Dependencies:
+# Python 3 standard library only.
+# Called by: coverage-report.sh. Read by generate-presubmit-report.py and the
+# Jenkinsfile, which publish the summary in the mail, the report, the GitHub
+# Check and a pull-request comment.
+
+import argparse
+import html
+import json
+import re
+import sys
+import xml.etree.ElementTree as ET
+from pathlib import Path
+
+# Marks the pull-request comment so a later run updates it in place.
+COMMENT_MARKER = ""
+TITLE = "ONTAP storage plugin test coverage"
+COUNTERS = ("LINE", "BRANCH", "METHOD", "CLASS")
+DOCTYPE = re.compile(r"]*>")
+
+
+def parse_report(path):
+ """Return {"totals": {...}, "files": {...}} for one JaCoCo XML report.
+
+ files maps "package/File.java" to {"lines": {nr: (missed, covered)}}.
+ Returns None when the file is missing or is not a JaCoCo report.
+ """
+ try:
+ text = Path(path).read_text(encoding="utf-8", errors="replace")
+ root = ET.fromstring(DOCTYPE.sub("", text, count=1))
+ except (OSError, ET.ParseError):
+ return None
+ if root.tag != "report":
+ return None
+ totals = {}
+ for counter in root.findall("counter"):
+ kind = counter.get("type")
+ if kind in COUNTERS:
+ totals[kind] = (int(counter.get("missed", 0)), int(counter.get("covered", 0)))
+ files = {}
+ for package in root.findall("package"):
+ package_name = package.get("name", "")
+ for source in package.findall("sourcefile"):
+ lines = {}
+ for line in source.findall("line"):
+ lines[int(line.get("nr"))] = (
+ int(line.get("mi", 0)), int(line.get("ci", 0)))
+ files[f"{package_name}/{source.get('name')}"] = {"lines": lines}
+ return {"totals": totals, "files": files}
+
+
+def percent(covered, missed):
+ total = covered + missed
+ return round(100.0 * covered / total, 1) if total else None
+
+
+def counter_summary(totals, kind):
+ missed, covered = totals.get(kind, (0, 0))
+ return {"covered": covered, "missed": missed, "percent": percent(covered, missed)}
+
+
+def covered_line_numbers(file_entry):
+ return {nr for nr, (_, covered) in file_entry["lines"].items() if covered > 0}
+
+
+def combine(unit, functional):
+ """Line coverage of the union of both runs, plus a row for every file."""
+ names = set()
+ for report in (unit, functional):
+ if report:
+ names.update(report["files"])
+ rows = []
+ total_lines = total_covered = 0
+ for name in sorted(names):
+ line_sets = []
+ all_lines = set()
+ for report in (unit, functional):
+ entry = report["files"].get(name) if report else None
+ if entry:
+ all_lines.update(entry["lines"])
+ line_sets.append(covered_line_numbers(entry))
+ else:
+ line_sets.append(set())
+ if not all_lines:
+ continue
+ unit_covered, functional_covered = line_sets
+ combined_covered = unit_covered | functional_covered
+ rows.append({
+ "file": name,
+ "lines": len(all_lines),
+ "unit": percent(len(unit_covered), len(all_lines) - len(unit_covered))
+ if unit else None,
+ "functional": percent(len(functional_covered),
+ len(all_lines) - len(functional_covered))
+ if functional else None,
+ "combined": percent(len(combined_covered),
+ len(all_lines) - len(combined_covered)),
+ "missed": len(all_lines) - len(combined_covered),
+ })
+ total_lines += len(all_lines)
+ total_covered += len(combined_covered)
+ return {
+ "covered": total_covered,
+ "missed": total_lines - total_covered,
+ "percent": percent(total_covered, total_lines - total_covered),
+ }, rows
+
+
+def build_summary(unit, functional, top, notes):
+ combined, rows = combine(unit, functional)
+ worst = sorted(rows, key=lambda row: (-row["missed"], row["file"]))
+ return {
+ "title": TITLE,
+ "unit": ({kind.lower(): counter_summary(unit["totals"], kind) for kind in COUNTERS}
+ if unit else None),
+ "functional": ({kind.lower(): counter_summary(functional["totals"], kind)
+ for kind in COUNTERS} if functional else None),
+ "combined_line": combined if (unit or functional) else None,
+ "least_covered": [row for row in worst if row["missed"] > 0][:top],
+ "notes": notes,
+ }
+
+
+def fmt(value):
+ return "n/a" if value is None else f"{value:.1f}%"
+
+
+def cell(section, kind):
+ return fmt(section[kind]["percent"]) if section else "not available"
+
+
+def render_markdown(summary, links):
+ unit, functional = summary["unit"], summary["functional"]
+ combined = summary["combined_line"]
+ lines = [COMMENT_MARKER, f"### {summary['title']}", ""]
+ lines += [
+ "| | Line | Branch | Method |",
+ "| --- | --- | --- | --- |",
+ f"| Unit tests | {cell(unit, 'line')} | {cell(unit, 'branch')} | {cell(unit, 'method')} |",
+ f"| Functional (Marvin) tests | {cell(functional, 'line')} | "
+ f"{cell(functional, 'branch')} | {cell(functional, 'method')} |",
+ f"| **Combined** | **{fmt(combined['percent']) if combined else 'not available'}** | | |",
+ "",
+ ]
+ if summary["least_covered"]:
+ lines += ["Least covered files (by lines not covered by either run):", "",
+ "| File | Lines | Unit | Functional | Combined |",
+ "| --- | --- | --- | --- | --- |"]
+ for row in summary["least_covered"]:
+ name = row["file"].replace("org/apache/cloudstack/storage/", "")
+ lines.append(f"| `{name}` | {row['lines']} | {fmt(row['unit'])} | "
+ f"{fmt(row['functional'])} | {fmt(row['combined'])} |")
+ lines.append("")
+ for note in summary["notes"]:
+ lines += [f"> {note}", ""]
+ for label, url in links:
+ if url:
+ lines.append(f"[{label}]({url}) ")
+ return "\n".join(lines).rstrip() + "\n"
+
+
+def render_html(summary, links):
+ unit, functional = summary["unit"], summary["functional"]
+ combined = summary["combined_line"]
+ th = "text-align:left;background:#f2f4f7;border:1px solid #d0d5dd;padding:6px 10px"
+ td = "border:1px solid #d0d5dd;padding:6px 10px"
+
+ def row(label, section, bold=False):
+ cells = [cell(section, kind) for kind in ("line", "branch", "method")]
+ style = td + (";font-weight:bold" if bold else "")
+ return (f'
| {html.escape(label)} | ' +
+ "".join(f'{html.escape(value)} | ' for value in cells) +
+ "
")
+
+ parts = [f'
{html.escape(summary["title"])}
',
+ '
',
+ f' | Line | '
+ f'Branch | Method |
|---|
',
+ row("Unit tests", unit), row("Functional (Marvin) tests", functional),
+ f'| Combined | '
+ f''
+ f'{html.escape(fmt(combined["percent"]) if combined else "not available")} | '
+ f' | |
',
+ "
"]
+ if summary["least_covered"]:
+ parts += ['
Least covered files '
+ '(lines not covered by either run):
',
+ '
',
+ f'| File | Lines | '
+ f'Unit | Functional | '
+ f'Combined |
|---|
']
+ for item in summary["least_covered"]:
+ name = item["file"].replace("org/apache/cloudstack/storage/", "")
+ parts.append(
+ f'{html.escape(name)} | '
+ f'{item["lines"]} | '
+ f'{html.escape(fmt(item["unit"]))} | '
+ f'{html.escape(fmt(item["functional"]))} | '
+ f'{html.escape(fmt(item["combined"]))} |
')
+ parts.append("
")
+ for note in summary["notes"]:
+ parts.append(f'
{html.escape(note)}
')
+ link_html = " | ".join(
+ f'
{html.escape(label)}'
+ for label, url in links if url)
+ if link_html:
+ parts.append(f'
{link_html}
')
+ return "\n".join(parts) + "\n"
+
+
+def parse_args(argv):
+ parser = argparse.ArgumentParser(
+ description="Summarise ONTAP plugin unit and functional test coverage.")
+ parser.add_argument("--unit", help="JaCoCo XML report of the unit tests")
+ parser.add_argument("--functional", help="JaCoCo XML report of the functional tests")
+ parser.add_argument("--output-dir", required=True, type=Path)
+ parser.add_argument("--top", type=int, default=10,
+ help="number of least covered files to list")
+ parser.add_argument("--unit-link", default="", help="URL of the unit coverage report")
+ parser.add_argument("--functional-link", default="",
+ help="URL of the functional coverage report")
+ return parser.parse_args(argv)
+
+
+def main(argv=None):
+ args = parse_args(argv)
+ notes = []
+ unit = parse_report(args.unit) if args.unit else None
+ functional = parse_report(args.functional) if args.functional else None
+ if unit is None:
+ notes.append("Unit-test coverage was not produced by this build.")
+ if functional is None:
+ notes.append("Functional-test coverage was not produced by this run.")
+ summary = build_summary(unit, functional, args.top, notes)
+ links = [("Unit coverage report", args.unit_link),
+ ("Functional coverage report", args.functional_link)]
+ args.output_dir.mkdir(parents=True, exist_ok=True)
+ (args.output_dir / "summary.json").write_text(
+ json.dumps(summary, indent=2), encoding="utf-8")
+ (args.output_dir / "summary.md").write_text(
+ render_markdown(summary, links), encoding="utf-8")
+ (args.output_dir / "summary.html").write_text(
+ render_html(summary, links), encoding="utf-8")
+ combined = summary["combined_line"]
+ print("ONTAP coverage: unit line %s, functional line %s, combined line %s" % (
+ cell(summary["unit"], "line"), cell(summary["functional"], "line"),
+ fmt(combined["percent"]) if combined else "not available"))
+ return 0
+
+
+if __name__ == "__main__":
+ sys.exit(main())
diff --git a/private-cicd/scripts/resolve-pull-request.py b/private-cicd/scripts/resolve-pull-request.py
new file mode 100644
index 000000000000..9f2200916958
--- /dev/null
+++ b/private-cicd/scripts/resolve-pull-request.py
@@ -0,0 +1,139 @@
+#!/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_state": (pull or {}).get("state") 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_state"] != "open":
+ decision["reason"] = (
+ f"pull request is not open (state: {decision['pr_state'] or 'unknown'}).")
+ 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/run-phase2-remote.sh b/private-cicd/scripts/run-phase2-remote.sh
index 1669cec166ac..24ca017a9ab0 100755
--- a/private-cicd/scripts/run-phase2-remote.sh
+++ b/private-cicd/scripts/run-phase2-remote.sh
@@ -105,6 +105,17 @@ fi
mkdir -p "$assets_root/test/integration/plugins" "$assets_root/marvin"
cp -a "$SOURCE_DIR/test/integration/plugins/ontap" "$assets_root/test/integration/plugins/"
cp "${marvin_archives[0]}" "$assets_root/marvin/"
+
+# Functional-test coverage needs the JaCoCo agent and command line on the VM.
+# If they cannot be fetched the run goes on without coverage.
+functional_coverage="${FUNCTIONAL_COVERAGE:-false}"
+if [[ "$functional_coverage" == "true" ]]; then
+ if ! "$SCRIPT_DIR/fetch-jacoco.sh" "$assets_root/jacoco"; then
+ echo "Could not fetch the JaCoCo jars; functional coverage is off." >&2
+ rm -rf "$assets_root/jacoco"
+ functional_coverage=false
+ fi
+fi
tar -czf "$assets_archive" -C "$assets_root" .
remote="${VM_SSH_USER}@${VM_HOST}"
@@ -189,8 +200,8 @@ ssh "${ssh_options[@]}" "$remote" \
chmod +x '$remote_scripts/'*.sh"
printf -v remote_command \
- 'PHASE2_ROOT=%q BRIDGE=%q %q/run-phase2-vm.sh' \
- "$remote_root" "$BRIDGE" "$remote_scripts"
+ 'PHASE2_ROOT=%q BRIDGE=%q FUNCTIONAL_COVERAGE=%q %q/run-phase2-vm.sh' \
+ "$remote_root" "$BRIDGE" "$functional_coverage" "$remote_scripts"
set +e
ssh "${ssh_options[@]}" "$remote" "$remote_command"
diff --git a/private-cicd/scripts/run-phase2-vm.sh b/private-cicd/scripts/run-phase2-vm.sh
index e24624b59e67..47aee947e698 100755
--- a/private-cicd/scripts/run-phase2-vm.sh
+++ b/private-cicd/scripts/run-phase2-vm.sh
@@ -68,10 +68,22 @@ on_exit() {
}
trap on_exit EXIT
+# Functional-test coverage is on only when it was requested and the pipeline
+# shipped the JaCoCo jars. Empty values leave it off.
+coverage_agent=""
+coverage_cli=""
+if [[ "${FUNCTIONAL_COVERAGE:-false}" == "true" \
+ && -f "$source_dir/jacoco/jacocoagent.jar" \
+ && -f "$source_dir/jacoco/jacococli.jar" ]]; then
+ coverage_agent="$source_dir/jacoco/jacocoagent.jar"
+ coverage_cli="$source_dir/jacoco/jacococli.jar"
+fi
+
DEB_DIR="$deb_dir" \
SECRETS_FILE="$secrets_file" \
ONTAP_CONFIG="$ontap_config" \
BRIDGE="$BRIDGE" \
+JACOCO_AGENT_JAR="$coverage_agent" \
"$scripts_dir/configure-cloudstack-vm.sh" \
2>&1 | tee "$result_dir/configure-cloudstack.log"
@@ -84,4 +96,6 @@ BRIDGE="$BRIDGE" \
CLOUDSTACK_DIR="$source_dir" \
PHASE2_RESULT_DIR="$result_dir/marvin" \
MARVIN_ARCHIVE="$marvin_archive" \
+JACOCO_CLI="$coverage_cli" \
+COVERAGE_DIR="$result_dir/coverage" \
"$scripts_dir/marvin-run.sh"
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..b35eeac0dba4
--- /dev/null
+++ b/private-cicd/scripts/test_generate_presubmit_report.py
@@ -0,0 +1,218 @@
+#!/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_cloudstack_job_failure_keeps_quoted_names_in_the_message(self):
+ messages = [
+ "Unable to start VM 'i-2-5-VM', host unreachable",
+ "Volume doesn't exist",
+ "Failed to detach 'vol-1' from 'VM-2'",
+ ]
+ for message in messages:
+ line = ("Exception: Job failed: {accountid : 'a', jobresult : "
+ "{errorcode : 530, errortext : '%s'}, jobinstancetype : "
+ "'Volume'}" % message)
+ text = f"{SEP}\nERROR: d\n{DASH}\n{line}\n"
+ self.assertEqual(
+ self.parse(text)[0]["error"],
+ "Job failed (code 530): " + message)
+
+ def test_cloudstack_job_failure_without_closing_quote_shows_the_rest(self):
+ line = ("Exception: Job failed: {jobresult : {errorcode : 530, "
+ "errortext : 'cut off here")
+ text = f"{SEP}\nERROR: d\n{DASH}\n{line}\n"
+ self.assertEqual(
+ self.parse(text)[0]["error"], "Job failed (code 530): cut off here")
+
+ 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 CoverageSectionTest(unittest.TestCase):
+ def test_summary_fragment_is_included_when_present(self):
+ with tempfile.TemporaryDirectory() as directory:
+ results = Path(directory)
+ (results / "coverage").mkdir()
+ (results / "coverage" / "summary.html").write_text(
+ "
ONTAP storage plugin test coverage
", encoding="utf-8")
+
+ self.assertIn("ONTAP storage plugin test coverage",
+ report.coverage_section(results))
+
+ def test_no_summary_gives_an_empty_section(self):
+ with tempfile.TemporaryDirectory() as directory:
+ self.assertEqual("", report.coverage_section(Path(directory)))
+
+
+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_ontap_coverage.py b/private-cicd/scripts/test_ontap_coverage.py
new file mode 100644
index 000000000000..6b4e7d6fd6e2
--- /dev/null
+++ b/private-cicd/scripts/test_ontap_coverage.py
@@ -0,0 +1,155 @@
+#!/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 ontap-coverage.py."""
+
+import importlib.util
+import json
+import tempfile
+import unittest
+from pathlib import Path
+
+
+SCRIPT = Path(__file__).with_name("ontap-coverage.py")
+SPEC = importlib.util.spec_from_file_location("ontap_coverage", SCRIPT)
+MODULE = importlib.util.module_from_spec(SPEC)
+SPEC.loader.exec_module(MODULE)
+
+DOCTYPE = (''
+ '')
+
+
+def report(files, line_missed, line_covered):
+ """A JaCoCo report. files maps a name to {line number: covered?}."""
+ sources = ""
+ for name, lines in files.items():
+ package, _, source = name.rpartition("/")
+ entries = "".join(
+ f'
'
+ for nr, hit in lines.items())
+ sources += (f'
'
+ f"{entries}")
+ return (f'{DOCTYPE}
{sources}'
+ f''
+ ''
+ ''
+ '')
+
+
+A = "org/apache/cloudstack/storage/service/A.java"
+B = "org/apache/cloudstack/storage/driver/B.java"
+
+
+def write(directory, name, text):
+ path = Path(directory) / name
+ path.write_text(text, encoding="utf-8")
+ return path
+
+
+class CoverageTest(unittest.TestCase):
+ def setUp(self):
+ self.directory = tempfile.TemporaryDirectory()
+ self.addCleanup(self.directory.cleanup)
+
+ def reports(self):
+ unit = write(self.directory.name, "unit.xml",
+ report({A: {1: True, 2: True, 3: False, 4: False}, B: {1: False}}, 3, 2))
+ functional = write(self.directory.name, "functional.xml",
+ report({A: {1: False, 2: False, 3: True, 4: False}, B: {1: True}}, 3, 2))
+ return unit, functional
+
+ def test_parse_reads_totals_and_lines(self):
+ unit, _ = self.reports()
+ parsed = MODULE.parse_report(unit)
+
+ self.assertEqual((3, 2), parsed["totals"]["LINE"])
+ self.assertEqual((3, 1), parsed["totals"]["BRANCH"])
+ self.assertEqual((0, 1), parsed["files"][A]["lines"][1])
+ self.assertEqual((1, 0), parsed["files"][A]["lines"][3])
+
+ def test_combined_coverage_is_the_union_of_covered_lines(self):
+ unit, functional = self.reports()
+ summary = MODULE.build_summary(
+ MODULE.parse_report(unit), MODULE.parse_report(functional), 10, [])
+
+ # A: lines 1,2 (unit) + 3 (functional) of 4; B: 1 (functional) of 1.
+ self.assertEqual(4, summary["combined_line"]["covered"])
+ self.assertEqual(1, summary["combined_line"]["missed"])
+ self.assertEqual(80.0, summary["combined_line"]["percent"])
+ self.assertEqual(40.0, summary["unit"]["line"]["percent"])
+ rows = {row["file"]: row for row in summary["least_covered"]}
+ self.assertEqual(75.0, rows[A]["combined"])
+ self.assertEqual(50.0, rows[A]["unit"])
+ self.assertEqual(25.0, rows[A]["functional"])
+ self.assertNotIn(B, rows) # fully covered files are not listed
+
+ def test_one_missing_report_still_summarises_the_other(self):
+ unit, _ = self.reports()
+ summary = MODULE.build_summary(MODULE.parse_report(unit), None, 10, [])
+
+ self.assertIsNone(summary["functional"])
+ self.assertEqual(40.0, summary["combined_line"]["percent"])
+
+ def test_missing_or_broken_report_is_not_a_report(self):
+ broken = write(self.directory.name, "broken.xml", "
")
+ other = write(self.directory.name, "other.xml", "")
+
+ self.assertIsNone(MODULE.parse_report(Path(self.directory.name) / "none.xml"))
+ self.assertIsNone(MODULE.parse_report(broken))
+ self.assertIsNone(MODULE.parse_report(other))
+
+ def test_main_writes_json_markdown_and_html(self):
+ unit, functional = self.reports()
+ out = Path(self.directory.name) / "out"
+ code = MODULE.main(["--unit", str(unit), "--functional", str(functional),
+ "--output-dir", str(out), "--unit-link", "https://j/u"])
+
+ self.assertEqual(0, code)
+ summary = json.loads((out / "summary.json").read_text(encoding="utf-8"))
+ self.assertEqual(80.0, summary["combined_line"]["percent"])
+ markdown = (out / "summary.md").read_text(encoding="utf-8")
+ self.assertTrue(markdown.startswith(MODULE.COMMENT_MARKER))
+ self.assertIn("| **Combined** | **80.0%** |", markdown)
+ self.assertIn("[Unit coverage report](https://j/u)", markdown)
+ page = (out / "summary.html").read_text(encoding="utf-8")
+ self.assertIn("Combined", page)
+ self.assertIn('href="https://j/u"', page)
+
+ def test_main_with_no_reports_explains_why(self):
+ out = Path(self.directory.name) / "out"
+ MODULE.main(["--unit", "/nonexistent.xml", "--output-dir", str(out)])
+
+ summary = json.loads((out / "summary.json").read_text(encoding="utf-8"))
+ self.assertIsNone(summary["combined_line"])
+ self.assertEqual(2, len(summary["notes"]))
+ self.assertIn("not available", (out / "summary.md").read_text(encoding="utf-8"))
+
+ def test_file_names_are_escaped_in_html(self):
+ unit = write(self.directory.name, "unit.xml",
+ report({"p/<b>.java": {1: False}}, 1, 0)) # XML-escaped name
+ out = Path(self.directory.name) / "out"
+ MODULE.main(["--unit", str(unit), "--output-dir", str(out)])
+
+ page = (out / "summary.html").read_text(encoding="utf-8")
+ self.assertNotIn(".java", page)
+ self.assertIn("<b>.java", page)
+
+
+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..71c9320fa876
--- /dev/null
+++ b/private-cicd/scripts/test_resolve_pull_request.py
@@ -0,0 +1,99 @@
+#!/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,
+ state="open"):
+ return {
+ "number": int(number),
+ "title": "Test",
+ "html_url": f"https://github.com/NetApp/cloudstack/pull/{number}",
+ "state": state,
+ "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_rejects_closed_or_merged_pr(self):
+ # A merged PR is also reported by GitHub as state "closed".
+ for state in ("closed", "", None):
+ decision = MODULE.assess(
+ pull(state=state), "98", SHA, "main", "NetApp/cloudstack")
+
+ self.assertFalse(decision["eligible"], state)
+ self.assertIn("not open", decision["reason"])
+
+ 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.