Skip to content

feat(cache): config-server cache write backend - #497

Open
steiler wants to merge 11 commits into
cache-read-backendfrom
cache-write-backend
Open

steiler wants to merge 11 commits into
cache-read-backendfrom
cache-write-backend

Conversation

@steiler

@steiler steiler commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Ticket 07 of the #460 + #471 PR-split reorg
(see graph.md/index.md in the tracking gist for the full stack). Base:
cache-read-backend (#496).

Scope

Real IntentWriter for Cache.Type: config-server: IntentModify/IntentDelete write
through to config-server's ConfigSnapshotService synchronously at apply time (ADR 0003),
closing the ghost-intent race where a deleted intent stayed last-applied until the next
best-effort refresh.

Cross-repo dependency

Depends on sdcio/sdc-protos#135, which renames
ConfigReadService to ConfigSnapshotService and adds Modify/Delete RPCs — a rebase of
the dangling last-applied-snapshot-write-at-apply branch's 5a14ea0 onto this stack's
cache-read-backend tip (sdc-protos#134). go.mod is pinned to that rebased commit; swap for
a tagged release once #123/#134/#135 merge.

What shipped

  • ConfigServerCache gains a configserver.ConfigSnapshotClient port (read+write) and
    implements IntentWriter directly — InstanceIntentModify/InstanceIntentDelete write
    through synchronously, at the same moment TransactionSet's apply loop calls them, matching
    Cache.Type: local's write-at-apply timing.
  • New tree-to-JSON flattener (configsnapshot.DocumentFromIntent) converts an applied
    tree_persist.Intent into the flat Document shape the read path already understands, for
    the wire Modify payload.
  • Fixed the log-only IntentDelete failure in lowlevelTransactionSet: a failed delete-write
    now hard-fails the transaction, matching IntentModify's existing behavior (ADR 0003's
    asymmetry fix).
  • GRPCConfigClient/FakeConfigSnapshotClient implement the collapsed ConfigSnapshotClient
    port (renamed from the read-only GRPCConfigReader/FakeLocalConfigReader); dropped the
    half-split LocalConfigReader/LocalConfigWriter interfaces entirely once nothing was typed
    against either alone.
  • Extracted the last-applied Document interchange (flatten, blob merge, NewImportAdapter) into
    its own pkg/cache/configsnapshot package, so Modify→Get round-trips live in one place;
    pkg/cache/configserver now owns only the remote ConfigSnapshotClient port.
  • Colocated the config-server Namespaced-name codec (splitDatastoreName/lookupConfigName)
    next to ConfigServerCache in pkg/cache/configserver_names.go.
  • Removed the now-dead noopIntentWriter and the dual NewConfigServerClient wrapper —
    Server.createConfigServerCacheClient composes cache.NewConfigServerCache directly.
  • Tests: seam-level modify/delete round trips, malformed-datastore-name coverage for the new
    write callers, and datastore-level regression tests for delete-failure hard-fail and
    rollback-restores-a-deleted-intent (content, not just name).
  • Small independent cleanup: replaced the hardcoded "unknown" owner literals across the four
    YANG validators with a new types.UnknownOwner constant (no behavior change).
  • Committed ADR 0003 (pkg/cache/docs/adr/0003-config-server-write-path-real-last-applied-writes.md)
    and the CONTEXT.md Last-applied/ConfigSnapshotService/Namespaced-name glossary updates.

Adapted from the source branch

  • Cherry-picked 52d842e/7f4f4da/22899e2/6f0d373/b3f52c6 from
    origin/config-server-cache-backend, plus ticket 06's deferred follow-ups 706ecb6/2e8ffc6,
    re-resolving each against this stack's actual, narrower type shapes (ticket 05's
    IntentAdapter/ImportConfigAdapter split, ticket 06's pkg/cache/configserver package
    layout) rather than the source branch's pre-split/pre-relocation originals.
  • Reused the existing buildFixtureIntent test helper from sync_test.go instead of
    duplicating it, and fixed two new IntentGetAll mock expectations to the
    importer.IntentAdapter channel type.
  • Dropped ticket 06's now-superseded TestConfigServerCache_WritesAreNoOps /
    TestCreateConfigServerCacheClient_WritesAreNoOps (writes are real now, not no-ops).
  • Deliberately not cherry-picked: 10652b4/b3ca60d/f0c91db's ghost-exclusion
    mechanism (net-cancelling in the source history; never added to this stack in the first
    place — ticket 05 already skipped 10652b4) and 181394c (superseded — ticket 06 already
    built InstanceCreate's empty-Intent seeding in correctly). b27ab78/15b7efd (net-zero
    lock-retry pair) and 2c28a90/69aab56/6c1bf7d (local-dev cruft / empty CI retriggers)
    also dropped, consistent with prior tickets.
  • Folded only the pkg/cache/-relevant sliver of 67f4bdf (a dead docs/architecture.md
    reference in ADR 0001) rather than that commit's much larger, unrelated repo-wide docs/agent-
    process reorg.
  • 0525b9d (CI-failure investigation notes for a fix that landed in config-server/
    integration-tests, not here) dropped entirely, per the ticket's own note.
  • Dropped .scratch/ planning artifacts, consistent with prior tickets.

