Skip to content

feat(metrics): bound influxdb and prometheus retention, and keep the history in ceph s3 - #1439

Merged
github-actions[bot] merged 9 commits into
developfrom
jim.lin/feat/replace-monasca-2
Sep 9, 2026
Merged

github-actions[bot] merged 9 commits into
developfrom
jim.lin/feat/replace-monasca-2

Conversation

@Eandalf-Bigstack

@Eandalf-Bigstack Eandalf-Bigstack commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

What type of PR is this?

  • feature
  • bug

What this PR does / why we need it

A metrics database should not be able to fill the system partition, and it should not lose history when it is capped. This PR does both halves of that, and fixes the credential collision that surfaced while wiring the second half up.

  • Bound both time-series stores. InfluxDB kept 364d + 35d on the def retention policy — worst case an InfluxDB 1.x shard survives duration + shardGroupDuration, because whole shard groups are dropped and a group only expires once its end time plus the duration has passed. Prometheus kept 90d with no size cap at all. Both are now tunable in hex_config rather than compiled-in: def drops to 14d/7d, hc to 7d/2d, prometheus to 30d with a 5 GiB cap
  • Stop throwing the capped history away. A 30d cap only makes sense if what falls off the end still exists somewhere, so the thanos sidecar now ships finished blocks to a ceph RGW bucket, a store gateway serves them back to the querier, and a compactor downsamples and enforces 90d retention in the bucket. Retention is thanos's, not an RGW lifecycle rule: expiry that thanos does not know about leaves dangling block metadata behind
  • Give cube-cos-api its own RGW identity. The API's accessKey setting is a keystone user name, not an S3 key, and it defaulted to admin — so it minted an EC2 credential whose access key is literally "admin", while sdk_health and cube_cluster_start_cluster list admin's credentials, cache the first in /run/ec2.key and delete/recreate on an empty read. ec2 credentials delete admin removes exactly the credential the API created. This is the residue of [Bug]: s3 credential secret key is empty, make Health page detail log Access Denied #703

Which issue(s) this PR fixes

Special notes for your reviewer

Why the two InfluxDB numbers add up. They are not two windows, they are duration and shard-group duration. InfluxDB 1.x never deletes points, only whole shard groups, and a group's expiry_time is its end time plus the retention duration — so the oldest point still on disk can be duration + shardGroupDuration old. 364d/35d was a worst case of 399d. The new pairs keep that arithmetic visible: 14d/7d and 7d/2d.

Why prometheus retention moved into the config file. 3.13 marks both --storage.tsdb.retention.time and .size [DEPRECATED] and points at the storage.tsdb.retention block, and a flag set on the command line takes precedence over that block — which would silently pin the value and make the tunings look inert. This is 3.x-only: 2.55 does not know the field and refuses to parse rather than ignoring it, which took all three accept-3cc nodes to activating when it was tried there before the upgrade.

The sidecar will not upload unless prometheus stops compacting. --storage.tsdb.max-block-duration must equal --min-block-duration. Prometheus derives max as 10% of retention when unset, so this is not a constant that could be left alone — cutting retention to 30d moved it to 3d and would have silently stopped every upload. The sidecar validates this itself and refuses loudly ("found that TSDB Max time is 3d and Min time is 2h. Compaction needs to be disabled") rather than shipping bad blocks.

Why the compactor is a pacemaker resource and not a systemd unit. Two compactors against one bucket corrupt it, so exactly one must run cluster-wide — the same shape as cinder-volume. That leaves a bootstrap window where pacemaker exists but the compactor has no config, so config_pacemaker masks it before bringing pacemaker up and config_prometheus unmasks it once the config is written. The openvswitch mask moves out of bootstrap_cube_config into the same place.

The mask is gated on IsBootstrap(), and that qualifier is the whole safety of the handshake. The mask lives in config_pacemaker while both unmasks live in the modules that own the units — openvswitch in config_neutron's OvnService, thanos-compact in config_prometheus — and those modules' CommitCheck predicates are not this one's:

commits pacemaker but NOT neutron     pacemaker.modified | G_MOD(MGMT_IF)
commits pacemaker but NOT prometheus  pacemaker.modified | G_MOD(MGMT_IF) | G_MOD(IS_MASTER)

Without the gate, a settings commit carrying any of those masks both units and nothing ever unmasks them. A tuning change is applied cluster-wide, and it lands on exactly this path: cube-cos-ui to cube-cos-api, whose ApplyTuning() runs hex_config apply <dir> on each node, which translates the policy and then spawns hex_config commit <settings> — a settings commit, so IsBootstrap() is false. One tuning change therefore masks openvswitch on every control and compute node at once, compute included because the mask sits above the if (enabled) block. A mask does not stop a running daemon, so nothing looks wrong at the time; what it does is arm every later restart to fail, the repair paths and the next boot included. Every CommitCheck in the pass short-circuits to true under IsBootstrap(), so that is exactly the set of commits where both unmasks are guaranteed to follow — and it is the only path where the window exists at all, since on any later commit both units are already configured. Caught in review by @SekiXu; see the verification below.

