Azure / Azure/unbounded

operator: reconcile the inventory server components as a Site component

Open
#636 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
28
Forks
11
Avg merge
1d 8h
Merged PRs (30d)
55

Description

Inventory has no lifecycle. Nothing deploys it: it is absent from
`DefaultRegistry()` (`internal/operator/reconciler.go:85-97`), and its manifests
under `deploy/inventory` are applied by hand. Every other component is reconciled
from the Site spec, gets a readiness condition, is cleaned up when disabled, and is
covered by the release gate.

This is "direction 1" of #531 for inventory, which deferred it precisely because it
is not in the component registry.

**Scope: the server side** - aggregator, viewer and inspector, all three. The agent
runs on the host as root and is tracked in #635.

**Prerequisite: #633**, which builds the three server images in the release and
nightly pipelines. `componentImages`
(`internal/operator/imagecoverage_test.go:61-67`) is cross-checked against
`DefaultRegistry()` and those workflows, so registering the component before its
images ship would fail the suite.

## Settled: per-Site

**The aggregator Service becomes per-Site.**
`deploy/inventory/aggregator/02-service.yaml.tmpl` is a single Service named
`inventory-collector`; N per-Site Services cannot share one name in
`unbounded-system`, so it becomes `inventory-collector-` following the
`metalman-controller-` precedent. Because the agent is a host process, that
address must also be reachable from outside the cluster - the coupling with #635.

**One aggregator, viewer and inspector per Site.** A fleet-wide view then needs N
viewers or a query across them. Worth recording as a known limitation.

**The database is already external.** The `postgres:17` container in
`aggregator/01-deployment.yaml.tmpl:21` is an *init container* that creates the
database on a server given by `inventory-database-config`. The operator does not
need to run stateful workloads; only the connection config and credentials need
deciding.

## Decisions

**1. Credentials.** `common/03-secret.yaml.tmpl` renders `pg-creds` from a
build-time variable defaulting to empty, which is why #633 excludes it from the
release tarball. Options: require a pre-created Secret named in the Site spec (as
`orca-credentials` already does), generate one, or consume an external connection
Secret. Related: one database per Site, or one shared.

**2. #631 is a constraint, not a separate task.** Inventory connects with
`sslmode=disable`, the precondition for five unfixable `lib/pq` advisories.
Whatever the operator writes into `inventory-database-config` should set
`POSTGRES_SSL_MODE` to `require` or better. This also removes a latent release-gate
risk: those advisories are Go vulndb entries today, and if they reach Trivy's
database as HIGH they would fail the three images added in #633.

**3. Readiness for the inspector.** The inspector is in scope: it is real,
implemented functionality (`internal/inventory/inspector`, two conflict detectors
writing a `conflicts` table), and it is part of the inventory system rather than an
optional extra.

Including it makes this the first operator component to emit a `CronJob`, which is
mechanically fine but leaves one genuine question. `InventoryReady` and the release
deploy gate both key off rollout status, and a CronJob has neither a rollout nor a
meaningful notion of ready. Decide what the condition reports for it: last
successful run within some window, mere existence of the object, or exclusion from
the condition entirely with only the two Deployments gating readiness. The deploy
gate cannot cover it either way, so `wait-rollouts.sh` will wait on the aggregator
and viewer only.

Nothing else blocks it. `CronJob` is already in `tierWorkload`
(`component/order.go:96-102`), the apply path is unstructured with no kind
allowlist, and `batch/v1` is in the scheme via `clientgoscheme`
(`cmd/unbounded-operator/main.go:270`). The RBAC and override table changes are
listed in the checklist below.

## Checklist

`storage` is the closest exemplar: per-Site, opt-in, embedded manifests.

