diff --git a/community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/scripts/setup.py b/community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/scripts/setup.py index 3d31ba1b82..65dc5ee135 100755 --- a/community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/scripts/setup.py +++ b/community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/scripts/setup.py @@ -459,6 +459,26 @@ def self_report_controller_address(lkp: util.Lookup) -> None: with blob.open('w') as f: f.write(yaml.dump(data)) +def check_slurmdbd_ready(maxtries: int = 30, interval: float = 15.0) -> None: + """Poll sacctmgr until slurmdbd is ready, with timeout safety and backoff.""" + sacctmgr = slurmdirs.prefix / "bin" / "sacctmgr" + wait = 10.0 + for try_cnt in range(1, maxtries + 1): + try: + res = run(f"{sacctmgr} -n -i show cluster", check=False, timeout=10) + if res.returncode == 0: + log.info(f"slurmdbd is ready (try {try_cnt}).") + return + except Exception as e: + log.debug(f"slurmdbd check error: {e}") + + log.info(f"Waiting for slurmdbd to be ready... (try {try_cnt}/{maxtries}, retrying in {int(wait)}s)") + time.sleep(wait) + wait = min(wait * 1.5, interval) + + log.error(f"Slurmdbd is still not ready after {maxtries} tries. Expecting issues!") + raise RuntimeError("Slurmdbd is not ready") + def setup_slurm_health_check_service(lkp: util.Lookup) -> None: if not lkp.cfg.get("slurm_backup_controller_name") or not lkp.cfg.get("enable_controller_load_balancer"): return @@ -546,8 +566,9 @@ def setup_controller(is_primary: bool): run("systemctl enable slurmdbd", timeout=30) run("systemctl restart slurmdbd", timeout=30) - # Wait for slurmdbd to come up - time.sleep(5) + # Wait for slurmdbd to come up. + # This can take a while in case of database migration after upgrades (up to an hour). + check_slurmdbd_ready(maxtries=30, interval=240) sacctmgr = f"{slurmdirs.prefix}/bin/sacctmgr -i" result = run( diff --git a/community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/scripts/slurmsync.py b/community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/scripts/slurmsync.py index 540239ac36..d611b20e84 100644 --- a/community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/scripts/slurmsync.py +++ b/community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/scripts/slurmsync.py @@ -451,32 +451,40 @@ def sync_instances(): action.apply(list(nodes)) -def reconfigure_slurm(): +def reconfigure_slurm() -> bool: + """Fetch new config and regenerate Slurm config files. + + Returns True if config was updated and a scontrol reconfigure is needed. + On the controller, the reconfigure is deferred until after topology + regeneration to avoid issuing two sequential scontrol reconfigure calls + in the same sync cycle (which can cause a socket timeout). + """ update_msg = "*** slurm configuration was updated ***" if lookup().cfg.hybrid: # terraform handles generating the config.yaml, don't do it here - return + return False upd, cfg_new = util.fetch_config() if not upd: log.debug("No changes in config detected.") - return + return False log.debug("Changes in config detected. Reconfiguring Slurm now.") util.update_config(cfg_new) if lookup().is_controller: conf.get_generator(lookup()).generate_configs() - log.info("Restarting slurmctld to make changes take effect.") + # Defer slurmctld restart and scontrol reconfigure: + # Running them now would cause slurmctld to temporarily load new node defs with old topology. + # This causes slurmctld to hang/timeout when update_topology tries to reconfigure it subsequently. + log.info("Config files regenerated. Deferring scontrol reconfigure until after topology update.") try: - # TODO: consider removing "restart" since "reconfigure" should restart slurmctld as well - run("sudo systemctl restart slurmctld.service", check=False) - util.scontrol_reconfigure(lookup()) + util.run(f"wall '{update_msg}'", timeout=30) except Exception: - log.exception("failed to reconfigure slurmctld") - util.run(f"wall '{update_msg}'", timeout=30) + log.warning("failed to broadcast wall message") log.debug("Done.") + return True elif lookup().instance_role_safe == "compute": log.info("Restarting slurmd to make changes take effect.") run("systemctl restart slurmd") @@ -487,19 +495,23 @@ def reconfigure_slurm(): run("systemctl restart sackd") util.run(f"wall '{update_msg}'", timeout=30) log.debug("Done.") + return False def _generate_topology(lkp: util.Lookup) -> Tuple[bool, Any]: return conf.get_generator(lkp).generate_topology_data() -def update_topology(lkp: util.Lookup) -> None: +def update_topology(lkp: util.Lookup) -> Tuple[bool, Any]: + """Regenerate topology files. + + Returns (updated, summary) so the caller can decide when to + issue the scontrol reconfigure and persist the summary. + """ updated, summary = _generate_topology(lkp) # type: ignore[attr-defined] if updated: - log.info("Topology configuration updated. Reconfiguring Slurm.") - util.scontrol_reconfigure(lkp) - # Safe summary only after Slurm got reconfigured, so summary reflects Slurm POV - summary.dump(lkp) + log.info("Topology configuration updated.") + return updated, summary def delete_reservation(lkp: util.Lookup, reservation_name: str) -> None: @@ -662,8 +674,16 @@ def main(): lkp = lookup() if util.should_mount_slurm_bucket() and not lkp.is_controller: return + + if lkp.is_controller and not util.is_active_controller(lkp): + log.info("Local controller is in STANDBY mode. Skipping slurmsync pass.") + return + + # Track if reconfigure_slurm changed cloud.conf so we can trigger + # a deferred scontrol reconfigure inside update_topology + config_changed = False try: - reconfigure_slurm() + config_changed = reconfigure_slurm() except Exception: log.exception("failed to reconfigure slurm") if lkp.is_controller: @@ -692,11 +712,30 @@ def main(): except Exception: log.exception("failed to sync placement groups") + topology_changed = False + topology_summary = None try: - update_topology(lkp) + topology_changed, topology_summary = update_topology(lkp) except Exception: log.exception("failed to update topology") + # Single deferred reconfigure for both config and topology changes. + # Placed here so it runs even if topology generation failed above. + if config_changed or topology_changed: + reasons = [] + if config_changed: + reasons.append("config updated") + if topology_changed: + reasons.append("topology changed") + try: + log.info(f"Reconfiguring Slurm ({', '.join(reasons)}).") + util.scontrol_reconfigure(lkp) + # Only dump summary after successful reconfigure so it reflects Slurm's view + if topology_changed and topology_summary is not None: + topology_summary.dump(lkp) + except Exception: + log.exception("failed to reconfigure slurmctld") + try: sync_maintenance_reservation(lkp) except Exception: diff --git a/community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/scripts/util.py b/community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/scripts/util.py index 0d08d09f35..92078899ef 100755 --- a/community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/scripts/util.py +++ b/community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/scripts/util.py @@ -2381,12 +2381,81 @@ def update_config(cfg: NSDict) -> None: global _lkp _lkp = Lookup(cfg) +def _is_target_controller_up(output: str, target_role: str) -> bool: + """Check if a specific controller role ('primary' or 'backup') is UP in scontrol ping output.""" + for line in output.splitlines(): + line_lower = line.lower() + if target_role in line_lower: + if ("is up" in line_lower or ": up" in line_lower or "(up)" in line_lower) and "down" not in line_lower: + return True + return False + + +def wait_slurmctld_up(lkp: Lookup, timeout: float = 60) -> None: + """Wait for local slurmctld daemon to respond to scontrol ping and report UP status. + This ensures scontrol reconfigure does not fail due to slurmctld not being ready + after a service restart. + """ + log.info("Waiting for slurmctld to be fully up...") + hostname = socket.gethostname().split(".")[0] + backup_name = lkp.cfg.get("slurm_backup_controller_name") + is_backup = (backup_name and hostname == backup_name) or hostname.endswith("-1") + target_role = "backup" if is_backup else "primary" + + for wait in backoff_delay(0.5, timeout=timeout): + try: + res = run(f"{lkp.scontrol} ping", check=False, timeout=5) + if res.returncode == 0: + output = (res.stdout + res.stderr).lower() + if _is_target_controller_up(output, target_role): + log.info(f"slurmctld ({target_role}) is fully up.") + return + except Exception as e: + log.debug(f"scontrol ping check failed: {e}") + if wait > 0: + sleep(wait) + raise TimeoutError(f"slurmctld ({target_role}) is not fully up after {timeout} seconds") + + def scontrol_reconfigure(lkp: Lookup) -> None: log.info("Running systemctl restart slurmctld.service") run("sudo systemctl restart slurmctld.service", timeout=30) + wait_slurmctld_up(lkp) log.info("Running scontrol reconfigure") run(f"{lkp.scontrol} reconfigure") + +def is_active_controller(lkp: Lookup) -> bool: + """Returns True if the local node is the currently active Slurm controller. + In non-HA setups, always returns True for the controller. + In HA setups, queries scontrol ping to check if local node is active primary or active takeover backup. + """ + if not lkp.is_controller: + return False + + backup_name = lkp.cfg.get("slurm_backup_controller_name") + if not backup_name: + return True + + hostname = socket.gethostname().split(".")[0] + is_backup_instance = (backup_name and hostname == backup_name) or hostname.endswith("-1") + + try: + res = run(f"{lkp.scontrol} ping", check=False, timeout=5) + output = (res.stdout + res.stderr).lower() + + primary_up = _is_target_controller_up(output, "primary") + backup_up = _is_target_controller_up(output, "backup") + + if not is_backup_instance: + return primary_up + else: + return (not primary_up) and backup_up + except Exception as e: + log.warning(f"Failed to query scontrol ping for HA active check: {e}") + return not is_backup_instance + + def slurm_version_gte(v1: str, v2: str) -> bool: """Returns true if v1 >= v2, expects YY.MM format""" try: