feat(fluid): support ThinRuntime advanced options#1297
Conversation
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() |
There was a problem hiding this comment.
These tests pass locally, but no workflow under .github/workflows/ invokes them (the only python3 references are curvine-tests/regression/build-server.py), so they won't run on PRs or guard against regressions in config-parse.py. Consider adding a lightweight CI step, e.g. python3 -B curvine-docker/fluid/tests/test_config_parse.py, so this coverage is actually enforced.
| cmd_parts.extend([f"--{cli_flag}", str(value)]) | ||
| emitted_cli_flags.add(cli_flag) | ||
|
|
||
| return " ".join(f'"{arg}"' if " " in arg else arg for arg in cmd_parts) |
There was a problem hiding this comment.
This quoting only wraps args containing spaces. Values with $(...), backticks, or $VAR are still expanded inside double quotes in the generated mount-curvine.sh (e.g. a Dataset option of $(reboot) would execute). This pattern predates this PR, but the change widens the set of user-controllable values flowing into the script. Consider shlex.quote(arg) for defense in depth. (Low severity — Dataset options are admin-controlled.)
| master_hostname, master_rpc_port = self.parse_master_endpoints() | ||
| master_endpoints = self.parse_master_endpoints() | ||
|
|
||
| master_web_port = self.get_option('master-web-port', 'CURVINE_MASTER_WEB_PORT', '8080') |
There was a problem hiding this comment.
The PR adds strict integer/range validation for master RPC endpoint ports, but master-web-port is still interpolated raw into web_port = {master_web_port}, so a non-numeric value yields invalid TOML at mount time rather than a fail-fast error. Minor consistency gap with the PR's stated "reject invalid config early" goal.
2 participants
Summary
This PR improves the Curvine Fluid ThinRuntime integration so Dataset mount options can configure HA Curvine masters, advanced Curvine
[client]/[fuse]TOML fields, and currentcurvine-fuseCLI flags.Issue Describe / Design
Related to the Fluid ThinRuntime integration work.
Before this change, the ThinRuntime parser had four gaps:
master-endpointswas parsed as a singlehost:port, so HA master lists could not be rendered correctly into[[client.master_addrs]].[client]or[fuse]advanced settings, such asclient.enable_unified_fs=falseor FUSE metadata cache TTLs.curvine-fuseexpectsentry-timeout-ms,attr-timeout-ms, andnegative-timeout-ms.The design keeps ThinRuntime-owned fields protected: mount path, filesystem path, worker/io threads, and
client.master_addrsare still managed by the runtime parser. User-provided advanced options are only accepted through explicitclient.<key>andfuse.<key>prefixes.Changes
curvine-docker/fluid/config-parse.py[[client.master_addrs]].curvine-docker/fluid/config-parse.pyclient.<key>andfuse.<key>Dataset options into generated Curvine TOML, while rejecting ThinRuntime-managed fields.curvine-docker/fluid/config-parse.pycurvine-fuseCLI flags where compatible.curvine-docker/fluid/config-parse.pycurvine-docker/fluid/tests/test_config_parse.pycurvine-docker/fluid/thin-runtime/curvine-thinruntime.yamlfluid-thin-runtimeand document advanced Dataset options.Test verified
CARGO_TARGET_DIR=/tmp/curvine-cargo-target-format make formattarget/releaseis root-owned.python3 -B curvine-docker/fluid/tests/test_config_parse.pypython3 -B -m py_compile curvine-docker/fluid/config-parse.py curvine-docker/fluid/tests/test_config_parse.pypython3 - <<'PY' ... yaml.safe_load_all(...) ... PYcurvine-docker/fluid/thin-runtime/curvine-thinruntime.yaml.git diff --check/git diff --cached --checkDependencies