1. **CRD** - `Inventory *InventoryComponentSpec` on `SiteComponents`
(`api/machina/v1alpha3/site_types.go:110-140`) embedding `SiteComponentSpec`
(`:152-156`); printcolumn near `:27-30`; `make generate` and
`make machina-manifests`.
2. **Component** - `internal/operator/components/inventory/inventory.go`
implementing `SiteComponent` (`component/component.go:60-79`): `Name`,
`ConditionType` (`InventoryReady`), `Enabled`, `Plan`, `CleanupPlan`.
3. **Per-Site objects** - `-` naming, `SiteOwnerReference`,
`SiteNodeAffinity`, site label in selector and pod template, images from
`env.Config.Image(...)`, workloads `Overridable: true`.
4. **No Namespace object** - operator-managed components skip it (#529/#530), so
`common/01-namespace.yaml.tmpl` leaves the picture and inventory's row in
`internal/unbounded/namespace_drift_test.go` flips to `operatorManaged: true`.
5. **Registry** - `inventory.New()` into `Registry.Site` (`reconciler.go:92-95`).
6. **Overrides** - `knownComponents` in `override/validate.go:28-37` with
`perSite: true`, and `CronJob` added to `knownKinds` (`:40-43`) so the
inspector can be overridden like every other workload.
`TestOverrideKindsMatchWhatComponentsPlan`
(`internal/operator/reconciler_test.go:881`) enforces agreement in both
directions, and only inspects operations marked `Overridable`.
7. **RBAC** - `create`, `patch`, `update` and `delete` on `batch` `cronjobs` in
`deploy/unbounded-operator/02-rbac.yaml.tmpl:51-53`, which today grants only
get/list/watch for the foreign workload audit. No test blocks this
(`TestOperatorClusterRoleGrantsForeignWorkloadAudit` asserts a floor), but it
widens the operator's cluster-wide write surface and deserves explicit review.
8. **Site init** - `--enable-inventory`
(`cmd/kubectl-unbounded/app/site_init.go:489-491`) and
`assets/unbounded-net-site/site.yaml:8-15`.
9. **Coverage** - `componentImages`, `wantConditions`
(`reconciler_test.go:394-415`), `docs/content/reference/workload-overrides.md`,
and the inventory section of `docs/content/reference/architecture.md`, which
#633 left saying inventory is not operator-reconciled.
10. **Deploy gate** - add the workloads to `wait-rollouts.sh` in
`release-upgrade.yaml`, per the per-Site discovery `metalman-targets.sh`
established. The aggregator and viewer only: a CronJob has no rollout status,
so the inspector cannot be gated this way (see decision 3).

## Open question: tests

Unit coverage is settled by precedent: golden `Plan()`/`CleanupPlan()` summaries
(`storage_test.go:373-420`).

- **Liveness only** adds nothing; `hack/release/smoke/core-namespaces-ready.sh`
already asserts every pod in the namespace is Ready.
- **Surface probes** - aggregator accepts gRPC, viewer returns 200. Cheap, proves
no data path.
- **Seeded round trip** - publish a synthetic record over the aggregator's gRPC
API, read it back through the viewer. Proves aggregator to PostgreSQL to viewer
without hardware. Probably the right level for a release gate.
- **Operator e2e** - kind-based test in `e2e/operator/` (pattern:
`reaper_e2e_test.go`): enable on a Site, assert workloads and owner references,
disable, assert cleanup. No database, no hardware.

**Recommendation:** operator e2e for the reconcile contract plus a seeded
round-trip smoke test. True hardware collection belongs to #635.

## Acceptance criteria

- `spec.components.inventory.enabled: true` creates the workloads; false removes
them; deleting the Site garbage-collects them.
- `Site.status.conditions` carries `InventoryReady`.
- Images resolve at the operator's own version.
- The release deploy gate waits on the inventory workloads.
- `make test` passes with the component registered.

## Non-goals

Changing what inventory collects or its schema; retention or multi-tenancy;
enabling inventory by default on `unbounded-nightly` or `unbounded-stable`.

Contributor guide

Open the contributing guide

Research direction

Start with the per-Site storage component and its golden Plan/CleanupPlan tests, then read SiteComponentSpec, DefaultRegistry(), and the inventory image-coverage checks. Trace the listed CRD, override, RBAC, site-init, and release-gate entry points, while resolving the credential, inspector readiness, and test decisions. Done means the acceptance criteria pass, including enable/disable cleanup, InventoryReady, image resolution, deploy gating, and make test.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.