Verification

go build ./..., go vet ./..., go test ./... (full repo), and golangci-lint run ./...
all pass.

@steiler
steiler requested a review from a team as a code owner September 23, 2026 11:48
@steiler
steiler added this pull request to stack #494 September 23, 2026 12:32
@steiler steiler mentioned this pull request Sep 23, 2026
@steiler
steiler force-pushed the cache-write-backend branch from d6a3e67 to dac3fa7 Compare October 5, 2026 09:58
@steiler
steiler force-pushed the cache-write-backend branch from dac3fa7 to 2c641e4 Compare October 14, 2026 08:18
@steiler
steiler removed this pull request from stack #494 October 14, 2026 08:20
@steiler
steiler added this pull request to stack #494 October 14, 2026 08:20
@steiler
steiler removed this pull request from stack #494 October 14, 2026 08:21
@steiler
steiler added this pull request to stack #528 October 14, 2026 08:21
@steiler
steiler force-pushed the cache-write-backend branch from 97c7272 to 0577d08 Compare October 14, 2026 09:06
steiler and others added 11 commits October 14, 2026 11:08
…C commit

Pins to sdcio/sdc-protos@2e109b71c1dd — a rebase of the dangling
last-applied-snapshot-write-at-apply branch's ConfigReadService ->
ConfigSnapshotService rename + Modify/Delete RPC commit onto this stack's
cache-read-backend tip (sdc-protos#134). Opened as
sdcio/sdc-protos#135; swap for a tagged release
once #123/#134/#135 merge.

Signed-off-by: Markus Vahlenkamp <markus.vahlenkamp@nokia.com>
IntentModify/IntentDelete now write through to config-server's
ConfigSnapshotService synchronously at apply time, replacing the
noopIntentWriter composition ADR 0001 used. Last-applied tracking under
this backend now matches Cache.Type: local's write-at-apply timing,
closing the ghost-intent race where a deleted intent stayed last-applied
until the next best-effort snapshot refresh.

- ConfigServerCache gains a LocalConfigClient seam (read+write) and
  implements IntentWriter directly; NewConfigServerClient no longer
  composes noopIntentWriter.
- New tree-to-JSON flattener (document_export.go) converts an applied
  tree_persist.Intent into the flat Document shape the read path already
  understands, for GRPCConfigClient.Modify's wire payload.
- Fix the log-only IntentDelete failure in lowlevelTransactionSet: a
  failed delete-write now hard-fails the transaction, matching
  IntentModify's existing behavior (ADR 0003's asymmetry fix).
- GRPCConfigReader/FakeLocalConfigReader renamed to
  GRPCConfigClient/FakeLocalConfigClient; mocks/mockconfigread
  regenerated against the renamed ConfigSnapshotServiceClient.
- Tests: seam-level modify/delete round trips, malformed-datastore-name
  coverage for the new write callers, and datastore-level regression
  tests for delete-failure hard-fail and rollback-restores-a-deleted-
  intent.
- Commit the previously-drafted ADR 0003 and CONTEXT.md Last-applied term
  update.