Why the endpoint is passed to the sdk rather than discovered. os_endpoint.snapshot is the obvious source and unusable at commit time — its only commit-path writers are cube_last (six levels after prometheus) and keystone, which skips it on a master's first bootstrap. Measured on accept-3cc, the file's mtime is 42 minutes after boot. Its public row also resolves to EXTERNAL, the wrong side of the appliance for a control-plane upload.

Why a separate keystone user is enough for the shared log bucket. rgw runs with rgw keystone implicit tenants = false, so the S3 owner is the keystone project, not the user — and radosgw-admin bucket stats --bucket=log reports its owner as the admin project id on both clusters. A user with an rgw-accepted role on that project authenticates as the same S3 owner, so no bucket policy or ACL is involved. member suffices; the role is not what grants the access.

Why hex_config owns that credential. cube-cos-api creates it on startup but treats an HTTP 409 as success. Without reconciliation, the first time the configured secret changes — a new seed, or the api.s3.secret tuning — keystone would keep the old secret while the API signed with the new one, and every request would be denied with nothing in either log saying why.

Additional documentation

Verified on accept-3cc (3-node HA) and jim-1cc (single control) unless stated otherwise.

For the requirement "InfluxDB is bounded", applied live after the tunings landed:

jim-1cc     125M -> 58M
accept-3cc  cc1 279M -> 122M    cc2 316M -> 147M    cc3 265M -> 124M

For the requirement "prometheus honours both caps", from prometheus's own startup log rather than the flags:

level=INFO msg="Time-based retention policy" duration=30d
level=INFO msg="Size-based retention policy" size=5GiB

accept-3cc was upgraded 2.55.1 -> 3.13.1 first, since the config-file form does not exist in 2.x.

For the requirement "blocks reach S3 and are readable back", on all three control nodes:

[cc1 ~]# systemctl is-active prometheus thanos-sidecar thanos-store thanos-query
active active active active
[cc1 ~]# curl -s http://10.1.0.1:10904/prometheus/api/v1/stores | jq -r ...
  sidecar  10.1.0.1:10901   err=None
  sidecar  10.1.0.2:10901   err=None
  sidecar  10.1.0.3:10901   err=None
  store    127.0.0.1:10905  err=None
[cc1 ~]# curl -s http://10.1.0.1:10906/metrics | grep 'blocks_meta_synced{state="loaded"}'
thanos_blocks_meta_synced{state="loaded"} 12

Zero "Compaction needs to be disabled" messages and thanos_shipper_upload_failures_total 0 on every node.

For the requirement "the compactor is a singleton and does real work":

[cc1 ~]# pcs status resources | grep thanos-compact
  * thanos-compact   (systemd:thanos-compact):  Started cc1
[cc1 ~]# hex_sdk cmd -c -v "systemctl is-active thanos-compact"
cc1|0|active
cc2|3|inactive
cc3|3|inactive

A full cycle in its log — initial sync, garbage collection, compaction, both downsampling passes, retention apply — and the bucket shows the result: three level-2 blocks each spanning 6h with 1,344,240 samples, exactly 3 x 448,080, i.e. three consecutive 2h level-1 blocks merged. 18 blocks, 10,740,568 samples, continuous coverage with no gap.

For the requirement "nothing is offered on an interface that does not need it", every listener this module writes was audited; none is left on 0.0.0.0:

9091  prometheus   127.0.0.1 + mgmt   sidecar/self-scrape/non-HA haproxy are local; health check is cross-node
10901 sidecar grpc mgmt               every node's querier dials it
10902 sidecar http loopback           nothing reads it
10903 query grpc   loopback           no querier fan-out, no thanos-rule
10904 query http   mgmt               haproxy backend, and the health check
10905 store grpc   loopback           only that node's own querier
10906 store http   mgmt               the health check probes across nodes
10907 compact http loopback           nothing reads it; the singleton is established through systemd

Confirmed on all three nodes: the four cross-node paths answer, the four loopback-only ports are refused off-box, the self-scrape is up, and grafana's route through the VIP (https://<vip>/prometheus/graph) returns 200.

For the requirement "commit order is correct", from hex_config -d on the built binary:

L9   pacemaker    <- masks openvswitch and thanos-compact, then starts pacemaker
L11  ceph         <- radosgw up
L12  prometheus   <- ceph cube_scan first        (was L8 without CONFIG_REQUIRES(prometheus, ceph))
L13  neutron      <- unmasks openvswitch
L16  api          <- apache2 cube_scan cyborg first influxdb keycloak keystone mongodb
L18  pacemaker_last <- creates the pcs resources

For the requirement "a settings commit cannot leave openvswitch masked" — an A/B of two binaries built from the same source, one with the IsBootstrap() gate and one without, each running the identical non-bootstrap, pacemaker-only commit on jim-1cc. WriteLogRotateConf is Commit()'s last statement, so /etc/logrotate.d/pacemaker witnesses that the module ran rather than returning at its CommitCheck:

[cc1 ~]# hex_config commit <settings with one added pacemaker.enabled line> pacemaker
without the gate   witness written   openvswitch/thanos-compact -> masked/masked
with the gate      witness written   openvswitch/thanos-compact -> unchanged

The delta has to be recreated from a pristine settings copy before each run: a commit writes its settings back, so a second run finds nothing modified, returns at CommitCheck and proves nothing — the first attempt at this measured exactly that false pass.

What the leaked mask costs, measured with both units masked:

[cc1 ~]# systemctl restart openvswitch
Failed to restart openvswitch.service: Unit openvswitch.service is masked.
[cc1 ~]# systemctl start thanos-compact
Failed to start thanos-compact.service: Unit thanos-compact.service is masked.
[cc1 ~]# systemctl is-active ovs-vswitchd
active                    # <- still up, which is why this would go unnoticed

For the requirement "health notices when either new component fails":

[cc1 ~]# hex_sdk health_thanos_check ; echo $?
0
[cc1 ~]# ssh cc3 systemctl stop thanos-store ; hex_sdk health_thanos_check ; echo $?
5
[cc1 ~]# hex_sdk health_thanos_repair ; hex_sdk health_thanos_check ; echo $?
0
[cc1 ~]# pcs resource disable thanos-compact ; hex_sdk health_thanos_check ; echo $?
7

For the requirement "cube-cos-api no longer shares admin's credential" — jim-1cc, converge from no user at all, then idempotence, then rotation:

[cc1 ~]# openstack user delete cube-cos-api
[cc1 ~]# hex_sdk api_s3_user_setup cube-cos-api SECRET_ONE ; echo $?
0
    access=cube-cos-api secret=SECRET_ONE     role on admin project: member
[cc1 ~]# hex_sdk api_s3_user_setup cube-cos-api SECRET_ONE     # credential id unchanged
[cc1 ~]# hex_sdk api_s3_user_setup cube-cos-api SECRET_TWO     # rotation
    old secret rejected: yes    new secret works: ok    credentials: 1

And the isolation that #703 was about — the delete that used to break the API:

[cc1 ~]# openstack ec2 credentials delete admin
[cc1 ~]# openstack credential list --user cube-cos-api --type ec2 -f value -c ID | wc -l
1
[cc1 ~]# systemctl is-active cube-cos-api
active

On accept-3cc, run concurrently from all three control nodes it still converges to exactly one credential, all three reach s3://log, all three API instances come up active, and health_api_check returns 0. Reaching that state needed keycloak repaired first: its realm was half-migrated behind the XA abort described in 92e9520a, which is a pre-existing fault on that cluster and not something this PR touches. Two details worth recording — a StatefulSet RollingUpdate will not replace a pod-0 that never becomes Ready, so the XA env var sat in the statefulset while the pod ran without it until the pod was deleted; and once MIGRATION_MODEL is empty while the master realm already holds its default client scopes, no amount of restarting recovers it.

@SekiXu SekiXu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thanos-compact-as-pacemaker-singleton shape looks right, and gating the pcs resource on
if (ha) while config_prometheus only unmasks under thanosEnabled keeps non-HA free of a
resource pointing at a masked unit.

One thing about the mask/unmask handshake: the two halves live in modules whose CommitCheck()
predicates are not the same, so a commit can mask without ever unmasking.

pacemaker: modified | s_bCubeModified | s_bNetModified | G_MOD(IS_MASTER)
           | G_MOD(MGMT_IF) | G_MOD(MGMT_ADDR) | G_MOD(SHARED_ID)
neutron:   modified | s_bMqModified | s_bNovaModified | s_bCubeModified
           | s_bKeystoneModified | s_bNetModified | G_MOD(IS_MASTER)
           | G_MOD(MGMT_ADDR) | G_MOD(SHARED_ID) | G_MOD(OVERLAY_ADDR)
           | G_MOD(PROVIDER_IF)

pacemaker's own modified - i.e. pacemaker.enabled itself changed - is not in neutron's set.
On that commit config_pacemaker::Commit() masks openvswitch and config_neutron::Commit()
returns at its CommitCheck before reaching OvnService(), which is the only
systemctl unmask openvswitch in the tree (config_neutron.cpp:620). The mask does not stop a
running daemon, but until the next full commit any systemctl restart openvswitch fails,
including the repair paths - and since the mask sits above the if (enabled) block it lands on
compute nodes too. thanos-compact has the same shape against config_prometheus's narrower
predicate, though there the consequence is only a stopped compactor.

IsBootstrap() short-circuits both, so the bootstrap path this replaces is unaffected - which is
why gating the mask on IsBootstrap(), or pairing it inside the if (enabled) block right
before pacemaker is started, keeps the window you want to close and drops the rest.

No comment on the secret in api_s3_user_setup's argv or the timeout 0 - both are how the tree
already does it (config_barbican.cpp:135 and config_cyborg.cpp:136 pass passwords the same
way, and HexUtilSystemF(0, 0, ...) is 452 of 454 call sites in core/modules).

Eandalf-Bigstack and others added 8 commits September 8, 2026 23:44
… to 14d

InfluxDB kept 364 days of metrics on the system partition -- the same
partition as the OS, ceph and prometheus -- with a 35-day shard group. Because
InfluxDB never deletes individual points but drops whole shard groups, and only
once the entire group is older than the duration, a point written at the start
of a group survived duration + shard duration: up to 399 days, reclaimed five
weeks at a time. Nothing else bounded it; the influx-curator cron only drops
series belonging to VMs that no longer exist.

The four durations become TUNING_PUB tunings so a cluster can be sized without
a rebuild, defaulting to def 14d/shard 7d and hc 7d/shard 2d. Duration and shard
duration are exposed and documented as a pair on purpose -- shortening the
duration alone leaves the old shard width, which caps how tight the real bound
can get and keeps freeing disk in cliffs. The new pairs give a worst-case point
age of 21d for def and 9d for hc, against 399d before.

The two policies are a cardinality split, now written down in the code: 'def'
carries series whose count scales with node count (host cpu/mem/disk, ceph_*),
'hc' carries sflow and vrouter.top, whose count scales with traffic -- one
series per flow tuple -- routed there by telegraf's namepass and logstash's
telegraf-hc-persister writing ?rp=hc. That is why the two get very different
retention.

CreateDBs clamps a shard duration wider than its policy and logs it, rather
than letting the commit fail: InfluxDB rejects such a policy, and one that got
through could never drop anything.

UPGRADE NOTE. On the first commit after this change, expiry is recalculated as
end_time + the new duration, and any shard group already past it is dropped
WHOLE -- so an operator loses up to a shard-width more history than the new
number suggests, in one step. On the test clusters that discarded a full 35-day
group rather than only the excess over 14 days. Conversely the new shard width
applies only to groups created after the change, so until the current group
rolls over, def still holds up to 49 days despite reading 14d.

Verified on jim-1cc and all three accept-3cc nodes: policies read
def=336h/shard=168h and hc=168h/shard=48h, no data older than 14d remains, and
/var/lib/influxdb went 125M->58M on jim-1cc and 279M->122M, 316M->147M,
265M->124M on accept-3cc -- about 54% on every node. Compiles and links clean in
the build jail with all four tuning names present in the object.

Signed-off-by: Jim Lin <jim.lin@bigstack.co>
Co-authored-by: Eandalf <clinah@connect.ust.hk>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Retention was a hardcoded 90d with no size limit at all
(storage.tsdb.retention.size=0B), which is the same unbounded shape influxdb had
at 364d and on the same system partition -- shared with the OS, ceph and the
influx TSDB. Time alone does not bound disk: if cardinality grows, 90d of series
is however large it turns out to be.

Both limits are now TUNING_PUB tunings, prometheus.rp.duration (30 days) and
prometheus.rp.size (5 GiB), so a cluster can be sized without a rebuild. Both
apply and whichever is reached first wins. Size 0 omits the setting entirely,
which is prometheus's own way of saying no size limit -- writing 0 would cap
storage at zero. Prometheus counts in powers of two, so the 5GB we emit is
5 GiB, and it echoes it back as '5GiB' in its startup log.

Written into prometheus.yml's storage.tsdb.retention block rather than onto
--storage.tsdb.retention.time/.size. 3.13 marks both flags [DEPRECATED] and
names that block as the replacement, and a flag would take precedence over the
block -- silently pinning the value and making the tunings look ineffective.
The block requires prometheus 3.x: a 2.x prometheus does not ignore the unknown
field, it refuses to parse the file and never starts. This branch ships 3.13
from EPEL, so that case cannot arise.

Verified on jim-1cc and all three accept-3cc nodes, both on 3.13.1: prometheus
logs 'TSDB retention updated duration=30d size=5GiB', the service is active with
Restart=always and LimitNOFILE=65536, every scrape target including the
self-scrape reports up, and thanos still sees all three sidecars.

Signed-off-by: Jim Lin <jim.lin@bigstack.co>
Co-authored-by: Eandalf <clinah@connect.ust.hk>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sidecar uploads blocks to object storage but nothing read them back:
the querier could only see what each prometheus still held locally, so
anything past the local retention window was unreachable despite being
safely in S3. thanos-store closes that gap.

thanos-compact enforces retention and downsampling in the bucket. Its unit
is installed but deliberately never enabled: two compactors against one
bucket corrupt it, so placement belongs to pacemaker, which runs exactly
one instance cluster-wide -- the same arrangement as cinder-volume.

Both run as prometheus, like the sidecar: no new system user, and they read
the same objstore credentials file. /var/lib/thanos/{store,compact} are
created and owned at rootfs install time.

Verified on accept-3cc: store active on all three nodes with
thanos_blocks_meta_synced{state="loaded"}=12 each, and the querier lists
the store gateway alongside all three sidecars.

Signed-off-by: Jim Lin <jim.lin@bigstack.co>
Co-authored-by: Eandalf <clinah@connect.ust.hk>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
thanos needs a bucket, credentials and an objstore.yml before any of its
components can start. That is radosgw-admin and s3cmd work, so it belongs in
the sdk rather than in config_prometheus; it is idempotent and runs on every
commit.

The rgw user is deliberately a native radosgw user rather than a keystone EC2
credential. The admin EC2 credential is contested: sdk_health's log-upload path
deletes and recreates it while cube-cos-api recreates a deterministic one of
its own, so each rotates the other out and anything caching the pair breaks. A
native user is invisible to both and cannot be rotated out from under thanos.
os_s3_bucket_quota already drives radosgw-admin directly, so native users
alongside keystone-mapped ones are an established arrangement here.

