Skip to content

Fix cross-cluster backup/restore, upgrade tests, resource preservation, CI pipelines, and Slack thread notifications - #1240

Open
RaunakJalan wants to merge 38 commits into
mainfrom
fix/new-nodes-quoting
Open

Fix cross-cluster backup/restore, upgrade tests, resource preservation, CI pipelines, and Slack thread notifications#1240
RaunakJalan wants to merge 38 commits into
mainfrom
fix/new-nodes-quoting

Conversation

@RaunakJalan

@RaunakJalan RaunakJalan commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Fix cross-cluster backup restore end-to-end flow in Docker mode (CLI commands, cluster-id flags, bootstrap validation, restore polling)
  • Fix K8s upgrade test reliability (R25 bootstrap params, FIO Multi-Attach, CR verification, data integrity via FIO verify_only)
  • Standardize CI pipelines: GitHub summary, Slack notifications, and Graylog log collection across all 21 workflows
  • Migrate Slack notifications from webhooks to Bot API enabling thread replies for log-collection-complete messages
  • Fix teardown, testname matching, FIO validation, and --new_nodes argument quoting

Cross-cluster backup restore

  • Use CLI commands (sbctl) instead of API calls for multi-cluster Docker mode
  • Add --cluster-id to all backup CLI commands (export, import, list, restore, source-switch)
  • Use distinct pool names per cluster to avoid ambiguity
  • Fix volume connect to use lvol ID instead of name
  • Validate C2 bootstrap via actual state (sn list, cluster ACTIVE) instead of stderr parsing
  • Fail fast when bootstrap steps (add-node, activate, pool) fail
  • Poll restore task to completion before attempting volume connect
  • Collect SPDK and management logs from C2 nodes

K8s upgrade tests

  • Fix R25 bootstrap: set lvol port offset, pass ndcs/npcs/partitions/jm_count
  • Add CR verification to Step 7 of migration
  • Fix Multi-Attach error on verify FIO by removing avoid_node scheduling
  • Make pre-upgrade FIO mandatory with 600s timeout so all lvols have data before snapshots/clones
  • Switch post-upgrade data integrity to FIO verify_only mode (checksum pod methods retained but unused)
  • Pass node_id=None to validate_migration_for_node (cluster-wide tasks have no node_id)
  • Fix validate_fio_job failing instantly on PodInitializing

CI pipelines

  • Standardize GitHub summary, Slack notifications, and Graylog log collection across all 21 workflows
  • Migrate Slack from webhooks to Bot API (chat.postMessage) with webhook fallback
  • Log-collection-complete notifications now post as thread replies under the test summary
  • Fix teardown deletion order and docker logging target node
  • Use exact class name matching for --testname in all test runners
  • Quote NEW_NODE_IPS to fix argument splitting for multi-IP values

Unquoted ${NEW_NODE_IPS} caused word splitting so multi-IP values
like "192.168.10.203 192.168.10.204" were passed as separate args,
making e2e.py fail with "unrecognized arguments".
'cluster create' reinitializes docker swarm and prompts to destroy
the existing cluster. 'cluster add' creates a new cluster on the
existing management layer. The --ifname flag is not supported by
'cluster add'.
…lure is set

Previously, skip_k8s_cleanup=True only skipped K8s resource cleanup (PVCs/pods)
but delete_lvols defaulted to True, so backend lvols were still deleted via
sbctl — making the preservation useless for debugging. Now passes
delete_lvols=not _skip_k8s to match the pattern used in e2e.py.
The 'sbctl backup restore' command requires --cluster-id as a
mandatory argument. Pass the Cluster-2 ID explicitly.
Multi-cluster setups require explicit cluster IDs on backup commands:
- backup export: add --cluster <src_cluster_id>
- backup import: add --cluster-id <target_cluster_id>
- backup list: add --cluster <target_cluster_id>
- backup restore: already had --cluster-id (previous commit)

Matches the documented cross-cluster restore CLI steps.
Override _ensure_pool_and_sc, _create_lvol, and _connect_and_mount
in TestBackupCrossClusterRestore to use sbctl CLI commands (pool add,
lvol add, volume connect) instead of sbcli_utils API calls in Docker
mode. The multi-cluster API issues will be addressed in v2.

