From 81e333c9d947da2d466fe365f467621ac150b3d8 Mon Sep 17 00:00:00 2001 From: Chandra Gorantla Date: Fri, 14 Aug 2026 09:23:59 +0530 Subject: [PATCH] NMS-20110: HA config sync observability, version guard, and gated status - skip sync while the two nodes run different releases - record sync success/failure and surface it in ha-status and REST - flag RESTART REQUIRED when sync replaces startup-only config - make sync roots configurable; add deploy for plugins - report a gated standby as running (exit 161) instead of stopped - drop the unused --partner-rest-url shell option --- .../org/opennms/netmgt/vmmgr/Invoker.java | 24 ++++- .../deep-dive/admin/high-availability.adoc | 30 ++++++ .../opennms/netmgt/ha/HaConfiguration.java | 20 ++++ .../org/opennms/netmgt/ha/HaConfigSyncer.java | 87 +++++++++++++++-- .../netmgt/ha/HaStartupCoordinator.java | 37 +++++++- .../org/opennms/netmgt/ha/HaStatusSchema.java | 7 ++ .../org/opennms/netmgt/ha/HaSyncFiles.java | 95 +++++++++++++++---- .../opennms/netmgt/ha/HaConfigSyncerTest.java | 57 +++++++++-- .../netmgt/ha/HaStartupCoordinatorTest.java | 17 ++++ .../opennms/netmgt/ha/rest/HaRestService.java | 2 +- .../ha/rest/dto/HaInstanceStatusDto.java | 18 ++++ .../ha/rest/impl/HaRestServiceImpl.java | 21 +++- .../netmgt/ha/shell/HaConfigCommand.java | 8 +- .../netmgt/ha/shell/HaStatusCommand.java | 35 ++++++- .../etc/ha-configuration.xml | 13 ++- .../src/main/filtered/bin/opennms | 29 +++++- 16 files changed, 441 insertions(+), 59 deletions(-) diff --git a/core/daemon/src/main/java/org/opennms/netmgt/vmmgr/Invoker.java b/core/daemon/src/main/java/org/opennms/netmgt/vmmgr/Invoker.java index af9d20112b8f..ab2e2d4b8453 100644 --- a/core/daemon/src/main/java/org/opennms/netmgt/vmmgr/Invoker.java +++ b/core/daemon/src/main/java/org/opennms/netmgt/vmmgr/Invoker.java @@ -92,6 +92,11 @@ public class Invoker { private List m_services; private final Path m_statusPath; + /** Written instead of a service list when the process is up but its + * services have not been started; bin/opennms reports this distinctly + * rather than as a failure to determine status. */ + public static final String GATED_STATUS_MARKER = "OpenNMS: gated"; + private static SignalHandler s_handler; /** @@ -168,10 +173,15 @@ private void writeStatusUpdate() { final var output = new StringBuilder(); - // Before setServices() (e.g. blocked at the HA startup gate) there are - // no services to report; an empty status file truthfully reads as such. - final List services = getServices() != null ? getServices() : List.of(); - for (final var invokerService : services) { + // Before setServices() — e.g. blocked at a startup gate — there is no + // service to report on. An empty file reads to bin/opennms as "failed + // to determine running services", so say what is actually true. + if (getServices() == null) { + writeStatusFile(GATED_STATUS_MARKER + "\n"); + return; + } + + for (final var invokerService : getServices()) { final var serviceName = invokerService.getService().getName(); for (final var invoke : invokerService.getService().getInvokes()) { if ("status".equals(invoke.getMethod())) { @@ -188,8 +198,12 @@ private void writeStatusUpdate() { } } + writeStatusFile(output.toString()); + } + + private void writeStatusFile(final String content) { try { - Files.writeString(m_statusPath, output.toString(), Charset.defaultCharset(), CREATE, TRUNCATE_EXISTING); + Files.writeString(m_statusPath, content, Charset.defaultCharset(), CREATE, TRUNCATE_EXISTING); } catch (final IOException e) { System.err.println("ERROR: failed to write current status to " + m_statusPath); e.printStackTrace(); diff --git a/docs/modules/operation/pages/deep-dive/admin/high-availability.adoc b/docs/modules/operation/pages/deep-dive/admin/high-availability.adoc index cb6f7c546f58..5c582f178378 100644 --- a/docs/modules/operation/pages/deep-dive/admin/high-availability.adoc +++ b/docs/modules/operation/pages/deep-dive/admin/high-availability.adoc @@ -137,6 +137,36 @@ While non-ACTIVE, a node replicates `etc/` from the active node every `sync-inte On identically installed nodes leave `` empty; list only files that genuinely differ per node (for example a host-specific certificate store). A file excluded on either node is neither overwritten nor deleted by sync. +`` selects which directories under `$OPENNMS_HOME` are replicated — `etc` by default, plus `deploy` to replicate plugins, which land while the standby's Karaf is stopped and install when it promotes. +Never list `data`: it is each node's own Karaf cache. + +Both nodes must run the same release: the manifest advertises the serving node's version, and a standby skips the cycle with a warning while the versions differ, so a rolling upgrade never mixes configuration formats. + +Startup-only files sync like any other, but the running process keeps the values it booted with, so that node reports `RESTART REQUIRED` until restarted. +In practice this means `opennms.properties.d/`, where your own settings belong; `opennms.properties` itself is product-owned, is replaced by upgrades, and is identical across nodes of the same release. +The credential vault is exempt — it reloads when the file changes, so a rotated credential needs no restart. +After installing a standby, let one sync cycle finish and restart it once, so it boots with the active node's configuration. + +=== Read the status output + +`opennms:ha-status` prints one row per node, then a `Config sync` block: + +[source] +---- +INSTANCE | ROLE | STATE | HEARTBEAT AGE | STALE +opennms-primary | PRIMARY | ACTIVE | 3s ago | no +opennms-secondary | SECONDARY | STANDBY | 2s ago | no + +Config sync +=========== + opennms-secondary last success 45s ago + opennms-primary NEVER SUCCEEDED — credentials not available +---- + +`STALE` means only that a heartbeat is older than the failover threshold; role and state are reported as they are, so a completed failover (a SECONDARY holding ACTIVE) is not a fault. +`NEVER SUCCEEDED` is the line to act on: nothing is being replicated, and a promotion would start from divergent configuration. +`GET /rest/ha/status` reports the same as `heartbeat-stale`, `last-sync-success`, `last-sync-error` and `restart-required`. + == Supervision modes `coordinator` (default) — the node runs the HA state machine described above itself. diff --git a/features/ha-management/ha-api/src/main/java/org/opennms/netmgt/ha/HaConfiguration.java b/features/ha-management/ha-api/src/main/java/org/opennms/netmgt/ha/HaConfiguration.java index 8cb037689eba..e4ba83b82b93 100644 --- a/features/ha-management/ha-api/src/main/java/org/opennms/netmgt/ha/HaConfiguration.java +++ b/features/ha-management/ha-api/src/main/java/org/opennms/netmgt/ha/HaConfiguration.java @@ -49,6 +49,9 @@ @XmlAccessorType(XmlAccessType.NONE) public class HaConfiguration implements Serializable { + /** The sync root every pair replicates; others are opt-in. */ + public static final String DEFAULT_SYNC_ROOT = "etc"; + private static final long serialVersionUID = 1L; @XmlElement(name = "enabled", defaultValue = "false") @@ -123,6 +126,15 @@ public class HaConfiguration implements Serializable { * relative to {@code $OPENNMS_HOME/etc}. A trailing {@code /} excludes a * subtree. {@code ha-configuration.xml} is always excluded regardless. */ + /** + * Directories under {@code $OPENNMS_HOME} to synchronize. Defaults to + * {@code etc} alone; add {@code deploy} to replicate installed plugins. + * {@code data} must never be listed — it is each node's own Karaf cache. + */ + @XmlElementWrapper(name = "sync-roots") + @XmlElement(name = "root") + private List syncRoots; + @XmlElementWrapper(name = "sync-excludes") @XmlElement(name = "exclude") private List syncExcludes = new ArrayList<>(); @@ -227,6 +239,14 @@ public List getSyncExcludes() { return syncExcludes == null ? new ArrayList<>() : syncExcludes; } + public List getSyncRoots() { + return syncRoots == null || syncRoots.isEmpty() ? List.of(DEFAULT_SYNC_ROOT) : syncRoots; + } + + public void setSyncRoots(List syncRoots) { + this.syncRoots = syncRoots; + } + public void setSyncExcludes(List syncExcludes) { this.syncExcludes = syncExcludes; } diff --git a/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaConfigSyncer.java b/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaConfigSyncer.java index 47bc1a6f01b0..309a43199146 100644 --- a/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaConfigSyncer.java +++ b/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaConfigSyncer.java @@ -80,8 +80,37 @@ public class HaConfigSyncer { private static final Pattern SCV_PATTERN = Pattern.compile("^\\$\\{scv:([^:|}]+):([^:|}]+)(?:\\|([^|}]*))?\\}$"); + /** Where a cycle's outcome is published so operators can see it without + * reading a standby's log. */ + public interface StatusRecorder { + void syncSucceeded(); + void syncFailed(String reason); + /** A file read only at JVM startup was replaced; this node needs a + * restart before the new values take effect. */ + void bootConfigChanged(); + } + + private static final StatusRecorder NO_RECORDER = new StatusRecorder() { + @Override public void syncSucceeded() {} + @Override public void syncFailed(String reason) {} + @Override public void bootConfigChanged() {} + }; + + /** Read once into system properties before services start, so replacing + * one leaves the running JVM behind its own files until it restarts. In + * practice this fires for opennms.properties.d/, where operator settings + * belong; opennms.properties itself is product-owned and identical across + * same-version nodes. The credential vault is deliberately absent — it + * reloads on change, and flagging routine rotation would devalue the + * signal (residual: a rotated credential Bootstrap already expanded from a + * ${scv:...} expression stays stale unflagged). */ + private static final String BOOT_ONLY_DIR = "opennms.properties.d/"; + private static final List BOOT_ONLY_FILES = List.of( + "opennms.properties", "bootstrap.properties", "libraries.properties"); + private final Supplier configSupplier; private final Supplier stateSupplier; + private final StatusRecorder recorder; private final HttpClient httpClient; /** Constructor used in tests and when no state tracking is needed (always treats self as STANDBY). */ @@ -100,8 +129,14 @@ public HaConfigSyncer(HaConfiguration config, Supplier stateSup * cycle without restarting the syncer. */ public HaConfigSyncer(Supplier configSupplier, Supplier stateSupplier) { + this(configSupplier, stateSupplier, NO_RECORDER); + } + + public HaConfigSyncer(Supplier configSupplier, Supplier stateSupplier, + StatusRecorder recorder) { this.configSupplier = configSupplier; this.stateSupplier = stateSupplier; + this.recorder = recorder != null ? recorder : NO_RECORDER; this.httpClient = HttpClient.newBuilder() .connectTimeout(Duration.ofSeconds(10)) .build(); @@ -126,6 +161,7 @@ public void sync() { if (config.getPartnerRestUrl() == null || config.getPartnerRestUrl().isBlank()) { LOG.warn("HA sync: partner-rest-url is not configured; skipping config sync"); + recorder.syncFailed("partner-rest-url is not configured"); return; } @@ -133,6 +169,7 @@ public void sync() { if (config.getSyncUsername() == null || resolvedPassword == null) { LOG.warn("HA sync: credentials not available (check SCV entry '{}'); skipping sync", extractScvAlias(config.getSyncPassword())); + recorder.syncFailed("credentials not available"); return; } @@ -147,21 +184,31 @@ public void sync() { if (!HaSyncFiles.isManifest(manifestText)) { LOG.warn("HA sync: response from {} is not an HA manifest; skipping cycle " + "(check partner-rest-url — a wrong host can answer 200 with unrelated text)", baseUrl); + recorder.syncFailed("partner response is not an HA manifest"); + return; + } + String partnerVersion = HaSyncFiles.parseManifestVersion(manifestText); + String localVersion = HaSyncFiles.localVersion(); + if (partnerVersion != null && localVersion != null && !partnerVersion.equals(localVersion)) { + LOG.warn("HA sync: partner runs {} but this node runs {}; skipping cycle until both nodes match " + + "(configuration formats can differ between releases)", partnerVersion, localVersion); + recorder.syncFailed("version mismatch: partner " + partnerVersion + ", local " + localVersion); return; } List manifest = HaSyncFiles.parseManifestText(manifestText); if (manifest.isEmpty()) { LOG.warn("HA sync: manifest from {} was empty; skipping cycle (refusing to delete everything)", baseUrl); + recorder.syncFailed("partner manifest was empty"); return; } - Path etcRoot = HaSyncFiles.etcRoot(); int fetched = 0; int failed = 0; + boolean bootConfigChanged = false; Set manifestPaths = new HashSet<>(); for (HaSyncFiles.Entry entry : manifest) { - manifestPaths.add(entry.relativePath()); + manifestPaths.add(entry.root() + '/' + entry.relativePath()); // The serving side applies exclusions too, but the local list may // legitimately be stricter — never let the partner overwrite an // excluded file. @@ -175,11 +222,15 @@ public void sync() { return; } try { - if (localMatches(etcRoot, entry)) { + Path root = HaSyncFiles.root(entry.root()); + if (localMatches(root, entry)) { continue; } - fetchFile(baseUrl, authHeader, etcRoot, entry); + fetchFile(baseUrl, authHeader, root, entry); fetched++; + if (isBootOnly(entry)) { + bootConfigChanged = true; + } } catch (Exception e) { if (stateSupplier.get() == HaInstanceState.ACTIVE) { LOG.warn("HA sync: this instance became ACTIVE mid-cycle; aborting sync"); @@ -197,13 +248,29 @@ public void sync() { if (excludes != null) { deletionExcludes.addAll(excludes); } - int deleted = propagateDeletions(etcRoot, manifestPaths, deletionExcludes); + int deleted = propagateDeletions(config.getSyncRoots(), manifestPaths, deletionExcludes); if (fetched > 0 || failed > 0 || deleted > 0) { LOG.info("HA sync complete: {} files fetched, {} deleted, {} failed", fetched, deleted, failed); } else { LOG.debug("HA sync complete: no changes"); } + if (bootConfigChanged) { + LOG.warn("HA sync: replaced configuration that is only read at startup; this node must be " + + "restarted before those values take effect"); + recorder.bootConfigChanged(); + } + recorder.syncSucceeded(); + } + + /** True if the entry is read once at JVM startup (so syncing it leaves the + * running process behind its own files until restarted). */ + private static boolean isBootOnly(HaSyncFiles.Entry entry) { + if (!HaSyncFiles.DEFAULT_ROOT.equals(entry.root())) { + return false; + } + String p = entry.relativePath(); + return BOOT_ONLY_FILES.contains(p) || p.startsWith(BOOT_ONLY_DIR); } // ------------------------------------------------------------------------- @@ -247,7 +314,8 @@ private void fetchFile(String baseUrl, String authHeader, Path etcRoot, HaSyncFi String encoded = URLEncoder.encode(entry.relativePath(), StandardCharsets.UTF_8); HttpRequest request = HttpRequest.newBuilder() - .uri(URI.create(baseUrl + "/rest/ha/sync/file?f=" + encoded)) + .uri(URI.create(baseUrl + "/rest/ha/sync/file?root=" + + URLEncoder.encode(entry.root(), StandardCharsets.UTF_8) + "&f=" + encoded)) .header("Authorization", authHeader) .GET() .timeout(Duration.ofSeconds(120)) @@ -286,13 +354,14 @@ private void fetchFile(String baseUrl, String authHeader, Path etcRoot, HaSyncFi /** Deletes local in-scope files that no longer exist on the partner. * {@code excludes} must already be the union of both nodes' exclusions. */ - private int propagateDeletions(Path etcRoot, Set manifestPaths, List excludes) { + private int propagateDeletions(List roots, Set manifestPaths, List excludes) { int deleted = 0; try { - for (HaSyncFiles.Entry local : HaSyncFiles.buildManifest(etcRoot, excludes)) { - if (manifestPaths.contains(local.relativePath())) { + for (HaSyncFiles.Entry local : HaSyncFiles.buildManifest(roots, excludes)) { + if (manifestPaths.contains(local.root() + '/' + local.relativePath())) { continue; } + Path etcRoot = HaSyncFiles.root(local.root()); if (stateSupplier.get() == HaInstanceState.ACTIVE) { LOG.warn("HA sync: this instance became ACTIVE mid-cycle; aborting deletion propagation"); return deleted; diff --git a/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaStartupCoordinator.java b/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaStartupCoordinator.java index f8971d1fa18c..662960e066b7 100644 --- a/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaStartupCoordinator.java +++ b/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaStartupCoordinator.java @@ -437,6 +437,7 @@ private static boolean configEquivalent(HaConfiguration a, HaConfiguration b) { && Objects.equals(a.getPartnerRestUrl(), b.getPartnerRestUrl()) && Objects.equals(a.getSyncUsername(), b.getSyncUsername()) && Objects.equals(a.getSyncPassword(), b.getSyncPassword()) + && Objects.equals(a.getSyncRoots(), b.getSyncRoots()) && Objects.equals(a.getSyncExcludes(), b.getSyncExcludes()); } @@ -731,7 +732,7 @@ private void startSyncIfApplicable() { LOG.debug("HA: this instance is ACTIVE; not starting config sync"); return; } - HaConfigSyncer syncer = new HaConfigSyncer(this::getConfig, this::getCurrentState); + HaConfigSyncer syncer = new HaConfigSyncer(this::getConfig, this::getCurrentState, syncStatusRecorder()); LOG.info("HA: config sync started — partner {}, interval {}s", cfg.getPartnerRestUrl(), cfg.getSyncIntervalSeconds()); syncFuture = scheduler.scheduleAtFixedRate(() -> { @@ -741,6 +742,39 @@ private void startSyncIfApplicable() { }, 0, cfg.getSyncIntervalSeconds(), TimeUnit.SECONDS); } + /** Publishes each sync cycle's outcome into this node's own row, so a + * broken sync is visible from the partner instead of only in this node's + * log — a gated standby serves no REST and raises no events. */ + private HaConfigSyncer.StatusRecorder syncStatusRecorder() { + return new HaConfigSyncer.StatusRecorder() { + @Override public void syncSucceeded() { + writeSyncStatus("last_sync_attempt = NOW(), last_sync_success = NOW(), last_sync_error = NULL", null); + } + @Override public void syncFailed(String reason) { + writeSyncStatus("last_sync_attempt = NOW(), last_sync_error = ?", reason); + } + @Override public void bootConfigChanged() { + writeSyncStatus("boot_config_changed_at = NOW()", null); + } + }; + } + + private void writeSyncStatus(String assignments, String errorParam) { + try (Connection conn = dbFactory.getConnection()) { + String sql = "UPDATE ha_instance_status SET " + assignments + " WHERE instance_id = ?"; + try (PreparedStatement ps = conn.prepareStatement(sql)) { + int idx = 1; + if (errorParam != null) { + ps.setString(idx++, errorParam); + } + ps.setString(idx, config.getInstanceId()); + ps.executeUpdate(); + } + } catch (Exception e) { + LOG.debug("HA: could not record sync status", e); + } + } + private static boolean cancelIfActive(ScheduledFuture f) { if (f != null && !f.isDone() && !f.isCancelled()) { f.cancel(false); // graceful: let in-flight task complete @@ -832,6 +866,7 @@ private void writeInitialStatus(HaInstanceState initialState) throws Exception { "current_state = EXCLUDED.current_state, " + "last_heartbeat = NOW(), " + "hostname = EXCLUDED.hostname, " + + "boot_config_changed_at = NULL, " + "active_since = EXCLUDED.active_since"; try (PreparedStatement ps = conn.prepareStatement(sql)) { ps.setString(1, config.getInstanceId()); diff --git a/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaStatusSchema.java b/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaStatusSchema.java index 078f50a36345..1608a219c359 100644 --- a/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaStatusSchema.java +++ b/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaStatusSchema.java @@ -66,6 +66,13 @@ last_heartbeat TIMESTAMPTZ NOT NULL DEFAULT now(), active_since TIMESTAMPTZ, hostname TEXT )""", + // v2: config-sync observability and boot-config staleness + """ + ALTER TABLE ha_instance_status + ADD COLUMN IF NOT EXISTS last_sync_attempt TIMESTAMPTZ, + ADD COLUMN IF NOT EXISTS last_sync_success TIMESTAMPTZ, + ADD COLUMN IF NOT EXISTS last_sync_error TEXT, + ADD COLUMN IF NOT EXISTS boot_config_changed_at TIMESTAMPTZ""", }; private HaStatusSchema() {} diff --git a/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaSyncFiles.java b/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaSyncFiles.java index 022db2d7b72a..a84411684ba5 100644 --- a/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaSyncFiles.java +++ b/features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaSyncFiles.java @@ -21,6 +21,8 @@ */ package org.opennms.netmgt.ha; +import org.opennms.core.utils.SystemInfoUtils; + import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; @@ -55,11 +57,30 @@ public final class HaSyncFiles { private HaSyncFiles() {} - public record Entry(String relativePath, String sha256, long size) {} + /** A file in the sync scope. {@code root} is the sync root it belongs to + * ("etc", "deploy"); {@code relativePath} is relative to that root. */ + public record Entry(String root, String relativePath, String sha256, long size) {} + + /** The only root synced unless the operator adds more. */ + public static final String DEFAULT_ROOT = "etc"; + + public static Path home() { + return Paths.get(System.getProperty("opennms.home", ".")).toAbsolutePath().normalize(); + } + + /** Resolves a sync root under {@code $OPENNMS_HOME}, rejecting anything + * that is not a plain directory name directly beneath it. */ + public static Path root(String name) throws IOException { + Path h = home(); + Path r = h.resolve(name).normalize(); + if (!r.getParent().equals(h)) { + throw new IOException("sync root must be a directory directly under the OpenNMS home: " + name); + } + return r; + } public static Path etcRoot() { - String opennmsHome = System.getProperty("opennms.home", "."); - return Paths.get(opennmsHome, "etc").toAbsolutePath().normalize(); + return home().resolve(DEFAULT_ROOT); } /** True if {@code relativePath} is excluded from sync (builtin rules plus @@ -111,18 +132,24 @@ public static Path resolveSafe(Path root, String relativePath) throws IOExceptio /** Walks {@code root} and builds manifest entries for every regular, * non-excluded file. Symlinks are skipped: they are never advertised, * served, or deleted — their targets live outside the sync contract. */ - public static List buildManifest(Path root, List configuredExcludes) throws IOException { + public static List buildManifest(List roots, List configuredExcludes) throws IOException { List entries = new ArrayList<>(); - try (Stream walk = Files.walk(root)) { - for (Path p : (Iterable) walk::iterator) { - if (!Files.isRegularFile(p, LinkOption.NOFOLLOW_LINKS)) { - continue; - } - String rel = root.relativize(p).toString().replace('\\', '/'); - if (isExcluded(rel, configuredExcludes)) { - continue; + for (String rootName : roots) { + Path root = root(rootName); + if (!Files.isDirectory(root)) { + continue; + } + try (Stream walk = Files.walk(root)) { + for (Path p : (Iterable) walk::iterator) { + if (!Files.isRegularFile(p, LinkOption.NOFOLLOW_LINKS)) { + continue; + } + String rel = root.relativize(p).toString().replace('\\', '/'); + if (isExcluded(rel, configuredExcludes)) { + continue; + } + entries.add(new Entry(rootName, rel, sha256(p), Files.size(p))); } - entries.add(new Entry(rel, sha256(p), Files.size(p))); } } return entries; @@ -134,14 +161,24 @@ public static List buildManifest(Path root, List configuredExclud * the active" when neither side excludes it. */ static final String EXCLUDE_HEADER_PREFIX = "#exclude "; + /** Advertises the serving node's release, so a standby can refuse to sync + * configuration from a different version. */ + static final String VERSION_HEADER_PREFIX = "#version "; + + /** Introduces the entries belonging to one sync root. */ + static final String ROOT_HEADER_PREFIX = "#root "; + /** First line of every manifest. Entry lines are permissive enough that * arbitrary text can parse as one, so a response without this marker is * never treated as a manifest — least of all as authority to delete. */ public static final String MANIFEST_MARKER = "#ha-manifest 1"; - public static String toManifestText(List entries, List configuredExcludes) { + public static String toManifestText(List entries, List configuredExcludes, String version) { StringBuilder sb = new StringBuilder(); sb.append(MANIFEST_MARKER).append('\n'); + if (version != null) { + sb.append(VERSION_HEADER_PREFIX).append(version).append('\n'); + } List all = new ArrayList<>(BUILTIN_EXCLUSIONS); if (configuredExcludes != null) { all.addAll(configuredExcludes); @@ -149,7 +186,12 @@ public static String toManifestText(List entries, List configured for (String exclude : all) { sb.append(EXCLUDE_HEADER_PREFIX).append(exclude).append('\n'); } + String currentRoot = null; for (Entry e : entries) { + if (!e.root().equals(currentRoot)) { + currentRoot = e.root(); + sb.append(ROOT_HEADER_PREFIX).append(currentRoot).append('\n'); + } sb.append(e.sha256()).append(' ').append(e.size()).append(' ') .append(e.relativePath()).append('\n'); } @@ -162,13 +204,18 @@ public static String toManifestText(List entries, List configured * spelling per file. */ public static List parseManifestText(String text) { List entries = new ArrayList<>(); + String root = DEFAULT_ROOT; for (String line : text.split("\n")) { + if (line.startsWith(ROOT_HEADER_PREFIX)) { + root = line.substring(ROOT_HEADER_PREFIX.length()).trim(); + continue; + } if (line.isBlank() || line.startsWith("#")) continue; int firstSpace = line.indexOf(' '); int secondSpace = line.indexOf(' ', firstSpace + 1); if (firstSpace < 0 || secondSpace < 0) continue; try { - entries.add(new Entry( + entries.add(new Entry(root, normalizeRelative(line.substring(secondSpace + 1)), line.substring(0, firstSpace), Long.parseLong(line.substring(firstSpace + 1, secondSpace)))); @@ -179,12 +226,28 @@ public static List parseManifestText(String text) { return entries; } - /** Parses the serving node's exclusion patterns from a manifest. */ /** True if {@code text} is a manifest emitted by {@link #toManifestText}. */ public static boolean isManifest(String text) { return text != null && text.startsWith(MANIFEST_MARKER); } + /** The release this node runs, as advertised in and compared against a + * manifest. */ + public static String localVersion() { + return new SystemInfoUtils().getVersion(); + } + + /** The release the manifest was served by, or null if it carries none. */ + public static String parseManifestVersion(String text) { + for (String line : text.split("\n")) { + if (line.startsWith(VERSION_HEADER_PREFIX)) { + return line.substring(VERSION_HEADER_PREFIX.length()).trim(); + } + } + return null; + } + + /** Parses the serving node's exclusion patterns from a manifest. */ public static List parseManifestExcludes(String text) { List excludes = new ArrayList<>(); for (String line : text.split("\n")) { diff --git a/features/ha-management/ha-daemon/src/test/java/org/opennms/netmgt/ha/HaConfigSyncerTest.java b/features/ha-management/ha-daemon/src/test/java/org/opennms/netmgt/ha/HaConfigSyncerTest.java index 086001645125..f4d6409ccedf 100644 --- a/features/ha-management/ha-daemon/src/test/java/org/opennms/netmgt/ha/HaConfigSyncerTest.java +++ b/features/ha-management/ha-daemon/src/test/java/org/opennms/netmgt/ha/HaConfigSyncerTest.java @@ -45,17 +45,17 @@ public class HaConfigSyncerTest { @Test public void manifestRoundTrips() { List entries = List.of( - new HaSyncFiles.Entry("poller-configuration.xml", "ab12", 1234L), - new HaSyncFiles.Entry("events/my events.xml", "cd34", 9L)); - String text = HaSyncFiles.toManifestText(entries, null); + new HaSyncFiles.Entry("etc", "poller-configuration.xml", "ab12", 1234L), + new HaSyncFiles.Entry("etc", "events/my events.xml", "cd34", 9L)); + String text = HaSyncFiles.toManifestText(entries, null, null); assertEquals(entries, HaSyncFiles.parseManifestText(text)); } @Test public void manifestCarriesServerExclusions() { List entries = - List.of(new HaSyncFiles.Entry("ok.xml", "aa", 5L)); - String text = HaSyncFiles.toManifestText(entries, List.of("node-local/")); + List.of(new HaSyncFiles.Entry("etc", "ok.xml", "aa", 5L)); + String text = HaSyncFiles.toManifestText(entries, List.of("node-local/"), null); // Header lines advertise the serving node's effective exclusions... List excludes = HaSyncFiles.parseManifestExcludes(text); @@ -76,10 +76,40 @@ public void arbitraryTextIsNotAManifest() { HaSyncFiles.parseManifestText(bogus).isEmpty()); String real = HaSyncFiles.toManifestText( - List.of(new HaSyncFiles.Entry("ok.xml", "aa", 5L)), null); + List.of(new HaSyncFiles.Entry("etc", "ok.xml", "aa", 5L)), null, null); assertTrue(HaSyncFiles.isManifest(real)); } + @Test + public void manifestCarriesTheServingVersion() { + String text = HaSyncFiles.toManifestText( + List.of(new HaSyncFiles.Entry("etc", "ok.xml", "aa", 5L)), null, "36.0.4"); + assertEquals("36.0.4", HaSyncFiles.parseManifestVersion(text)); + assertNull("a manifest without a version must parse as unknown, not fail", + HaSyncFiles.parseManifestVersion( + HaSyncFiles.toManifestText(List.of(), null, null))); + } + + @Test + public void manifestGroupsEntriesByRoot() { + String text = HaSyncFiles.toManifestText(List.of( + new HaSyncFiles.Entry("etc", "poller-configuration.xml", "aa", 5L), + new HaSyncFiles.Entry("deploy", "plugin.kar", "bb", 9L)), null, null); + List parsed = HaSyncFiles.parseManifestText(text); + assertEquals(2, parsed.size()); + assertEquals("etc", parsed.get(0).root()); + assertEquals("deploy", parsed.get(1).root()); + assertEquals("plugin.kar", parsed.get(1).relativePath()); + } + + @Test + public void entriesWithoutARootSectionBelongToEtc() { + // headers are skipped by the entry parser, so a manifest that names no + // root still resolves against the default one + List parsed = HaSyncFiles.parseManifestText("#ha-manifest 1\naa 5 ok.xml\n"); + assertEquals("etc", parsed.get(0).root()); + } + @Test public void manifestParserSkipsMalformedLines() { assertTrue(HaSyncFiles.parseManifestText("garbage\n\nno-size path\n").isEmpty()); @@ -99,17 +129,21 @@ public void manifestPathsMayContainSpaces() { @Test public void buildManifestIncludesBinaryFilesAndAppliesExclusions() throws Exception { - Path etc = tmp.newFolder("etc").toPath(); + Path home = tmp.newFolder("home-a").toPath(); + Path etc = Files.createDirectory(home.resolve("etc")); + System.setProperty("opennms.home", home.toString()); Files.write(etc.resolve("scv.jce"), new byte[]{0, 1, 2, (byte) 0xFF}); // binary Files.writeString(etc.resolve("ha-configuration.xml"), ""); // builtin exclusion Files.createDirectories(etc.resolve("local")); Files.writeString(etc.resolve("local/keep.xml"), ""); // operator exclusion - List manifest = HaSyncFiles.buildManifest(etc, List.of("local/")); + List manifest = HaSyncFiles.buildManifest(List.of("etc"), List.of("local/")); assertEquals(1, manifest.size()); assertEquals("scv.jce", manifest.get(0).relativePath()); assertEquals(4, manifest.get(0).size()); assertEquals(HaSyncFiles.sha256(etc.resolve("scv.jce")), manifest.get(0).sha256()); + assertEquals("etc", manifest.get(0).root()); + System.clearProperty("opennms.home"); } @Test @@ -172,16 +206,19 @@ public void resolveSafeRejectsSymlinkEscape() throws Exception { @Test public void buildManifestSkipsSymlinks() throws Exception { - Path etc = tmp.newFolder("etc-manifest").toPath().toAbsolutePath().normalize(); + Path home = tmp.newFolder("home-b").toPath().toAbsolutePath().normalize(); + Path etc = Files.createDirectory(home.resolve("etc")); Path outside = tmp.newFolder("outside-manifest").toPath().toAbsolutePath().normalize(); + System.setProperty("opennms.home", home.toString()); Files.writeString(etc.resolve("real.xml"), ""); Files.writeString(outside.resolve("secret.txt"), "s3cret"); Files.createSymbolicLink(etc.resolve("leak.txt"), outside.resolve("secret.txt")); - List manifest = HaSyncFiles.buildManifest(etc, null); + List manifest = HaSyncFiles.buildManifest(List.of("etc"), null); assertEquals(1, manifest.size()); assertEquals("real.xml", manifest.get(0).relativePath()); + System.clearProperty("opennms.home"); } // ------------------------------------------------------------------------- diff --git a/features/ha-management/ha-daemon/src/test/java/org/opennms/netmgt/ha/HaStartupCoordinatorTest.java b/features/ha-management/ha-daemon/src/test/java/org/opennms/netmgt/ha/HaStartupCoordinatorTest.java index 519f36339741..e9fec12e8df4 100644 --- a/features/ha-management/ha-daemon/src/test/java/org/opennms/netmgt/ha/HaStartupCoordinatorTest.java +++ b/features/ha-management/ha-daemon/src/test/java/org/opennms/netmgt/ha/HaStartupCoordinatorTest.java @@ -182,6 +182,21 @@ public void configReloadAppliesSyncEnabledToggle() throws Exception { assertFalse("sync-enabled should have flipped to false", coord.getConfig().isSyncEnabled()); } + @Test + public void configReloadAppliesAddedSyncRoot() throws Exception { + HaConfiguration original = primaryConfig(); + original.setSyncRoots(List.of("etc")); + HaStartupCoordinator coord = createCoordinator(original, mockDbFactory); + + HaConfiguration updated = copyOf(original); + updated.setSyncRoots(List.of("etc", "deploy")); + + coord.applyConfigReload(updated); + + assertEquals("an added sync root must take effect without a restart", + List.of("etc", "deploy"), coord.getConfig().getSyncRoots()); + } + @Test public void configReloadAppliesHeartbeatIntervalChange() throws Exception { HaConfiguration original = primaryConfig(); @@ -1372,6 +1387,8 @@ private static HaConfiguration copyOf(HaConfiguration src) { c.setPartnerRestUrl(src.getPartnerRestUrl()); c.setSyncUsername(src.getSyncUsername()); c.setSyncPassword(src.getSyncPassword()); + c.setSyncRoots(src.getSyncRoots()); + c.setSyncExcludes(src.getSyncExcludes()); return c; } diff --git a/features/ha-management/ha-rest/src/main/java/org/opennms/netmgt/ha/rest/HaRestService.java b/features/ha-management/ha-rest/src/main/java/org/opennms/netmgt/ha/rest/HaRestService.java index 6bf7b6c53f4b..cf1b772aaebc 100644 --- a/features/ha-management/ha-rest/src/main/java/org/opennms/netmgt/ha/rest/HaRestService.java +++ b/features/ha-management/ha-rest/src/main/java/org/opennms/netmgt/ha/rest/HaRestService.java @@ -88,5 +88,5 @@ public interface HaRestService { @GET @Path("sync/file") @Produces(MediaType.APPLICATION_OCTET_STREAM) - Response getSyncFile(@QueryParam("f") String relativePath); + Response getSyncFile(@QueryParam("root") String root, @QueryParam("f") String relativePath); } diff --git a/features/ha-management/ha-rest/src/main/java/org/opennms/netmgt/ha/rest/dto/HaInstanceStatusDto.java b/features/ha-management/ha-rest/src/main/java/org/opennms/netmgt/ha/rest/dto/HaInstanceStatusDto.java index 6e90eb1f2b8f..b1953606f118 100644 --- a/features/ha-management/ha-rest/src/main/java/org/opennms/netmgt/ha/rest/dto/HaInstanceStatusDto.java +++ b/features/ha-management/ha-rest/src/main/java/org/opennms/netmgt/ha/rest/dto/HaInstanceStatusDto.java @@ -75,6 +75,24 @@ public class HaInstanceStatusDto { public String getHostname() { return hostname; } public void setHostname(String hostname) { this.hostname = hostname; } + @XmlElement(name = "last-sync-success") + private String lastSyncSuccess; + + @XmlElement(name = "last-sync-error") + private String lastSyncError; + + /** Set when sync replaced a file that is only read at JVM startup: this + * node needs a restart before those values take effect. */ + @XmlElement(name = "restart-required") + private boolean restartRequired; + + public String getLastSyncSuccess() { return lastSyncSuccess; } + public void setLastSyncSuccess(String lastSyncSuccess) { this.lastSyncSuccess = lastSyncSuccess; } + public String getLastSyncError() { return lastSyncError; } + public void setLastSyncError(String lastSyncError) { this.lastSyncError = lastSyncError; } + public boolean isRestartRequired() { return restartRequired; } + public void setRestartRequired(boolean restartRequired) { this.restartRequired = restartRequired; } + public boolean isHeartbeatStale() { return heartbeatStale; } public void setHeartbeatStale(boolean heartbeatStale) { this.heartbeatStale = heartbeatStale; } } diff --git a/features/ha-management/ha-rest/src/main/java/org/opennms/netmgt/ha/rest/impl/HaRestServiceImpl.java b/features/ha-management/ha-rest/src/main/java/org/opennms/netmgt/ha/rest/impl/HaRestServiceImpl.java index fe9737962222..2adc29f34987 100644 --- a/features/ha-management/ha-rest/src/main/java/org/opennms/netmgt/ha/rest/impl/HaRestServiceImpl.java +++ b/features/ha-management/ha-rest/src/main/java/org/opennms/netmgt/ha/rest/impl/HaRestServiceImpl.java @@ -89,7 +89,8 @@ public Response getStatus() { Statement stmt = conn.createStatement(); ResultSet rs = stmt.executeQuery( "SELECT instance_id, configured_role, current_state, last_heartbeat, active_since, " + - "EXTRACT(EPOCH FROM (NOW() - last_heartbeat)) AS age_seconds, hostname " + + "EXTRACT(EPOCH FROM (NOW() - last_heartbeat)) AS age_seconds, hostname, " + + "last_sync_success, last_sync_error, boot_config_changed_at " + "FROM ha_instance_status ORDER BY configured_role")) { List instances = new ArrayList<>(); @@ -103,6 +104,10 @@ public Response getStatus() { Timestamp activeSinceTs = rs.getTimestamp("active_since"); dto.setActiveSince(activeSinceTs != null ? activeSinceTs.toInstant().toString() : null); dto.setHostname(rs.getString("hostname")); + Timestamp syncTs = rs.getTimestamp("last_sync_success"); + dto.setLastSyncSuccess(syncTs != null ? syncTs.toInstant().toString() : null); + dto.setLastSyncError(rs.getString("last_sync_error")); + dto.setRestartRequired(rs.getTimestamp("boot_config_changed_at") != null); long ageSeconds = rs.getLong("age_seconds"); dto.setHeartbeatStale(!rs.wasNull() && ageSeconds > failoverThresholdSeconds); @@ -276,8 +281,9 @@ public Response getSyncManifest() { try { List excludes = coord.getConfig().getSyncExcludes(); List manifest = - HaSyncFiles.buildManifest(HaSyncFiles.etcRoot(), excludes); - return Response.ok(HaSyncFiles.toManifestText(manifest, excludes)).build(); + HaSyncFiles.buildManifest(coord.getConfig().getSyncRoots(), excludes); + return Response.ok( + HaSyncFiles.toManifestText(manifest, excludes, HaSyncFiles.localVersion())).build(); } catch (Exception e) { LOG.error("HA sync: failed to build manifest", e); return Response.serverError().entity("Failed to build manifest: " + e.getMessage()).build(); @@ -285,7 +291,7 @@ public Response getSyncManifest() { } @Override - public Response getSyncFile(String relativePath) { + public Response getSyncFile(String root, String relativePath) { HaStartupCoordinator coord = HaStartupCoordinator.getInstance(); if (coord == null) { return Response.status(Response.Status.NOT_FOUND) @@ -299,8 +305,13 @@ public Response getSyncFile(String relativePath) { return Response.status(Response.Status.FORBIDDEN) .entity("file is excluded from sync: " + relativePath).build(); } + String rootName = (root == null || root.isBlank()) ? HaSyncFiles.DEFAULT_ROOT : root; + if (!coord.getConfig().getSyncRoots().contains(rootName)) { + return Response.status(Response.Status.FORBIDDEN) + .entity("not a configured sync root: " + rootName).build(); + } try { - Path file = HaSyncFiles.resolveSafe(HaSyncFiles.etcRoot(), relativePath); + Path file = HaSyncFiles.resolveSafe(HaSyncFiles.root(rootName), relativePath); if (!Files.isRegularFile(file, LinkOption.NOFOLLOW_LINKS)) { return Response.status(Response.Status.NOT_FOUND) .entity("no such file: " + relativePath).build(); diff --git a/features/ha-management/ha-shell/src/main/java/org/opennms/netmgt/ha/shell/HaConfigCommand.java b/features/ha-management/ha-shell/src/main/java/org/opennms/netmgt/ha/shell/HaConfigCommand.java index e63fcff209b9..57e5c5d1c9de 100644 --- a/features/ha-management/ha-shell/src/main/java/org/opennms/netmgt/ha/shell/HaConfigCommand.java +++ b/features/ha-management/ha-shell/src/main/java/org/opennms/netmgt/ha/shell/HaConfigCommand.java @@ -62,10 +62,6 @@ public class HaConfigCommand implements Action { description = "Set the failover threshold in seconds (minimum 20).") private Integer failoverThreshold; - @Option(name = "--partner-rest-url", - description = "Set the partner REST URL (e.g. http://partner:8980/opennms).") - private String partnerRestUrl; - @Override public Object execute() throws Exception { HaStartupCoordinator coord = HaStartupCoordinator.getInstance(); @@ -82,8 +78,7 @@ public Object execute() throws Exception { boolean modified = syncEnabled != null || syncInterval != null || heartbeatInterval != null - || failoverThreshold != null - || partnerRestUrl != null; + || failoverThreshold != null; if (modified) { HaConfiguration newCfg = copyOf(coord.getConfig()); @@ -91,7 +86,6 @@ public Object execute() throws Exception { if (syncInterval != null) newCfg.setSyncIntervalSeconds(syncInterval); if (heartbeatInterval != null) newCfg.setHeartbeatIntervalSeconds(heartbeatInterval); if (failoverThreshold != null) newCfg.setFailoverThresholdSeconds(failoverThreshold); - if (partnerRestUrl != null) newCfg.setPartnerRestUrl(partnerRestUrl); try { coord.writeConfig(newCfg); // writes to disk and applies in one step diff --git a/features/ha-management/ha-shell/src/main/java/org/opennms/netmgt/ha/shell/HaStatusCommand.java b/features/ha-management/ha-shell/src/main/java/org/opennms/netmgt/ha/shell/HaStatusCommand.java index 6eae2f83f48c..096871876149 100644 --- a/features/ha-management/ha-shell/src/main/java/org/opennms/netmgt/ha/shell/HaStatusCommand.java +++ b/features/ha-management/ha-shell/src/main/java/org/opennms/netmgt/ha/shell/HaStatusCommand.java @@ -30,6 +30,8 @@ import org.opennms.netmgt.ha.HaConfiguration; import org.opennms.netmgt.ha.HaStartupCoordinator; +import java.util.ArrayList; +import java.util.List; import java.sql.Connection; import java.sql.SQLException; import java.sql.ResultSet; @@ -70,7 +72,9 @@ public Object execute() throws Exception { Statement stmt = conn.createStatement(); ResultSet rs = stmt.executeQuery( "SELECT instance_id, configured_role, current_state, active_since, " + - "EXTRACT(EPOCH FROM (NOW() - last_heartbeat)) AS age_seconds, hostname " + + "EXTRACT(EPOCH FROM (NOW() - last_heartbeat)) AS age_seconds, hostname, " + + "EXTRACT(EPOCH FROM (NOW() - last_sync_success)) AS sync_age_seconds, " + + "last_sync_error, boot_config_changed_at " + "FROM ha_instance_status ORDER BY configured_role")) { ShellTable haTable = new ShellTable(); @@ -82,6 +86,7 @@ public Object execute() throws Exception { haTable.column("HOSTNAME"); haTable.column("STALE"); + List syncLines = new ArrayList<>(); boolean anyRows = false; while (rs.next()) { anyRows = true; @@ -91,6 +96,12 @@ public Object execute() throws Exception { String activeSince = rs.getString("active_since"); String hostname = rs.getString("hostname"); + long syncAge = rs.getLong("sync_age_seconds"); + boolean syncEverRan = !rs.wasNull(); + String syncError = rs.getString("last_sync_error"); + boolean restartNeeded = rs.getTimestamp("boot_config_changed_at") != null; + syncLines.add(formatSyncLine(instanceId, syncEverRan, syncAge, syncError, restartNeeded)); + long ageSeconds = rs.getLong("age_seconds"); boolean heartbeatKnown = !rs.wasNull(); boolean heartbeatStale = heartbeatKnown && ageSeconds > failoverThresholdSeconds; @@ -105,6 +116,10 @@ public Object execute() throws Exception { System.out.println("================="); haTable.print(System.out); System.out.println(); + System.out.println("Config sync"); + System.out.println("==========="); + syncLines.forEach(System.out::println); + System.out.println(); } else { System.out.println("(HA table is empty or HA is not configured)"); } @@ -121,6 +136,24 @@ public Object execute() throws Exception { return null; } + /** One line per instance: sync that has never succeeded is the dangerous + * state, so it is named rather than left blank. */ + private static String formatSyncLine(String instanceId, boolean everRan, long ageSeconds, + String error, boolean restartNeeded) { + StringBuilder sb = new StringBuilder(" ").append(nvl(instanceId)).append(" "); + if (!everRan) { + sb.append(error == null ? "never run" : "NEVER SUCCEEDED — " + error); + } else if (error != null) { + sb.append("FAILING — ").append(error).append(" (last success ").append(formatAge(ageSeconds)).append(")"); + } else { + sb.append("last success ").append(formatAge(ageSeconds)); + } + if (restartNeeded) { + sb.append(" [RESTART REQUIRED: startup-only configuration changed]"); + } + return sb.toString(); + } + private static String formatAge(long ageSeconds) { if (ageSeconds < 60) { return ageSeconds + "s ago"; diff --git a/opennms-base-assembly/src/main/filtered-meridian/etc/ha-configuration.xml b/opennms-base-assembly/src/main/filtered-meridian/etc/ha-configuration.xml index 273748b2ae76..00df8910fb67 100644 --- a/opennms-base-assembly/src/main/filtered-meridian/etc/ha-configuration.xml +++ b/opennms-base-assembly/src/main/filtered-meridian/etc/ha-configuration.xml @@ -63,7 +63,18 @@ ${scv:hasync:password|opennms} + +