The endpoint is a required parameter, not discovered. os_endpoint.snapshot is
the obvious source and the wrong one twice over: it is not written until
cube_last, six commit levels after prometheus, and a master's first bootstrap
skips it entirely, so at commit time it is routinely absent -- measured on
accept-3cc, its mtime is 42 minutes after boot. Its "public" row also resolves
to EXTERNAL, the wrong side of the appliance for a control-plane upload.
config_prometheus already holds the control vip, so it passes <vip>:8888, the
same internal endpoint config_swift publishes; that needs only rgw (ceph, L11)
and haproxy's radosgw_proxy (L10), both already ahead of prometheus at L12.
Keeping the sdk a pure consumer of that value avoids a second source of truth
that could drift from the caller's.

Verified on accept-3cc: endpoint with bucket and endpoint alone both produce
bucket thanos at 10.1.0.100:8888; a call with no endpoint returns 1, writes no
config and does not touch rgw. thanos-store, thanos-sidecar and thanos-query
restart clean against the result, health_thanos_check passes, and all three
sidecars report zero upload failures.

Signed-off-by: Jim Lin <jim.lin@bigstack.co>
Co-authored-by: Eandalf <clinah@connect.ust.hk>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes items 5 and 6 of the issue 672 comment. Prometheus keeps 30d locally
under a 5GiB cap; everything the sidecar has finished with now lives in a ceph
RGW bucket instead of being dropped, and 90d of it stays queryable.

Three pieces:

- The sidecar gains --objstore.config-file, so finished blocks are uploaded.
  This also requires prometheus's own compaction to be off:
  max-block-duration = min-block-duration = 2h. The sidecar validates that
  itself and refuses loudly rather than shipping bad blocks -- observed as
  "found that TSDB Max time is 3d and Min time is 2h. Compaction needs to be
  disabled". Prometheus derives max as 10% of retention when unset, so this is
  not a constant that could be left alone: the 30d retention change moved it to
  3d and would have silently stopped every upload.
- thanos-store is configured and enabled, and added to the querier's endpoint
  list. Without it the querier only ever sees what each prometheus still holds
  locally, so anything past 30d would be unreachable despite being in S3.
- thanos-compact is configured but deliberately not enabled here. Two
  compactors against one bucket corrupt it, so pacemaker owns placement.
  Unmasking it once its config exists is this module's half of the handshake
  config_pacemaker starts.

Retention in the bucket is a new tunable, prometheus.thanos.rp.duration,
defaulting to 90 days and applied to all three resolutions so the number means
"90d of history" whatever it has been downsampled to. It is enforced by the
compactor rather than by an RGW lifecycle rule: expiry that thanos does not
know about leaves dangling block metadata behind.

CONFIG_REQUIRES(prometheus, ceph) is what makes any of this work at bootstrap.
thanos_objstore_setup needs a live radosgw to create its user and bucket, and
without the dependency prometheus commits at L8 -- verified against hex_config
-d, cube_scan is L7 -- three levels ahead of ceph at L11. It now commits at L12.
The endpoint passed to the sdk is <shared_id>:8888 rather than something
discovered, because os_endpoint.snapshot is not written until cube_last.

While adding these listeners, every port this module writes was audited and
none is left on 0.0.0.0. A port binds the management address only where
something on another node calls it, and loopback otherwise:

  9091  prometheus   127.0.0.1 + mgmt  sidecar, self-scrape and the non-HA
                                       haproxy backend are local; the health
                                       check probes across nodes. The flag is
                                       documented "Can be repeated".
  10901 sidecar grpc mgmt              every node's querier dials it
  10902 sidecar http loopback          nothing reads it
  10903 query grpc   loopback          no querier fan-out, no thanos-rule
  10904 query http   mgmt              haproxy backend, and the health check
  10905 store grpc   loopback          only that node's own querier
  10906 store http   mgmt              the health check probes across nodes
  10907 compact http loopback          nothing reads it; the singleton is
                                       established through systemd, not HTTP

The compactor's scratch dir is /var/lib/thanos/compact, on the root filesystem
rather than /store: /store is the cross-version upgrade share, not a working
directory. thanos has no flag capping that directory; what bounds it is the
concurrency defaults, one compaction group at a time.

Verified on accept-3cc (3 control nodes): bucket created, objstore.yml valid on
all three, prometheus/sidecar/store/query all active, the querier lists three
sidecars and the store gateway with no error, zero "Compaction needs to be
disabled" messages and zero upload failures. The compactor completed full
cycles as a pacemaker singleton, producing level-2 blocks that each merge three
consecutive 2h level-1 blocks. Every cross-node path answers on all three nodes
and every loopback-only port is refused off-box; the self-scrape is up on all
three and grafana's route through the VIP returns 200. Verified on jim-1cc that
the non-HA path is unaffected: no thanos units active, prometheus healthy.

Signed-off-by: Jim Lin <jim.lin@bigstack.co>
Co-authored-by: Eandalf <clinah@connect.ust.hk>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two compactors against one thanos bucket corrupt it, so exactly one must run
cluster-wide. That is placement, which is pacemaker's job -- the same shape as
cinder-volume: a plain systemd resource with no colocation, so pacemaker runs
it wherever it can.

The unit is never enabled in systemd, or every control node would start one.
That leaves a window during bootstrap where pacemaker exists but the compactor
has no configuration yet, so this masks it before bringing pacemaker up and
config_prometheus unmasks it once it has written the config. The masking
precedent is openvswitch, which has used exactly this handshake; the resource
itself is created from CommitLast at L18, well after both unmasks, so by the
time pacemaker is told about thanos-compact the unit exists and is runnable.
Commit order, from hex_config -d: pacemaker L9 masks, prometheus L12 unmasks
thanos-compact, neutron L13 unmasks openvswitch, pacemaker_last L18 creates
the resources.

failure-timeout="60s" matters here specifically. Because the resource is
masked when pacemaker first comes up, pacemaker will see it fail a few times on
the way through a bootstrap, and without the timeout those failures never age
out and the resource stays stopped. vaw already carries the same meta for the
same reason.

The openvswitch mask moves here from the top of bootstrap_cube_config so both
masks live in one place. It is not a behaviour change on any path that matters:
bootstrap_cube_config is a PROJ_BOOTSTRAP script and ran the mask on every boot
whether or not a commit followed, while pacemaker and corosync are disabled in
systemd -- verified on accept-3cc -- so nothing starts pacemaker except the
hex_config commit pass that now performs the mask a few lines earlier. Commit()
has already returned for any role other than control or compute, which is
exactly the set config_neutron's OvnService unmasks openvswitch for, so the
mask and the unmask cover the same nodes; the old unconditional mask in the
boot script could leave other roles masked with nothing to unmask them.

Verified on accept-3cc: pacemaker places thanos-compact on cc1 with cc2 and cc3
inactive, systemd reports it disabled on all three, and it logs complete
compaction cycles -- initial sync, garbage collection, compaction, both
downsampling passes and retention apply. Stopping it cluster-wide is reported
by health_thanos_check as error 7 and recovers on pcs resource enable. The
resource is created inside the HA branch of SetupCluster, so a non-HA node
never creates it and simply leaves the unit masked.

Signed-off-by: Jim Lin <jim.lin@bigstack.co>
Co-authored-by: Eandalf <clinah@connect.ust.hk>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The existing thanos check covered the sidecar and the querier. Neither notices
the two components this branch adds, and both fail silently.

Errors 5 and 6 cover the store gateway, per node. It is what makes anything
past local prometheus retention readable at all: with it down the querier still
answers every existing probe, just from whatever is still on disk, so a cluster
can quietly lose all its history in object storage while reporting healthy.
Repair restarts it, alongside the sidecar and querier restarts already there.

Error 7 covers the compactor, and is checked cluster-wide rather than per node
because pacemaker places exactly one instance anywhere. Both directions are
faults: two would corrupt the bucket, none means retention and downsampling
have silently stopped and the bucket grows without bound.

The compactor's repair deliberately does not restart the unit. Starting it
directly would either fight pacemaker or, worse, bring up a second compactor
against the bucket while pacemaker still believes its own instance is running
-- the exact corruption the singleton exists to prevent. It asks pacemaker to
replace the instance instead, with pcs resource cleanup, and only from the
first control node so three nodes do not issue it at once.

Verified on accept-3cc: with everything healthy the check returns 0. Stopping
thanos-store on one node returns 5 and health_thanos_repair restores it, with
the recheck back to 0. Disabling the compactor cluster-wide returns 7 and it
clears once pacemaker is allowed to place it again. The cluster-wide count is
taken with the sdk's own cmd -c -v, which reports cc1|0|active against cc2 and
cc3 inactive.

Signed-off-by: Jim Lin <jim.lin@bigstack.co>
Co-authored-by: Eandalf <clinah@connect.ust.hk>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
cube-cos-api and hex_sdk were fighting over one EC2 credential. The API's
accessKey setting is a keystone USER NAME, not an S3 key, and it defaulted to
"admin", so newBucketSecret() minted a credential whose access key is literally
"admin" for the admin user. sdk_health and cube_cluster_start_cluster then list
admin's credentials, cache the first in /run/ec2.key and delete/recreate on an
empty read -- so `ec2 credentials delete admin` removes exactly the credential
the API created, and whichever side runs next silently adopts or destroys the
other's key. Observed on jim-1cc, where the only credential in the cluster was
the API's and hex_sdk was using it. Issue #703.

The API now gets its own keystone user. Nothing in cube-cos-api changes: it
already reads accessKey and secretKey from spec.resourceControl.aws, and the
template simply stops leaving them at their defaults.

Access to the shared "log" bucket survives the split for free. rgw runs with
`rgw keystone implicit tenants = false`, so the S3 owner is the keystone PROJECT
rather than the user, and `radosgw-admin bucket stats --bucket=log` reports its
owner as the admin project id on both test clusters. A user holding an
rgw-accepted role on that project therefore authenticates as the same S3 owner,
so no bucket policy or ACL is involved. "member" is enough; the role is not what
grants the access.