K8s mode continues to use base class methods (CRD-based).
Two clusters on the same mgmt node with the same pool name causes
ambiguity. Use 'bck_pool_c1' for Cluster-1 and 'bck_pool_c2' for
Cluster-2. Also pass --cluster-id on 'lvol add' to ensure the lvol
is created on the correct cluster.
'sbctl volume connect' takes the lvol UUID, not the name.
The CLI source-switch ignores the CLUSTER_ID env var and falls back to
clusters[0] (C1) when --cluster-id is not passed. This caused C1's
source to be switched instead of C2's, leaving C2 on local source and
failing the restore with "active S3 source mismatch".
Previously all bootstrap commands were fire-and-forget: exec_command
return values were ignored, so C2 could end up with 0 storage nodes
and UNREADY status while the test continued. Now each critical step
checks for errors and raises RuntimeError immediately.
…rror'

stderr contains sbcli log lines (INFO:, WARNING:, ERROR: prefixes)
that are not actual command failures. Checking 'error' in stderr
caused false positives when add-node succeeded but had log output.
Now only check stdout for 'Error:' (actual CLI error format) and
rely on post-step validations (cluster ACTIVE, sn list).
Instead of checking stderr/stdout for error strings (which false-
positives on log lines), validate actual state:
- After add-node: sn list --cluster-id shows expected node count
- After activate: cluster list row for C2 shows ACTIVE
- After activate: sn list --cluster-id checks node health
volume connect requires the lvol UUID, not the display name. Extract
the ID from lvol list output when the restored lvol appears, then
pass it to volume connect.
Previously the cross-cluster restore code connected as soon as the
lvol name appeared in lvol list, even while status was 'restoring'.
Now mirrors the base class pattern: poll cluster list-tasks until
the s3_backup_restore task reaches 'done', wait 60s for stabilisation,
then verify lvol is online and extract its ID for volume connect.
The backup's node_id belongs to C1, so without an explicit --node
flag the restore creates the lvol on C1's nodes instead of C2's.
Query sn list --cluster-id to find an online C2 node and pass it
via the existing --node CLI flag.
The polling loop only checked for 'done' and silently proceeded on
timeout. Now detects max retries exhausted (e.g. 10/10 suspended)
and raises AssertionError immediately. Also aborts if the lvol
reaches restore_failed status instead of proceeding to mount.
C2 storage nodes (203, 204) are bootstrapped during the test and were
not included in log collection. Start docker logging after C2 bootstrap
and collect final logs in teardown before cluster destruction.
balancing_on_restart master tasks have no node_id set (they are
cluster-wide), so filtering by a specific node_id always yields zero
matches.  Every other caller in the codebase already passes None;
align the upgrade tests to do the same.
Pre-upgrade FIO was wrapped in try/except as non-fatal, allowing the
test to proceed even when some lvols had 0B data written. This made
snapshot/clone validation unreliable. Remove the try/except and increase
timeout from 300s to 600s so all PVCs must complete FIO before the
upgrade continues.
- Delete clones before snapshots before lvols in both setup_base
  and teardown (snapshot delete fails while clones still exist)
- Fix restart_docker_logging to SSH to each node's own IP instead
  of the currently-upgraded node for all containers
Substring matching caused TestMajorUpgrade to also run
TestMajorUpgradeSingleNode and TestMajorUpgradeDualNode.
Changed to exact match in e2e.py, upgrade_e2e.py, stress.py,
and load.py.
The pre-check in validate_fio_job treated PodInitializing and
ContainerCreating as terminal errors and raised immediately. These
are normal transient states — the FIO job has an init container
(fio-warmup) that runs before the main benchmark container, so the
pod is legitimately in PodInitializing during warmup. Only fail fast
on actual unrecoverable states (ErrImagePull, ImagePullBackOff).
…s/jm_count

R25 SPDK has port conflicts on the default lvol NVMe-oF port range,
causing bind() failures (EADDRINUSE) for user lvol subsystems. Shift
the port range by setting ports.lvolNvmfPortStart=9110 in the sbcli
helm chart.

