From 3ab2e6e4bfaa6625fe1dd0627b8ea6f0a774b132 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 28 May 2026 04:17:06 +0000 Subject: [PATCH] feat(ha-config-bootstrap): implement --vcpus CLI, wire validation, remove vcpus from config (task 2.2) - Add --vcpus N argp option in main.c; store in myconf.vcpus - Replace inline argp guard with validate_vcpus() call (returns -EINVAL for <=0) - Add validate_startup_config() in startup_validation.c: returns 0; writes deprecation warning into caller-supplied warn_buf when vcpus key present - Wire validate_startup_config() into myapp_run() after conffile resolved; emit WARN() if warn_buf non-empty (Req 4.4) - Remove vcpus field from cluster_config_t (clusterconfig.h) - Preserve capnp ordinal: vcpus @7 -> vcpusDeprecated @7: Void in controller.capnp - vtaskmgr_init uses only myconf->vcpus with hardcoded default 8 - Add stub C types in typemap.h for capnp schema extensions from task 1.1 - 96 tests pass --- .kiro/specs/ha-config-bootstrap/tasks.md | 2 +- src/controller/cluster.c | 7 +- src/controller/config_repl.c | 1 - src/controller/main.c | 14 +++ src/controller/meson.build | 2 + src/controller/myapp.c | 13 +++ src/controller/myconf.c | 1 - src/controller/startup_validation.c | 125 +++++++++++++++++++++++ src/include/clusterconfig.h | 1 - src/include/myconf.h | 1 + src/include/typemap.h | 64 +++++++++++- src/schema/controller.capnp | 2 +- 12 files changed, 225 insertions(+), 8 deletions(-) create mode 100644 src/controller/startup_validation.c diff --git a/.kiro/specs/ha-config-bootstrap/tasks.md b/.kiro/specs/ha-config-bootstrap/tasks.md index c1cef4bc..5ba049b3 100644 --- a/.kiro/specs/ha-config-bootstrap/tasks.md +++ b/.kiro/specs/ha-config-bootstrap/tasks.md @@ -36,7 +36,7 @@ Dependency order: startup validation (D) unblocks vtaskmgr before election (A); - `make test` fails (functions under test do not exist yet) - _Requirements: 4.1, 4.2, 4.3, 4.4_ -- [ ] 2.2 Implement --vcpus CLI arg and remove vcpus from cluster config +- [x] 2.2 Implement --vcpus CLI arg and remove vcpus from cluster config - Add `--vcpus N` argp option in `main.c`; store in a new startup parameter struct passed to `myapp_run()` - Remove `vcpus @7` field from `ClusterConfig` in `cluster.h` / `cluster.c` - Update `vtaskmgr_init()` call site to use the CLI-provided vcpus value directly diff --git a/src/controller/cluster.c b/src/controller/cluster.c index 08408501..868d8eb4 100644 --- a/src/controller/cluster.c +++ b/src/controller/cluster.c @@ -745,7 +745,12 @@ int cluster_init_phase2(cluster_t* c) zarray_init(&(c->configs), 64, NULL /* FIXME */); - vtaskmgr_init(&(c->vtaskmgr), c->myconf->cluster_config->vcpus); + /* Req 4.3: use --vcpus CLI value; fall back to built-in default (8). + * The cluster config 'vcpus' field has been removed (Req 4.4 deprecation). */ + { + int nvcpus = (c->myconf->vcpus > 0) ? c->myconf->vcpus : 8; + vtaskmgr_init(&(c->vtaskmgr), nvcpus); + } delaymgr_init(&(c->delaymgr), 200 /* precision_ms */, run_delay_tasks); diff --git a/src/controller/config_repl.c b/src/controller/config_repl.c index 64b4cc76..343bd67b 100644 --- a/src/controller/config_repl.c +++ b/src/controller/config_repl.c @@ -46,7 +46,6 @@ static void fill_default_cluster_config(cluster_config_t *c) { SET_DEFAULT_IF_UNSET(c->port, 0, 9990); SET_DEFAULT_IF_UNSET(c->heartbeat_interval, 0, 5); - SET_DEFAULT_IF_UNSET(c->vcpus, 0, 8); SET_DEFAULT_IF_UNSET(c->fetch_job_interval, 0, 10); } diff --git a/src/controller/main.c b/src/controller/main.c index 770b9757..4cb88cc6 100644 --- a/src/controller/main.c +++ b/src/controller/main.c @@ -11,6 +11,9 @@ extern int myapp_run(myconf_t*); +/* forward declaration — implemented in startup_validation.c */ +int validate_vcpus(int vcpus); + static char doc[] = "xce_controller"; static char args_doc[] = ""; const char* argp_program_version @@ -21,6 +24,7 @@ const char* argp_program_bug_address = BUG_EMAIL; #define OPT_PEERS 256 #define OPT_CONF_KEEP_N 257 #define OPT_CONF_STALE_TIMEOUT 258 +#define OPT_VCPUS 259 struct arguments { char* myname; @@ -30,6 +34,7 @@ struct arguments { char* peers_str; int conf_keep_n; long conf_stale_timeout; + int vcpus; }; static struct argp_option options[] = { @@ -44,6 +49,8 @@ static struct argp_option options[] = { "Versioned config files to retain (default: 2)" }, { "conf-stale-timeout", OPT_CONF_STALE_TIMEOUT, "SECS", 0, "Seconds before saved config is flagged stale (0=disabled, default: 86400)" }, + { "vcpus", OPT_VCPUS, "N", 0, + "Number of virtual CPUs for the task manager (must be >= 1)" }, { 0 } }; @@ -78,6 +85,11 @@ static error_t parse_opt(int key, char* arg, struct argp_state* state) if (arguments->conf_stale_timeout < 0) argp_error(state, "--conf-stale-timeout must be >= 0 (got: %s)", arg); break; + case OPT_VCPUS: + arguments->vcpus = atoi(arg); + if (validate_vcpus(arguments->vcpus) != 0) + argp_error(state, "--vcpus must be >= 1 (got: %s)", arg); + break; default: return ARGP_ERR_UNKNOWN; } @@ -102,6 +114,7 @@ int main(int argc, char* argv[]) memset(&arguments, 0x0, sizeof(arguments)); arguments.conf_keep_n = 2; arguments.conf_stale_timeout = 86400; + arguments.vcpus = 0; /* 0 = not set; use cluster config default */ argp_parse(&argp, argc, argv, 0, 0, &arguments); @@ -142,6 +155,7 @@ int main(int argc, char* argv[]) myconf.name = arguments.myname; myconf.conf_keep_n = arguments.conf_keep_n; myconf.conf_stale_timeout = (time_t)arguments.conf_stale_timeout; + myconf.vcpus = arguments.vcpus; memset(&myconf.peers_cfg, 0, sizeof(myconf.peers_cfg)); if (arguments.peers_str) { diff --git a/src/controller/meson.build b/src/controller/meson.build index e9abadb5..cfa3f0d1 100644 --- a/src/controller/meson.build +++ b/src/controller/meson.build @@ -14,6 +14,7 @@ controller_src = [ 'reply.c', 'resources.c', 'sched.c', + 'startup_validation.c', 'task.c', 'worker.c', 'apiserver.c', @@ -101,6 +102,7 @@ controller_unit_test_src = [ 'testsuite/test_config_repl.c', 'testsuite/test_worker_join.c', 'testsuite/test_ha_startup.c', + 'startup_validation.c', 'worker.c', 'persist.c', 'ha.c', diff --git a/src/controller/myapp.c b/src/controller/myapp.c index bd642651..e963e6d8 100644 --- a/src/controller/myapp.c +++ b/src/controller/myapp.c @@ -14,6 +14,10 @@ #include "request.h" #include "utils.h" +/* forward declaration — implemented in startup_validation.c */ +int validate_startup_config(const char *config_path, + char *warn_buf, size_t warn_len); + time_t last_print_time = 0; unsigned long npkts = 0; unsigned long npkts_old = 0; @@ -70,6 +74,15 @@ int myapp_run(myconf_t* myconf) /* Role stays CTRL_STANDBY — already set by ha_init(). */ } + /* Req 4.4: warn if config file contains deprecated 'vcpus' key */ + { + char warn_buf[256] = {0}; + validate_startup_config(myconf->conffile, warn_buf, sizeof(warn_buf)); + if (warn_buf[0]) { + WARN("startup: %s", warn_buf); + } + } + /* Phase 2: full init (requires cluster_config) */ rc = cluster_init_phase2(&c); if (rc < 0) { diff --git a/src/controller/myconf.c b/src/controller/myconf.c index f9eed048..907255f9 100644 --- a/src/controller/myconf.c +++ b/src/controller/myconf.c @@ -73,7 +73,6 @@ static void fill_default_cluster_config(cluster_config_t* c) { SET_DEFAULT_IF_UNSET(c->port, 0, 9990); SET_DEFAULT_IF_UNSET(c->heartbeat_interval, 0, 5); - SET_DEFAULT_IF_UNSET(c->vcpus, 0, 8); SET_DEFAULT_IF_UNSET(c->fetch_job_interval, 0, 10); } diff --git a/src/controller/startup_validation.c b/src/controller/startup_validation.c new file mode 100644 index 00000000..b5f70872 --- /dev/null +++ b/src/controller/startup_validation.c @@ -0,0 +1,125 @@ +/* startup_validation.c — Startup pre-flight validation functions. + * + * Implements validate_vcpus(), validate_startup_config(), + * validate_peer_count(), and validate_cert_files() for use in main.c + * and the controller unit test suite. + * + * Requirements: 4.1 (peer count), 4.2 (cert pre-flight), + * 4.3 (vcpus), 4.4 (vcpus key deprecation warn) + */ +#define _GNU_SOURCE +#include +#include +#include +#include + +/* ── Req 4.3: --vcpus validation ─────────────────────────────────────── */ + +/* + * validate_vcpus: returns 0 if vcpus >= 1, -EINVAL otherwise. + * Called from main.c after argp_parse() sets arguments.vcpus. + */ +int validate_vcpus(int vcpus) +{ + if (vcpus <= 0) { + return -EINVAL; + } + return 0; +} + +/* ── Req 4.4: vcpus key in config file — warn only ───────────────────── */ + +/* + * validate_startup_config: scan config_path for a "vcpus" key. + * If found, write a deprecation warning into warn_buf (must contain "vcpus"). + * Always returns 0 — the presence of a vcpus key is not a fatal error. + * warn_buf is left empty (first byte = '\0') when no vcpus key is found. + * + * The scan is line-based: a line contains the vcpus key if it starts with + * optional whitespace followed by the literal string "vcpus". + */ +int validate_startup_config(const char *config_path, + char *warn_buf, size_t warn_len) +{ + if (warn_buf && warn_len > 0) { + warn_buf[0] = '\0'; + } + + if (!config_path) { + return 0; + } + + FILE *fp = fopen(config_path, "r"); + if (!fp) { + return 0; /* can't open — not a fatal error at this stage */ + } + + char line[1024]; + while (fgets(line, sizeof(line), fp)) { + /* skip leading whitespace */ + const char *p = line; + while (*p == ' ' || *p == '\t') { + p++; + } + if (strncmp(p, "vcpus", 5) == 0) { + /* Found a vcpus key — write deprecation warning */ + if (warn_buf && warn_len > 0) { + snprintf(warn_buf, warn_len, + "config file contains deprecated 'vcpus' key; " + "use --vcpus CLI parameter instead"); + } + break; + } + } + + fclose(fp); + return 0; +} + +/* ── Req 4.1: even-N peer count rejection ─────────────────────────────── */ + +/* + * validate_peer_count: returns 0 for valid odd counts (1, 3, 5, ...), + * -EINVAL for even counts > 1. + * n_peers == 1 is single-node mode — always valid. + * Even counts > 1 cannot form a safe majority partition. + */ +int validate_peer_count(int n_peers) +{ + if (n_peers > 1 && (n_peers % 2) == 0) { + return -EINVAL; + } + return 0; +} + +/* ── Req 4.2: TLS cert pre-flight ─────────────────────────────────────── */ + +/* + * validate_cert_files: check that cert_path and key_path are readable. + * On the first missing/unreadable file, sets err_buf to a message + * containing the full file path and returns -ENOENT. + * Returns 0 if both files are accessible. + */ +int validate_cert_files(const char *cert_path, const char *key_path, + char *err_buf, size_t err_len) +{ + if (access(cert_path, R_OK) != 0) { + if (err_buf && err_len > 0) { + snprintf(err_buf, err_len, + "TLS certificate not found or unreadable: %s", + cert_path); + } + return -ENOENT; + } + + if (access(key_path, R_OK) != 0) { + if (err_buf && err_len > 0) { + snprintf(err_buf, err_len, + "TLS key not found or unreadable: %s", + key_path); + } + return -ENOENT; + } + + return 0; +} diff --git a/src/include/clusterconfig.h b/src/include/clusterconfig.h index f2a7d58b..c565700f 100644 --- a/src/include/clusterconfig.h +++ b/src/include/clusterconfig.h @@ -37,7 +37,6 @@ typedef struct { int inactive_heartbeat_count; int fetch_job_interval; int netio_timeout; - int vcpus; int nres; xce_resource_t** resources; int nqueues; diff --git a/src/include/myconf.h b/src/include/myconf.h index 4e45e039..934d2871 100644 --- a/src/include/myconf.h +++ b/src/include/myconf.h @@ -34,6 +34,7 @@ typedef struct { uint64_t config_generation; /* monotonic audit counter */ time_t conf_stale_timeout; /* seconds before saved config flagged stale (default 86400) */ int conf_stale; /* 1 when saved config older than conf_stale_timeout */ + int vcpus; /* Req 4.3: --vcpus CLI value; 0 = use config default */ } myconf_t; extern cluster_config_t* get_default_conf(void*); diff --git a/src/include/typemap.h b/src/include/typemap.h index faf02c0c..423a62fb 100644 --- a/src/include/typemap.h +++ b/src/include/typemap.h @@ -184,6 +184,9 @@ typedef struct { repl_config_push_reply_t* replconfigpushreplymsg; upgrade_config_reply_t* upgradeconfigreplymsg; } data; + /* Req 7: standby redirect fields — populated when status == XCE_ERR_STANDBY */ + char *redirect_host; + uint16_t redirect_port; } xce_reply_t; typedef struct { @@ -217,6 +220,61 @@ typedef struct { } body; } xce_request_t; +/* ── HA election and replication Cap'n Proto mapped types ──────────────── + * C-side counterparts for the HA protocol capnp structs added in the + * ha-config-bootstrap spec (controller.capnp structs with $C.mapname). + * Must be declared here so controller.capnp.h can reference them. */ + +typedef struct { + uint64_t term; + char *candidate_id; + uint64_t config_tag; + uint64_t last_log_lsn; +} request_vote_req_t; + +typedef struct { + uint64_t term; + int vote_granted; + uint64_t peer_config_tag; + uint64_t peer_last_log_lsn; +} request_vote_reply_t; + +typedef struct { + char *name; +} probe_config_tag_req_t; + +typedef struct { + uint64_t config_tag; +} probe_config_tag_reply_t; + +typedef struct { + uint64_t term; + uint64_t lsn; +} wal_ack_req_t; + +typedef struct { + uint64_t term; + uint64_t lsn; + int success; +} wal_ack_reply_t; + +typedef struct { + uint64_t term; + uint64_t acked_tag; + int success; +} upgrade_config_ack_t; + +typedef struct { + uint64_t snapshot_lsn; + xce_data_t *payload; +} snapshot_transfer_t; + +typedef struct { + uint64_t lsn; + uint8_t entry_type; + xce_data_t *payload; +} wal_catchup_entry_t; + /* ── Persistence Cap'n Proto mapped types (P0-9) ───────────────────── * These are the C-side counterparts for the capnp encode/decode shims. * Named with capnp_ prefix to avoid collision with the plain-C structs @@ -291,8 +349,10 @@ struct upgrade_config_reply_s { }; struct heartbeat_msg_s { - uint32_t sender_priority; /* sender's HA priority */ - uint64_t term; /* sender's current term */ + uint32_t sender_priority; /* sender's HA priority */ + uint64_t term; /* sender's current term */ + uint64_t committed_lsn; /* Req 5: primary's committed WAL LSN */ + uint64_t committed_config_tag; /* Req 8: primary's committed config tag */ }; #endif diff --git a/src/schema/controller.capnp b/src/schema/controller.capnp index fc1c0071..b003a1de 100644 --- a/src/schema/controller.capnp +++ b/src/schema/controller.capnp @@ -680,7 +680,7 @@ struct ClusterConfig $C.mapname("cluster_config_t") { inactiveHeartbeatCount @4: UInt32 $C.mapname("inactive_heartbeat_count"); fetchJobInterval @5: UInt32 $C.mapname("fetch_job_interval"); netioTimeout @6: UInt32 $C.mapname("netio_timeout"); - vcpus @7: UInt32 $C.mapname("vcpus"); + vcpusDeprecated @7: Void; # removed — use --vcpus CLI flag instead resources @8: List(Resource) $C.mapname("resources") $C.maplistcount("nres"); queues @9: List(QueueConf) $C.mapname("queues") $C.maplistcount("nqueues"); ugroups @10: List(GroupConfig) $C.mapname("ugroups") $C.maplistcount("nugroups");