hex_config also owns the credential rather than leaving it to the API, which
creates it on startup but treats an HTTP 409 as success. Without this, the first
time the configured secret changes -- a new seed, or the api.s3.secret tuning --
keystone would keep the old secret while the API signed with the new one, and
every S3 request would be denied with nothing in either log explaining why.
api_s3_user_setup reconciles instead: it leaves a matching credential alone and
only deletes and recreates one that is absent or stale, which also makes it safe
to run from every control node.

It lives in sdk_api.sh rather than under os_ because the credential shape is the
API's own -- the access key must equal the user name -- not a general one.

Verified on jim-1cc: converges from no user at all, is idempotent (the credential
id does not change on a re-run), and rotates correctly -- after a new secret the
old one is rejected, the new one works, and exactly one credential remains. The
API comes up active against it and round-trips an object through s3://log, while
admin's own credential and hex_sdk's upload path are untouched.

Verified on accept-3cc: run concurrently from all three control nodes it still
leaves exactly one credential, all three reach the log bucket, all three API
instances come up active, and health_api_check returns 0. (Reaching that state
needed keycloak repaired first -- its realm was half-migrated behind the XA
abort 92e9520 describes -- which is a separate pre-existing fault on that
cluster, not something this change touches.)

Signed-off-by: Jim Lin <jim.lin@bigstack.co>
Co-authored-by: Eandalf <clinah@connect.ust.hk>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Eandalf-Bigstack
Eandalf-Bigstack force-pushed the jim.lin/feat/replace-monasca-2 branch from 7c7f07c to 21e47d7 Compare September 8, 2026 15:45
…path only

The mask half of the handshake lived in this module, unconditionally, while both
unmask halves live in the modules that own the units -- openvswitch in
config_neutron's OvnService (L13), thanos-compact in config_prometheus (L12).
Those modules' CommitCheck predicates are not this one's, so a commit could mask
without anything ever unmasking:

  commits pacemaker but not neutron     pacemaker.modified | G_MOD(MGMT_IF)
  commits pacemaker but not prometheus  pacemaker.modified | G_MOD(MGMT_IF)
                                        | G_MOD(IS_MASTER)

So changing a pacemaker tunable on its own -- or the management interface, or the
master flag -- left both units masked. A mask does not stop a running daemon, so
nothing breaks at the time; what breaks is every later restart. `systemctl
restart openvswitch` fails for as long as the mask stands, the repair paths
included, and because the mask sat above the `if (enabled)` block it landed on
compute nodes too. For thanos-compact the cost is a compactor pacemaker can no
longer start.

Gating on IsBootstrap() is what makes the handshake safe rather than lucky: every
CommitCheck in the pass short-circuits to true under IsBootstrap(), so that is
exactly the set of commits where both unmasks are guaranteed to follow. It is
also the only path where the window exists -- on any later commit both units are
already configured, so there is nothing to protect them from. This restores the
scope the mask had at the top of bootstrap_cube_config, a PROJ_BOOTSTRAP script,
before it moved into this module, with the one difference that Commit() has
already returned for any role other than control or compute -- the same set
OvnService unmasks openvswitch for.

Reported by SekiXu in review on #1439, who derived the whole path from the two
predicates and noted that IsBootstrap() short-circuits both, which is what makes
this the minimal fix. Pairing the mask inside the `if (enabled)` block instead
would not have closed it: a commit that flips pacemaker.enabled to true still
masks and still never reaches an unmask.

Verified on jim-1cc as an A/B of two binaries built from this same source, one
with the guard and one without, each running the identical
`hex_config commit <settings> pacemaker` -- a non-bootstrap commit of the
pacemaker module alone, which is the reviewer's case exactly. WriteLogRotateConf
is Commit()'s last statement, so /etc/logrotate.d/pacemaker is the witness that
the module ran rather than returning at its CommitCheck; the delta was recreated
from a pristine settings copy before each run, because a commit writes its
settings back and the second run would otherwise find nothing modified and prove
nothing.

  without the guard   witness written, openvswitch/thanos-compact -> masked/masked
  with the guard      witness written, openvswitch/thanos-compact -> unchanged

Also measured what the leaked mask costs: with both units masked, `systemctl
restart openvswitch` fails with "Unit openvswitch.service is masked" and
thanos-compact will not start, while ovs-vswitchd stays active -- which is why
this would have gone unnoticed. Node left with both units unmasked, settings and
pacemaker enablement as found, ovs-vswitchd and ovn-controller active, br-int
and provider present, and the management IP held.

Refs #1439

Signed-off-by: Jim Lin <jim.lin@bigstack.co>
Co-authored-by: Eandalf <clinah@connect.ust.hk>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Eandalf-Bigstack

Copy link
Copy Markdown
Collaborator Author

@SekiXu thank you — and this one deserves more than a routine thanks. You stopped a routine cluster-wide tuning change from turning into a cluster-wide network blackout, with openvswitch masked on every node at once. Fixed in 3b56128.

I traced the path end to end after your comment, and it lands exactly where you said it would:

cube-cos-ui -> cube-cos-api  ApplyTuning()
             -> hex_config apply <dir>        (per node)
             -> hex_config commit <settings>  <- a settings commit, so IsBootstrap() is false
             -> config_pacemaker::Commit()    <- masks openvswitch + thanos-compact
                config_neutron::Commit()      <- returns at CommitCheck, never reaches OvnService
                config_prometheus::Commit()   <- same

So the trigger is not an exotic one. It is the normal way a tuning gets applied, it runs on every control and compute node — compute included, because as you noted the mask sits above the if (enabled) block — and every one of those nodes ends up with openvswitch masked and nothing left in the pass to unmask it.

The part that makes it nastier than it first looks is that it is silent. A mask does not stop a running daemon, so the cluster keeps forwarding and nothing looks wrong at the time:

[cc1 ~]# systemctl restart openvswitch
Failed to restart openvswitch.service: Unit openvswitch.service is masked.
[cc1 ~]# systemctl start thanos-compact
Failed to start thanos-compact.service: Unit thanos-compact.service is masked.
[cc1 ~]# systemctl is-active ovs-vswitchd
active                    # <- still up, which is why this would have gone unnoticed

What it actually does is arm the blackout: the next thing to restart openvswitch on any of those nodes fails — the repair paths, a rolling restart, or simply the next boot, where a masked unit does not come up at all. A cluster that had been told nothing more than "change this tuning" would then lose its datapath, everywhere, at whatever later moment someone rebooted.

I confirmed your predicate analysis and it holds, with one addition — G_MOD(MGMT_IF) is in pacemaker's set and in neither of the others, so a management-interface change reaches the same place:

commits pacemaker but NOT neutron     pacemaker.modified | G_MOD(MGMT_IF)
commits pacemaker but NOT prometheus  pacemaker.modified | G_MOD(MGMT_IF) | G_MOD(IS_MASTER)

I took your IsBootstrap() suggestion rather than the other one, for the reason your own comment implies: pairing the mask inside the if (enabled) block would not have closed it, since a commit that flips pacemaker.enabled to true still masks and still never reaches an unmask. Gating on IsBootstrap() works because every CommitCheck in the pass short-circuits to true there, so it is exactly the set of commits where both unmasks are guaranteed to follow — and it is the only path where the window exists at all, since on any later commit both units are already configured.

Verified on jim-1cc as an A/B of two binaries built from the same source, one with the gate and one without, each running the identical non-bootstrap pacemaker-only commit:

[cc1 ~]# hex_config commit <settings + one pacemaker.enabled line> pacemaker
without the gate   witness written   openvswitch/thanos-compact -> masked/masked
with the gate      witness written   openvswitch/thanos-compact -> unchanged

WriteLogRotateConf is Commit()'s last statement, so /etc/logrotate.d/pacemaker is the witness that the module really ran rather than returning at its CommitCheck. Worth recording that the first attempt at this measured a false pass: a commit writes its settings back, so the second run of the pair found nothing modified, returned at CommitCheck, and reported "no mask" for entirely the wrong reason. The delta now gets recreated from a pristine settings copy before each run.

One thing you raised that this PR does not change: config_prometheus::CommitCheck() does not include its own modified at all, so a prometheus.* tuning change alone never commits that module. That is pre-existing and independent of the masking bug, so I have left it out of this PR rather than widen it — happy to take it as a follow-up if you think it should move sooner.

Thanks also for clearing the two things you explicitly did not object to — the secret in api_s3_user_setup's argv and the timeout 0 — with the precedents counted. Saved a round trip.

@SekiXu SekiXu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3b56128 closes it, and the reasoning it rests on holds where it has to.

Both unmasks are guaranteed inside a bootstrap pass, which is what the IsBootstrap() gate needs to be true:

  • all three CommitChecks do short-circuit under it (config_pacemaker.cpp:178, config_neutron.cpp:826, config_prometheus.cpp:428)
  • OvnService() is called unconditionally in neutron's Commit() after its own early return (config_neutron.cpp:911), and it unmasks openvswitch for control|compute (config_neutron.cpp:617-621) - the same role set pacemaker's Commit() gates itself on (config_pacemaker.cpp:193), so there is no node where the mask lands and the unmask cannot reach

One residual worth having on the record rather than acted on. thanos-compact's unmask is gated on thanosEnabled = IsControl && s_ha (config_prometheus.cpp:448,486) while the mask runs for control|compute, so the unit stays masked after bootstrap on a non-HA control and on every pure compute node. The non-HA control heals itself the moment it matters: a cubesys ha change sets s_bCubeModified (config_prometheus.cpp:405, IsModifiedTune(1)), which is in prometheus's CommitCheck, so the commit that turns HA on is also the one that unmasks. The compute node keeps the mask, and the compactor resource carries only INFINITY location prefers on the control hosts (config_pacemaker.cpp:104,118) while pure computes join as pacemaker remotes (config_pacemaker.cpp:327), so placement there is possible and would find the unit unstartable rather than transiently failing. That is the same exposure cinder-volume already has one line above, so it is not something this PR introduces.

@Eandalf-Bigstack Eandalf-Bigstack added the done Merge the pull request label Sep 9, 2026
@github-actions
github-actions Bot merged commit 3b56128 into develop Sep 9, 2026
9 checks passed
@github-actions
github-actions Bot deleted the jim.lin/feat/replace-monasca-2 branch September 9, 2026 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

done Merge the pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants