hashgraph / hashgraph/solo-weaver

feat(daemon): gate daemon upgrades on a declared CLI/daemon compatibility range instead of version equality

Open
#975 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
3
Forks
0
Avg merge
3d 2h
Merged PRs (30d)
46

Description

## Problem

There is no policy governing when a `solo-provisioner` CLI upgrade should upgrade or
restart `solo-provisioner-daemon`. The current behaviour is not a decision — it is what
two unrelated early-returns happen to produce:

- **A CLI upgrade never restarts the daemon.** `ensureBlockNodeDaemon`
(`cmd/cli/commands/block/node/daemon_offer.go`) returns early when the service is
running, so `NewDaemonServiceInstallWorkflow` — which ends in an unconditional
`RestartService` (`internal/workflows/steps/step_daemon.go`) — never runs on an
already-provisioned host.
- **The daemon can never be upgraded through the catalog path.** `InstallDaemonBinaryStep`
skips the download when `installer.IsInstalled()` reports true, and that method returns
`softwareState.Installed`, a plain bool (`pkg/software/base_installer.go`). It is
version-agnostic: once a daemon has ever been installed, no newer one is ever fetched.

So the effective policy is "install once, never upgrade, never restart". That is a
defensible default, but nothing states it, nothing enforces it, and nothing reports when
the daemon on disk stops matching the daemon in memory.

The gap becomes visible now that CLI self-install places the co-built daemon binary on the
host (#973). The rename cannot disturb a running daemon — it keeps its own inode — but the
on-disk and running versions can now diverge with no surface reporting it. The failure mode
is not restart churn; it is an unrelated reboot silently adopting a daemon version nobody
decided to roll out.

## Why version equality is the wrong trigger

The obvious fix — "CLI version changed, so replace the daemon" — treats *CLI version
changed* as a proxy for *the daemon must change*. Those are different questions, and the
proxy over-triggers: it forces a daemon replacement (and eventually a restart) on every
release, including the majority where no daemon code changed at all.

Content comparison cannot rescue it either. The version string is stamped into the daemon
binary via `-ldflags` (`LDFLAGS` in `Taskfile.yaml`), so **every release produces a
byte-different daemon binary even when zero daemon source changed.** Checksums cannot
discriminate "the daemon actually changed" from "the version stamp moved".

## Proposed fix

Declare a compatibility contract rather than comparing versions for equality.

**1. The CLI declares the daemon range it supports.** A `minDaemonVersion` (and optionally
a max) as a field on the `solo-provisioner-daemon` catalog entry in
`pkg/software/infrastructure-catalog.yaml`, or an embedded constant. Bumping the minimum
becomes a deliberate release-time act — "this CLI needs the new daemon RPC / the new nft
set layout" — instead of an accident of the version stamp.

**2. Decide against the running daemon, not the installed CLI.** The daemon's `--version`
already emits JSON and the parser exists (`daemonVersionOutput` in `step_daemon.go`).

| Running daemon | Action |
|---|---|
| Satisfies the declared range | Nothing. No copy, no restart. |
| Below the minimum | Required upgrade: replace the binary and restart, logging the reason explicitly (e.g. `daemon 1.2.0 < minimum 1.3.0 required by CLI 1.5.0`) |
| Not installed | Install, as today |

`pkg/semver.CheckVersionRequirements(progVersion, minimum, maximum)` already exists and has
no production caller — it is exactly this primitive.

**3. Never restart implicitly for an in-range daemon.** Provide an explicit escape hatch
for operators who want the newest build regardless — `solo-provisioner daemon service
upgrade`, or a `--force-daemon-upgrade` flag.

**4. Make skew visible.** `daemon service check` should report the running version, the
on-disk version, the CLI's required range, and whether a restart is pending. Whatever the
policy, silent divergence between disk and memory is the part that bites.

**5. Make `IsInstalled()` version-aware, or stop using it as the upgrade gate.** As long as
it is a bool, the catalog path can never deliver a newer daemon even when one is required.

## Restart cost, for calibration

Worth recording, because it sets how conservative the policy needs to be: a daemon restart
is a brief reconciliation gap, not a traffic interruption.

- Nothing tears down enforcement on shutdown. The nft sets and tc qdiscs live in the
kernel; the monitors simply stop. Existing classification keeps being enforced while the
daemon is down.
- The pod-lifecycle watcher performs a full `List` on startup and dispatches an upsert for
every existing block-node pod (`internal/daemon/blocknode/pod_watcher.go`), so veth HTB
state is re-asserted rather than lost.
- The shaper `Reconciler` holds no authoritative in-memory state
(`internal/blocknode/shaper/reconciler.go`): it re-derives desired membership from
statusz, reads live nft, and writes only the diff.

The residual exposure is that membership changes published during the downtime are not
applied until the daemon returns. With the statusz poll interval at 5 min, a restart window
is small relative to the polling cadence itself.

This lowers the stakes of getting the policy wrong, but it does not make version-coupled
restarts desirable — it argues for a moderate policy rather than a paranoid one.

## Acceptance

- [ ] The CLI declares a supported daemon version range in one authoritative place.
- [ ] A CLI upgrade whose declared range still admits the running daemon performs no daemon
binary replacement and no restart.
- [ ] A CLI upgrade whose declared range excludes the running daemon replaces the binary and
restarts, logging which bound was violated.
- [ ] An explicit operator-invoked path upgrades the daemon regardless of range.
- [ ] `daemon service check` reports running version, on-disk version, required range, and
pending-restart state.
- [ ] The catalog auto-download path can deliver a newer daemon when one is required
(i.e. the install gate is no longer a version-agnostic bool).

### Related Issues

* Follow-up from #973 / #974, which introduced on-disk daemon replacement during CLI
self-install and surfaced the absence of an upgrade policy.

Contributor guide

Open the contributing guide

Research direction

Start with ensureBlockNodeDaemon in cmd/cli/commands/block/node/daemon_offer.go, InstallDaemonBinaryStep and daemonVersionOutput in internal/workflows/steps/step_daemon.go, and the catalog entry in pkg/software/infrastructure-catalog.yaml. Read pkg/semver.CheckVersionRequirements and pkg/software/base_installer.go, then trace daemon service check and upgrade paths. Done means the acceptance checklist passes, including range-aware installation, explicit upgrades, restart decisions, and skew reporting.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
cli, devops, infrastructure
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.