Also pass cluster_params (ndcs, npcs, partitions, jm_count) through
to cluster create and spdk-csi helm install instead of hardcoding
defaults.
After applying custom resources (StorageCluster, StoragePool,
StorageNodeSet), verify that the critical CRs were actually created.
Previously, kubectl apply errors went to stderr which was silently
ignored, causing confusing failures much later at Step 10 when node
restart could not find the StorageNodeSet CR.
@RaunakJalan
RaunakJalan force-pushed the fix/new-nodes-quoting branch 3 times, most recently from 198010d to 9b4940e Compare August 13, 2026 13:39
Post-upgrade verify FIO pods were scheduled to a different node than
where the PVC was previously attached. The CSI VolumeAttachment detach
is async, so the PVC could still be attached to the old node when the
verify pod tries to mount it on the new node, causing Multi-Attach
errors. Remove avoid_node for verify-only FIO to let K8s schedule
pods on the node where the volume is already attached.
…cross all pipelines

- Upgrade 4 pipelines (k8s-native-upgrade, e2e-docker, k8s-e2e-ha,
  k8s-native-cross-cluster-restore) from minimal summaries to rich format
  with test results tables, per-test details, failure reasons, and run artifacts
- Replace curl/slackapi one-liner Slack notifications with Python-based rich
  messages matching the upgrade-bootstrap reference format
- Add graylog/opensearch log collection to k8s-e2e-ha and
  k8s-native-cross-cluster-restore (were missing it entirely)
- Add post-log-collection Slack follow-up notification to all 16 pipelines
  so devs know when logs are available
Replace Incoming Webhook with Slack Bot API (chat.postMessage) across all
21 CI workflow files. The initial test summary captures a message timestamp
(ts) via GITHUB_ENV, and the log-collection-complete notification posts as
a threaded reply using thread_ts. Falls back to webhook when SLACK_BOT_TOKEN
is not configured.
Stop calling _capture_pvc_checksums / _verify_pvc_checksums during the
upgrade flow — FIO's built-in verify=md5 + verify_only mode (Phase 4.1)
already covers data integrity verification without creating extra pods
that risk Multi-Attach errors. The methods are kept for future use (e.g.
unmount/remount verification).
@RaunakJalan
RaunakJalan force-pushed the fix/new-nodes-quoting branch from 5a41b16 to 92dc481 Compare August 14, 2026 09:27
@RaunakJalan RaunakJalan changed the title Fix cross-cluster backup, upgrade tests, and resource preservation Fix cross-cluster backup/restore, upgrade tests, resource preservation, CI pipelines, and Slack thread notifications Aug 14, 2026
After copying the trace log to the artifact directory, delete all
/tmp/minio-trace-*.log files and kill any orphaned mc admin trace
processes. Prevents trace logs from accumulating across runs and
filling the runner's root partition (58 GB observed on one runner).
The 4 K8s-native workflows installed mc to /usr/local/bin without
fallback, failing with curl exit 23 when the runner user (ec2-user)
lacks write permission. Use the same if/else pattern already in the
other 5 workflows: try /usr/local/bin first, fall back to /tmp/mc.
sbcli-dev sn list --json returns keys with capital first letters
(e.g. "Status", "UUID") but the jq filter was using lowercase
".status", causing the online-node count to always be 0 and the
poll loop to time out after 10 minutes.
_cleanup_fio_jobs_only() now polls until all FIO pods are fully
terminated (up to 120s) and then waits 15s for CSI NodeUnstageVolume
to complete. Previously the method only issued kubectl delete and
relied on a 10s sleep, which could leave stale mounts on worker
nodes when storage nodes are shut down for the upgrade.
The mc install block tried /usr/local/bin/mc first and fell back to
/tmp/mc, but a stale /usr/local/bin/mc with wrong permissions caused
"Permission denied" errors on shared runners.  Simplify all 9 workflow
files to always install to /tmp/mc, which is universally writable.
…e upgrade

After FIO pods terminate and CSI unmount completes, run host-level
cleanup on every worker node via oc debug (OpenShift) or kubectl debug:
- Force-unmount any stale CSI volume mount-points
- Disconnect all NVMe-oF subsystems
- Delete lingering VolumeAttachments for upgrade PVCs

This ensures no stale device references remain when storage nodes are
shut down during the upgrade maintenance window, preventing I/O errors
on PVC re-mount after upgrade.
Comment thread e2e/e2e_tests/upgrade_tests/k8s_major_upgrade.py Fixed
@RaunakJalan
RaunakJalan force-pushed the fix/new-nodes-quoting branch from e57307e to 0190d7d Compare August 15, 2026 10:21
The cleanup script had nested single quotes — the outer wrapping
quotes conflicted with awk '{print $3}', causing a shell syntax error
on every worker node. Use shlex.quote() to properly escape the script
and avoid embedded single quotes in the awk expression.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant