From cff8b7f31155dfdf9d09f346a53bf6125054eeeb Mon Sep 17 00:00:00 2001 From: Joshua Hoblitt Date: Wed, 10 Jun 2026 21:52:07 -0700 Subject: [PATCH] ci: reduce CephUpgradeSuite flakes The CephUpgradeSuite workflow failed ~31% of PR runs vs the ~19% broken-PR baseline of the other integration suites. Classifying the failing step of every failed run from the last ~400 PR runs and the logs of every failure on non-dependabot branches shows the excess comes from a handful of too-tight waits, unretried network fetches, and environment races rather than from the upgrade logic itself: - The biggest class (21 of 44 genuine test-phase failure runs): "giving up waiting for deployment(s) with label app=rook-ceph-{rgw,osd},ceph-version!=..." during the Squid->Tentacle upgrade. The operator updates daemons sequentially (mons, mgr, osds with PG health gates between them, mds, rgw), so each later daemon's fixed 275s wait also absorbs the time spent on every daemon before it; rgw, last in the sequence, failed most often. The mon wait was already extended for slow image pulls in 98bd03a42; extend the same retry count to all the daemon waits. - ~7 runs: "snapshot controller is not ready" in the Helm upgrade path. WaitForSnapshotController(30) allows only 150s, and the logs show the deployment still converging (readyreplicas 1 < replicas 2) on the final poll. Raise to 90 retries. - InstallOrUpgradeHelmRepoChart ran helm with no retry; an observed failure fetched the ceph-csi-drivers chart tarball from GitHub release assets and got a 504. Retry up to 5 times, as InstallLocalHelmChart already does. - Raise the go test timeouts (2400s->3600s rook, 1800s->2700s helm). Failed runs ended in "panic: test timed out" during teardown, which aborts cleanup and log collection; the longer daemon waits above also need the headroom. - The "setup cluster resources" composite action (shared by all suites) accounted for half of all failed jobs. The one steady class there (8 distinct runs across 7 different days): minikube exits with K8S_FAIL_CONNECT (code 40) when the requested kubernetes version is missing from its bundled version list, because it then validates the version with an anonymous GitHub API request (GITHUB_TOKEN is not honored on that code path), and anonymous requests from shared runner IPs are regularly rate-limited. minikube 1.38.x predates v1.35.5, so only the v1.35.5 jobs hit this class. Pass --force to minikube start to skip that check: the kubernetes versions used in CI are pinned constants already validated by the PRs that bump them, and an invalid version would still fail fast at the kubeadm download. Also seen twice: dpkg failing on a corrupt cri-dockerd .deb because curl ran without --fail and saved an HTTP error page as the package; add --fail and retries. - On runners without the /mnt resource disk, the fallback OSD disk is an iSCSI (LIO) device and use_local_disk_for_integration_test returned early on those runners, skipping the udev nowatch workaround for the device re-probe storms of rook issue 8975. A burn-in failure on this PR captured the consequence with full kernel forensics: ~100 udev change events on the OSD disk, and ceph-volume activate wedged in uninterruptible sleep on the block device lock (blkdev_llseek -> rwsem_down_write_slowpath) for 18+ minutes during the ceph version upgrade while the cluster's only OSD stayed down. Install the nowatch rule before the early return so it applies to every runner type, and before the disk is first written rather than after. - One burn-in round failed before the upgrade even began: the pre-upgrade PVC create on the Helm path expired WaitUntilPVCIsBound (RetryLoop, 275s) while the CSI provisioner pods were still starting. Set RETRY_MAX=110 for this workflow to double the framework's base wait budget, instead of extending individual waits one flake at a time. - minikube also exits with GUEST_START (code 80) when its internal 6 minute node-ready wait expires on slow runners (5 runs in the dataset, two more observed while burning in this PR, one of them on another suite). Pass --wait-timeout=15m to minikube start. Not addressed (small or episodic classes): OSDs never coming up on initial deploy (3 runs, possibly the same udev/iSCSI wedge), the filesystem not becoming active (2 runs), and several day-clustered minikube incidents (exit codes 65/67/90). Signed-off-by: Joshua Hoblitt --- .../action.yaml | 14 +++++++++- .../integration-test-upgrade-suite.yaml | 8 ++++-- .../installer/ceph_helm_installer.go | 4 ++- tests/framework/utils/helm_helper.go | 19 ++++++++++--- tests/integration/ceph_upgrade_test.go | 27 +++++++++++-------- tests/scripts/github-action-helper.sh | 12 +++++++-- 6 files changed, 64 insertions(+), 20 deletions(-) diff --git a/.github/workflows/integration-test-setup-cluster-resources/action.yaml b/.github/workflows/integration-test-setup-cluster-resources/action.yaml index 33ea03550..1fc72a0af 100644 --- a/.github/workflows/integration-test-setup-cluster-resources/action.yaml +++ b/.github/workflows/integration-test-setup-cluster-resources/action.yaml @@ -26,7 +26,9 @@ runs: shell: bash --noprofile --norc -eo pipefail -x {0} run: | ARCH=$(go env GOARCH) - curl -LO https://github.com/Mirantis/cri-dockerd/releases/download/v0.3.21/cri-dockerd_0.3.21.3-0.ubuntu-focal_${ARCH}.deb + # --fail so an HTTP error page is not saved as the .deb (dpkg then fails on the + # corrupt archive), and retry transient download errors + curl -fLO --retry 5 --retry-all-errors --retry-delay 10 https://github.com/Mirantis/cri-dockerd/releases/download/v0.3.21/cri-dockerd_0.3.21.3-0.ubuntu-focal_${ARCH}.deb sudo dpkg -i cri-dockerd_0.3.21.3-0.ubuntu-focal_${ARCH}.deb rm -f cri-dockerd_0.3.21.3-0.ubuntu-focal_${ARCH}.deb @@ -43,6 +45,16 @@ runs: cpus: 2 addons: ingress cni: calico + # --force: when the requested kubernetes version is missing from minikube's compiled-in + # version list (any version released after the pinned minikube), 'minikube start' + # verifies the version with an anonymous GitHub API request, which is regularly + # rate-limited on shared runner IPs and fails the job (K8S_FAIL_CONNECT, exit code 40). + # The versions used here are pinned constants already validated by the PRs that bump + # them, so that check adds nothing in CI; an invalid version would still fail fast at + # the kubeadm/kubelet download. + # --wait-timeout: minikube's default 6m node-ready wait expires on slow runners + # (GUEST_START, exit code 80). + start-args: --force --wait-timeout=15m - name: install deps shell: bash --noprofile --norc -eo pipefail -x {0} diff --git a/.github/workflows/integration-test-upgrade-suite.yaml b/.github/workflows/integration-test-upgrade-suite.yaml index 695f1b531..e11fe43f7 100644 --- a/.github/workflows/integration-test-upgrade-suite.yaml +++ b/.github/workflows/integration-test-upgrade-suite.yaml @@ -26,6 +26,10 @@ env: # the ceph_preview build tag. Can be removed when go-ceph promotes # the account API out of preview. GOFLAGS: "-tags=ceph_preview" + # Double the test framework's default wait budget (55 retries x 5s = 275s); + # CI runners regularly need longer, e.g. for PVC binding while the CSI + # pods are still starting during the initial cluster deploy. + RETRY_MAX: "110" jobs: TestCephUpgradeSuite: @@ -56,7 +60,7 @@ jobs: run: | tests/scripts/github-action-helper.sh collect_udev_logs_in_background export DEVICE_FILTER=$(tests/scripts/github-action-helper.sh find_extra_block_dev) - go test -v -timeout 2400s -failfast -run CephUpgradeSuite/TestUpgradeRook github.com/rook/rook/tests/integration + go test -v -timeout 3600s -failfast -run CephUpgradeSuite/TestUpgradeRook github.com/rook/rook/tests/integration - name: collect common logs if: always() @@ -112,7 +116,7 @@ jobs: tests/scripts/github-action-helper.sh collect_udev_logs_in_background export DEVICE_FILTER=$(tests/scripts/github-action-helper.sh find_extra_block_dev) - go test -v -timeout 1800s -failfast -run CephUpgradeSuite/TestUpgradeHelm github.com/rook/rook/tests/integration + go test -v -timeout 2700s -failfast -run CephUpgradeSuite/TestUpgradeHelm github.com/rook/rook/tests/integration - name: collect common logs if: always() diff --git a/tests/framework/installer/ceph_helm_installer.go b/tests/framework/installer/ceph_helm_installer.go index 6e0de9194..50f312054 100644 --- a/tests/framework/installer/ceph_helm_installer.go +++ b/tests/framework/installer/ceph_helm_installer.go @@ -199,7 +199,9 @@ func (h *CephInstaller) InstallCephCsiDriversViaHelm() error { if err := h.k8shelper.CreateSnapshotController("create"); err != nil { return errors.Wrap(err, "failed to install snapshot controller") } - if err := h.k8shelper.WaitForSnapshotController(30); err != nil { + // the snapshot controller is installed while the cluster is busy rolling daemons during + // upgrade tests, and its image pull plus rollout regularly exceeds 150s in CI + if err := h.k8shelper.WaitForSnapshotController(90); err != nil { return errors.Wrap(err, "snapshot controller is not ready") } op := h.settings.OperatorNamespace diff --git a/tests/framework/utils/helm_helper.go b/tests/framework/utils/helm_helper.go index 968e863c5..e223d7c27 100644 --- a/tests/framework/utils/helm_helper.go +++ b/tests/framework/utils/helm_helper.go @@ -151,10 +151,23 @@ func (h *HelmHelper) InstallOrUpgradeHelmRepoChart(namespace, release, repoURL, "--version", version, "--namespace", namespace, } - if err := h.installChart(cmdArgs, values); err != nil { - return fmt.Errorf("helm upgrade --install %q in namespace %q: %w", release, namespace, err) + // retry: helm fetches the chart tarball from the repo's download URL (e.g. GitHub release + // assets), and transient fetch errors (504s) have failed CI runs + var lastErr error + maxRetries := 5 + for i := 0; i < maxRetries; i++ { + err := h.installChart(cmdArgs, values) + if err != nil { + lastErr = fmt.Errorf("helm upgrade --install %q in namespace %q: %w", release, namespace, err) + logger.Error(lastErr) + time.Sleep(10 * time.Second) + continue + } + logger.Infof("helm chart %q installed successfully after %d attempt(s)", release, i+1) + return nil } - return nil + logger.Errorf("failed to install helm chart %s after %d attempts", release, maxRetries) + return lastErr } func (h *HelmHelper) UninstallHelmReleaseIfExists(namespace, release string) error { diff --git a/tests/integration/ceph_upgrade_test.go b/tests/integration/ceph_upgrade_test.go index 0ea167847..ed284f488 100644 --- a/tests/integration/ceph_upgrade_test.go +++ b/tests/integration/ceph_upgrade_test.go @@ -363,26 +363,31 @@ func (s *UpgradeSuite) verifyRookUpgrade(numOSDs int) { } func (s *UpgradeSuite) waitForUpgradedDaemons(previousVersion, versionLabel string, numOSDs int, waitForMDS bool) { + // extend the timeout for all the daemon checks: pulling ceph-ci images can take 6+ minutes, + // and the operator updates the daemons sequentially (mons, mgr, osds, mds, rgw, with PG + // health gates between OSDs), so the wait for each later daemon also absorbs the time spent + // on every daemon before it. The rgw wait, being last, timed out most often in CI. + upgradeRetries := utils.RetryLoop + 120 + // wait for the mon(s) to be updated monsNotOldVersion := fmt.Sprintf("app=rook-ceph-mon,%s!=%s", versionLabel, previousVersion) - // extend the timeout for this check because it can take 6+ minutes to pull ceph-ci images - err := s.k8sh.WaitForDeploymentCountWithRetries(monsNotOldVersion, s.namespace, s.settings.Mons, utils.RetryLoop+120) + err := s.k8sh.WaitForDeploymentCountWithRetries(monsNotOldVersion, s.namespace, s.settings.Mons, upgradeRetries) require.NoError(s.T(), err, "mon(s) didn't update") - err = s.k8sh.WaitForLabeledDeploymentsToBeReady(monsNotOldVersion, s.namespace) + err = s.k8sh.WaitForLabeledDeploymentsToBeReadyWithRetries(monsNotOldVersion, s.namespace, upgradeRetries) require.NoError(s.T(), err) // wait for the mgr to be updated mgrNotOldVersion := fmt.Sprintf("app=rook-ceph-mgr,%s!=%s", versionLabel, previousVersion) - err = s.k8sh.WaitForDeploymentCount(mgrNotOldVersion, s.namespace, 1) + err = s.k8sh.WaitForDeploymentCountWithRetries(mgrNotOldVersion, s.namespace, 1, upgradeRetries) require.NoError(s.T(), err, "mgr didn't update") - err = s.k8sh.WaitForLabeledDeploymentsToBeReady(mgrNotOldVersion, s.namespace) + err = s.k8sh.WaitForLabeledDeploymentsToBeReadyWithRetries(mgrNotOldVersion, s.namespace, upgradeRetries) require.NoError(s.T(), err) // wait for the osd pods to be updated osdsNotOldVersion := fmt.Sprintf("app=rook-ceph-osd,%s!=%s", versionLabel, previousVersion) - err = s.k8sh.WaitForDeploymentCount(osdsNotOldVersion, s.namespace, numOSDs) + err = s.k8sh.WaitForDeploymentCountWithRetries(osdsNotOldVersion, s.namespace, numOSDs, upgradeRetries) require.NoError(s.T(), err, "osd(s) didn't update") - err = s.k8sh.WaitForLabeledDeploymentsToBeReady(osdsNotOldVersion, s.namespace) + err = s.k8sh.WaitForLabeledDeploymentsToBeReadyWithRetries(osdsNotOldVersion, s.namespace, upgradeRetries) require.NoError(s.T(), err) // wait for the mds pods to be updated @@ -390,16 +395,16 @@ func (s *UpgradeSuite) waitForUpgradedDaemons(previousVersion, versionLabel stri // the check for MDS upgrade in case it's just a ceph upgrade (no operator restart) if waitForMDS { mdsesNotOldVersion := fmt.Sprintf("app=rook-ceph-mds,%s!=%s", versionLabel, previousVersion) - err = s.k8sh.WaitForDeploymentCount(mdsesNotOldVersion, s.namespace, 2 /* always expect 2 mdses */) + err = s.k8sh.WaitForDeploymentCountWithRetries(mdsesNotOldVersion, s.namespace, 2 /* always expect 2 mdses */, upgradeRetries) require.NoError(s.T(), err) - err = s.k8sh.WaitForLabeledDeploymentsToBeReady(mdsesNotOldVersion, s.namespace) + err = s.k8sh.WaitForLabeledDeploymentsToBeReadyWithRetries(mdsesNotOldVersion, s.namespace, upgradeRetries) require.NoError(s.T(), err) } rgwsNotOldVersion := fmt.Sprintf("app=rook-ceph-rgw,%s!=%s", versionLabel, previousVersion) - err = s.k8sh.WaitForDeploymentCount(rgwsNotOldVersion, s.namespace, 1 /* always expect 1 rgw */) + err = s.k8sh.WaitForDeploymentCountWithRetries(rgwsNotOldVersion, s.namespace, 1 /* always expect 1 rgw */, upgradeRetries) require.NoError(s.T(), err) - err = s.k8sh.WaitForLabeledDeploymentsToBeReady(rgwsNotOldVersion, s.namespace) + err = s.k8sh.WaitForLabeledDeploymentsToBeReadyWithRetries(rgwsNotOldVersion, s.namespace, upgradeRetries) require.NoError(s.T(), err) // Give a few seconds for the daemons to settle down after the upgrade diff --git a/tests/scripts/github-action-helper.sh b/tests/scripts/github-action-helper.sh index 5ee3fe905..ae518310e 100755 --- a/tests/scripts/github-action-helper.sh +++ b/tests/scripts/github-action-helper.sh @@ -152,6 +152,16 @@ function use_local_disk_for_integration_test() { : "$(block_dev)" sudo lsblk + # Stop udev from watching the OSD disk, on every kind of runner. Without this, every + # close-after-write on the device retriggers a udev probe of the device, and the resulting + # re-probe storm during OSD restarts has been seen to wedge ceph-volume activate in + # uninterruptible sleep on the block device lock on runners using the iSCSI fallback disk. + # See https://github.com/rook/rook/issues/8975 and https://access.redhat.com/solutions/1465913 + echo "ACTION==\"add|change\", KERNEL==\"$(block_dev_basename)\", OPTIONS:=\"nowatch\"" | sudo tee -a /etc/udev/rules.d/99-z-rook-nowatch.rules + sudo udevadm control --reload-rules || true + sudo udevadm trigger || true + time sudo udevadm settle || true + unset pipefail mountpoint -q /mnt || return 0 set pipefail @@ -166,8 +176,6 @@ function use_local_disk_for_integration_test() { # we have observed that some runners keep detaching/re-attaching the additional disk overriding the permissions to the default root:disk # for more details see: https://github.com/rook/rook/issues/7405 echo "SUBSYSTEM==\"block\", ATTR{size}==\"29356032\", ACTION==\"add\", RUN+=\"/bin/chown 167:167 $PARTITION\"" | sudo tee -a /etc/udev/rules.d/01-rook.rules - # for below, see: https://access.redhat.com/solutions/1465913 - echo "ACTION==\"add|change\", KERNEL==\"$(block_dev_basename)\", OPTIONS:=\"nowatch\"" | sudo tee -a /etc/udev/rules.d/99-z-rook-nowatch.rules # The partition is still getting reloaded occasionally during operation. See https://github.com/rook/rook/issues/8975 # Try issuing some disk-inspection commands to jog the system so it won't reload the partitions # during OSD provisioning.