s3lvol: rename COS to S3, add MinIO support, portable ISA and py3.8 RPC client - #1524
Conversation
Review: s3lvol COS→S3 rename, MinIO support, portable ISA, py3.8 RPC client (PR #1524)AI-generated review. Reviewed against the base branch ( VerdictApprove. This is a large PR (1398+/219−, 37 files) but internally consistent: the COS→S3 rename is complete, the new S3/MinIO wiring is correct, the Python 3.8 RPC launcher is a sound design, and the accompanying tests genuinely exercise the new code paths. I found no correctness bugs. Three low-severity observations follow. What I verified
Findings (low severity)
Notes
|
e51f53f to
fd9e173
Compare
fd9e173 to
4545202
Compare
4545202 to
0308b5b
Compare
|
On review finding 3 ( |
…PC client Rename the s3lvol backend from Tencent COS to generic S3 across the target, scripts, tests and deploy tooling, and add first-class MinIO support: - Config moves from /data/cubelet/cos.cfg to s3.cfg with S3 field names (access_key_id / secret_access_key / endpoint / buckets / path_style / no_tls). There is no fallback from the old file or the old keys. - rcow_add_cos_config becomes rcow_add_s3_config. - path_style / no_tls flags are threaded through the startup scripts, the purge path and an address-flags helper so a path-style plain-HTTP backend (MinIO in CI) works. Ship a stdlib SigV4 ensure-bucket tool (s3_bucket.py) and use it in the one-click supervisor before rcow_start.sh; the bucket is dedicated to s3lvol so it never shares prefixes with the volume plugin. Make release binaries CPU-portable: SPDK builds with --target-arch=haswell (x86_64) or armv8.2-a+crypto (aarch64) instead of native, and make_release.sh refuses to package a native tree (mk/s3lvol_isa_gate.sh). The one-click preflight now checks for avx2. Ship an rpc.py launcher (scripts/rpc.py + rpc_compat.py) that backfills argparse.BooleanOptionalAction so SPDK's unmodified client runs on Python 3.8 (Ubuntu 20.04); SPDK's rpc.py is packaged as scripts/spdk_rpc.py. One-click: install.sh renders s3.cfg from CUBE_S3_*, installs nvme-cli when missing, validates the packaged RPC client, and the supervisor preserves rcow_start's exit status so systemd Restart=on-failure retries. Tests: add offline ISA baseline and rpc.py py3.8 compat suites, extend the MinIO S3 guard and package layout tests, and wire the offline tools into run_all.sh. Signed-off-by: jinlong <jinlong@tencent.com>
0308b5b to
a297d0a
Compare
| RCOW_SPDK_RPC_PY="${RCOW_SPDK_RPC_PY:-${SPDK_ROOT}/scripts/rpc.py}" | ||
| # Same launcher as the package: it applies the 3.8 argparse shim and | ||
| # then runs $SPDK_ROOT/scripts/rpc.py. Calling the SPDK file directly | ||
| # would blow up on Ubuntu 20.04 (no BooleanOptionalAction). |
There was a problem hiding this comment.
Low-severity observation: pointing RCOW_SPDK_RPC_PY at the always-present launcher changes the failure behavior of rcow_start.sh's preflight [ -x "${RCOW_SPDK_RPC_PY}" ]. That check now passes even when SPDK_ROOT is wrong and no fallback exists, so the problem surfaces only on the first rcow_srpc call — i.e. after the target is already started and rcow_wait_rpc polls for up to 60 s before bail tears it down. The new rcow_wait_rpc last-error capture does surface the launcher's clear "cannot find SPDK rpc.py" message at that point (better than the old silent timeout), but the fail-fast preflight is gone. A preflight that proves the launcher resolves its upstream — e.g. run it with --help, as validate_s3lvol_rpc_client already does for the package — would close the gap.
| TGT="${ROOT}/app/s3lvol_tgt/s3lvol_tgt" | ||
| COS_CFG="${RCOW_COS_CFG:-/data/cubelet/cos.cfg}" | ||
| S3_CFG="${RCOW_S3_CFG:-/data/cubelet/s3.cfg}" | ||
| ACTIVE_FILE=/data/cubelet/rcow/active_lvols |
There was a problem hiding this comment.
Low-severity consistency note: this file (like the other test/dataplane/*.sh scripts) still invokes ${SPDK_ROOT}/scripts/rpc.py directly, so it bypasses the new scripts/rpc.py launcher. On Python 3.8 — the exact platform this PR makes the deployment support — that direct call hits the argparse.BooleanOptionalAction error the launcher was added to fix, so the dataplane suite would break on a 20.04 host even though the runtime scripts are 3.8-safe. Not currently breaking (the tests run on the dev/CI host, which is 3.9+), but the runtime and test paths now use different RPC entry points. Consider having these scripts resolve SPDK_RPC_PY through rcow_common.sh (or RCOW_SPDK_RPC_PY) so both stay in sync.
Summary
Rename the s3lvol backend from Tencent COS to generic S3 and add first-class MinIO support, across the target, scripts, tests and deploy tooling.
Highlights
cos.cfgtos3.cfgwith S3-style field names (access_key_id,secret_access_key,endpoint,buckets,path_style,no_tls). There is no fallback from the old file or the old keys. Thercow_add_cos_configRPC is renamed torcow_add_s3_config.path_style/no_tlsflags are threaded through the startup scripts, the purge path and an address-flags helper, so a path-style plain-HTTP backend works out of the box.s3_bucket.pycreates/confirms the dedicated s3lvol bucket (kept separate from the volume plugin's) beforercow_start.sh; no awscli or boto3 needed.--target-arch=haswell(x86_64) /armv8.2-a+crypto(aarch64) instead ofnative;make_release.shrefuses to package a native tree, and the one-click preflight checks for AVX2.rpc.pylauncher plusrpc_compat.pybackfillsargparse.BooleanOptionalActionso SPDK's unmodified client runs on Ubuntu 20.04; SPDK's ownrpc.pyships asspdk_rpc.py.install.shrenderss3.cfgfromCUBE_S3_*, installsnvme-cliwhen missing, validates the packaged RPC client, and the supervisor preservesrcow_start's exit status so systemdRestart=on-failureretries.Tests
test_isa_baseline.sh) and rpc.py py3.8 compat (test_rpc_py38_compat.sh), plus thes3_bucket.pyself-test, all wired intorun_all.sh.test_minio_s3_guard.sh(s3.cfg rendering, bucket separation, nvme-cli installer, RPC client validation, supervisor exit code) andtest_package_layout.sh(packaged bucket tool and RPC launcher).