Adapted from origin/config-server-cache-backend's 52d842e: reused the
existing buildFixtureIntent test helper from sync_test.go instead of
duplicating it, fixed the two new IntentGetAll mock expectations to the
importer.IntentAdapter channel type (ticket 05's IntentAdapter/
ImportConfigAdapter split postdates this commit's original branch), and
dropped ticket06's now-superseded TestConfigServerCache_WritesAreNoOps.

Signed-off-by: Markus Vahlenkamp <markus.vahlenkamp@nokia.com>
Spec review of the real-IntentWriter commit found the delete-no-rehydration
checklist item was backed by a weaker test than described, and the
rollback-restore test only checked the intent's name, not its content.

- Add TestConfigServerBackend_DeleteApply_NoRehydration: wires a real
  *cache.ConfigServerCache over configserver.FakeLocalConfigClient (not a
  generic mock) through two TransactionSet calls and asserts the exact
  regression signature named in the ticket — LoadAllButRunningIntents does
  not rehydrate a deleted intent.
- Strengthen TestTransactionRollback_RestoresDeletedIntent to assert the
  restored leaf content, not just the intent name.
- Clarify ConfigServerCache's ignoreNonExisting doc comment: unused by
  today's LocalConfigWriter implementations, not an inherent design
  guarantee.
- Note in GRPCConfigClient.Modify why the wire payload is plaintext (ADR
  0003's "encrypted payload" describes the persisted record; config-server
  encrypts on receipt, same as northbound SensitiveConfig).

Standards review separately flagged Document<->ConfigEntry mapping
duplicated across grpc.go/document_export.go; added configEntryFromDocument
as the Modify-side counterpart to the existing documentFromEntry to remove
the wire-mapping half of that duplication.

Signed-off-by: Markus Vahlenkamp <markus.vahlenkamp@nokia.com>
Renames doc/comment references from ConfigReadService to
ConfigSnapshotService in pkg/cache/CONTEXT.md, pkg/cache/configserver.go,
pkg/cache/configserver/localread.go, and pkg/config/datastore.go.

Follow-up carried over from ticket 06, which deliberately deferred it until
ConfigSnapshotService actually existed (landed by the real-IntentWriter
commit earlier in this ticket).

Signed-off-by: Markus Vahlenkamp <markus.vahlenkamp@nokia.com>
Drop the unused no-op IntentWriter and dual NewConfigServerClient wrapper,
and align comments/ADRs with real config-server last-applied writes.

Kept HEAD's already-diverged always-union sensitive-redaction comments in
intent_rpc.go/sensitive_path_union_test.go (ticket 04 rewrote that scoping
independently of this source branch's snapshot at the time) rather than
reapplying this commit's now-stale 'scoped semantics' wording verbatim.

Signed-off-by: Markus Vahlenkamp <markus.vahlenkamp@nokia.com>
Colocate Intent flatten, blob merge, and NewImportAdapter so Modify->Get
round-trips live in one package; configserver keeps only the remote port.

Adapted the rename/rename conflict against ticket 06's own configserver/
package layout (which had already moved importer.go/merge.go there from
pkg/tree/importer/configserver): re-homed those four files into
configsnapshot instead, and kept NewImportAdapter returning
importer.IntentAdapter (ticket 05's split) rather than this commit's
original pre-split importer.ImportConfigAdapter return type.

Signed-off-by: Markus Vahlenkamp <markus.vahlenkamp@nokia.com>
ADR 0003 already merged the wire into one ConfigSnapshotService; nothing
typed against Reader or Writer alone, so the half-interfaces and half-files
only mirrored a split the adapters refuse. Align the Go port name with the
glossary term.

Signed-off-by: Markus Vahlenkamp <markus.vahlenkamp@nokia.com>
Move split/strip next to the backend and keep join as a single helper
behind Document.IntentName so the encoding story lives with its owners.

Follow-up carried over from ticket 06 (deliberately deferred until
configsnapshot's own document.go existed, which this ticket's 6f0d373
equivalent landed earlier).

Signed-off-by: Markus Vahlenkamp <markus.vahlenkamp@nokia.com>
… types.UnknownOwner

Extracted the orthogonal, no-behavior-change half of f0c91db (from
origin/config-server-cache-backend): the ghost-intent exclusion mechanism
that commit also removed was never added to this stack in the first place
(ticket 05 skipped 10652b4 entirely), so there's nothing to revert here —
just the pure magic-string cleanup, done as its own tiny commit per the
ticket 07 notes.

Signed-off-by: Markus Vahlenkamp <markus.vahlenkamp@nokia.com>
docs/architecture.md was never split into this repo's history — fold the
two facts it was cited for (config-server as sole northbound Intent writer;
cold-start recovery on the control plane) inline instead of pointing at a
file that doesn't exist.

Extracted the pkg/cache-relevant sliver of origin/config-server-cache-backend's
67f4bdf, which otherwise spans an unrelated repo-wide docs/agent-process
reorg out of scope for this ticket.

Signed-off-by: Markus Vahlenkamp <markus.vahlenkamp@nokia.com>
New tests from ticket 03 referenced treetypes.NewSensitivePathIndex, which
does not exist; match the rest of the file and Datastore wiring.

Signed-off-by: Markus Vahlenkamp <markus.vahlenkamp@nokia.com>
@steiler
steiler force-pushed the cache-write-backend branch from 0577d08 to ed7e491 Compare October 14, 2026 09:09
@codecov

codecov Bot commented Oct 14, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants