Azure / Azure/unbounded

release-upgrade silently wipes machina-config apiServerEndpoint, crashing the controller

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

Description

## Summary

Every `release-upgrade.yaml` deploy silently overwrites the `machina-config` ConfigMap with the bundled `deploy/machina/03-config.yaml.tmpl`, which is missing the per-cluster `apiServerEndpoint` field that `machina-controller` v0.1.5+ requires. The controller then exits at startup with `error: resolve cluster info: API server endpoint not set in config` and stays in `CrashLoopBackOff` until an operator manually re-patches the ConfigMap.

The only code path that populates `apiServerEndpoint` is `kubectl unbounded site init`, which runs once at cluster bootstrap. Any subsequent upgrade through the release-upgrade workflow undoes it.

## How this surfaced

`unbounded-stable` had been stuck on `v0.1.5` for ~4 weeks because every release-upgrade run since v0.1.3 failed at the asset-download step (fixed in #233). When the first successful release-upgrade run after #233 deployed v0.1.11, the rollout failed at `Wait for rollouts`: the new `machina-controller` pod was in `CrashLoopBackOff` for the same reason as the existing v0.1.5 pod had been for the past month. Investigation showed the cluster has had a missing-config-field problem since 2026-05-13, hidden by the asset-download failure masking every later deploy.

A manual `kubectl apply` to re-add the field unbroke the cluster, but the underlying workflow bug means the next release-upgrade run (for any future tag) will silently break it again.

## Root cause

`cmd/machina/machina/controller/config.go`:

```go
type Config struct {
APIServerEndpoint string `yaml:"apiServerEndpoint"`
MetricsAddr string `yaml:"metricsAddr"`
ProbeAddr string `yaml:"probeAddr"`
EnableLeaderElection bool `yaml:"enableLeaderElection"`
MaxConcurrentReconciles int `yaml:"maxConcurrentReconciles"`
ProvisioningTimeout time.Duration `yaml:"provisioningTimeout"`
}
```

`cmd/machina/machina/controller/cluster_info.go:50` makes the field mandatory:

```go
if cfg.APIServerEndpoint == "" {
return nil, fmt.Errorf("API server endpoint not set in config")
}
```

`deploy/machina/03-config.yaml.tmpl` does NOT include the field:

```yaml
data:
config.yaml: |
metricsAddr: ":8080"
probeAddr: ":8081"
enableLeaderElection: false
maxConcurrentReconciles: 50
```

The only writer of `apiServerEndpoint` is `cmd/kubectl-unbounded/app/site_init.go:289-326`:

```go
func (h *siteInitHandler) ensureMachinaIsRunning(ctx context.Context) error {
machinaCfg := controller.DefaultConfig()
machinaCfg.APIServerEndpoint = h.kubeConfig.Host
...
// Apply the populated ConfigMap directly
...
// Then run the installer, telling it to skip 03-config.yaml so the
// bundled empty template doesn't overwrite the populated one.
h.installMachina.skipPaths = []string{"03-config.yaml"}
return h.installMachina.run(ctx)
}
```

Note the explicit `skipPaths` - `site init` already knows the bundled template is destructive and refuses to apply it. The release-upgrade workflow does NOT have an equivalent skip.

`.github/workflows/release-upgrade.yaml` (`Apply manifests (upgrade)` step):

```yaml
kubectl apply --server-side --force-conflicts -R -f "${MANIFESTS_DIR}/machina/"
```

`-R -f` recursively applies every file in the directory, including `03-config.yaml`. `--force-conflicts` actively takes ownership of fields managed by other field managers. So even if `site init` had set `apiServerEndpoint` correctly, the first release-upgrade run wipes it.

## Reproduction

1. Bootstrap a cluster via `kubectl unbounded site init`. Confirm `kubectl -n unbounded-kube get cm machina-config -o yaml` contains an `apiServerEndpoint` line, and that `machina-controller` is `1/1 Running`.
2. Trigger `release-upgrade.yaml` against any release v0.1.5 or later (`gh workflow run release-upgrade.yaml -f tag=v0.1.11`).
3. Observe the `machina-config` ConfigMap no longer contains `apiServerEndpoint`.
4. Observe `machina-controller` enter `CrashLoopBackOff` with `error: resolve cluster info: API server endpoint not set in config`.
5. The `deploy` job in the workflow may also time out at `Wait for rollouts` because the new ReplicaSet's pod never becomes Ready.

## Why the release tarball can't just include the value

The value of `apiServerEndpoint` is per-cluster (e.g. `https://unbounded-stable-pqfy2vse.hcp.canadacentral.azmk8s.io:443` for unbounded-stable). The release manifests tarball is a single artifact shipped to every consumer. There's no sensible default to put in the template.

## Fix options

These are not mutually exclusive; some combinations make sense.

### Option (i) - Workflow skips `03-config.yaml`

Mirror what `site_init.go` already does: apply everything in `machina/` except `03-config.yaml`. Tiny workflow change, e.g.:

```yaml
- name: Apply manifests (upgrade)
if: env.MODE == 'upgrade'
run: |
set -euo pipefail
kubectl apply --server-side --force-conflicts -R -f "${MANIFESTS_DIR}/net/"
find "${MANIFESTS_DIR}/machina" -type f -name '*.yaml' ! -name '03-config.yaml' \
-print0 | xargs -0 -n1 kubectl apply --server-side --force-conflicts -f
```

- **Pros**: Smallest possible diff. Safe for upgrade. Requires no controller or release-pipeline changes.
- **Cons**: Operator must have run `site init` once for the ConfigMap to exist with the right field. Doesn't fix the smell of shipping a template that's broken when applied as-is. Future upgrade workflows in other contexts (e.g. helm chart, third-party operator) would hit the same trap and have to reinvent the skip.

### Option (ii) - Drop `03-config.yaml` from the release tarball

Have `release.yaml` (or wherever the manifests tarball is built) exclude `03-config.yaml` from the bundle. The template stays in the repo for use by `site init` (which currently consumes it from an embed.FS), but it never ships to consumers as a directly-appliable file.

- **Pros**: Matches `site_init.go`'s existing `skipPaths` intent at the release boundary instead of having every consumer reinvent the skip. Anyone doing `kubectl apply -R -f` on the tarball just works.
- **Cons**: A bit more pipeline plumbing than option (i). `site init` will still need the template via embed.FS or similar, so the source file stays.

### Option (iii) - `kubectl unbounded site upgrade` subcommand

Add a new subcommand that owns the apply semantics (skip rules, field-manager handling, validation), and have `release-upgrade.yaml` call that instead of raw `kubectl apply`.

- **Pros**: Most maintainable long-term. The plugin is the source of truth for "what does an upgrade actually mean"; the workflow becomes dumb. Easy to add validation, dry-run, rollback hooks.
- **Cons**: New Go code. Bigger surface to maintain. Versioning question: does `site upgrade` from v0.1.20 know how to upgrade a v0.1.11 cluster? Probably yes (it just applies the target tag's manifests with the right skips), but the contract needs to be designed.

### Option (iv) - Controller falls back to in-cluster API discovery

Change `ResolveClusterInfo` in `cmd/machina/machina/controller/cluster_info.go` to treat `APIServerEndpoint` as optional. When empty, fall back to either:

- The in-cluster service account's `KUBERNETES_SERVICE_HOST` and `KUBERNETES_SERVICE_PORT` env vars, or
- `rest.InClusterConfig()` from client-go, which reads the same env vars and produces a usable host.

The explicit field becomes an override for out-of-cluster usage (rare).

- **Pros**: Removes the per-cluster config requirement entirely. The cluster's API server endpoint is already available to every in-cluster pod via the standard Downward API - requiring an operator-set field for something the controller can discover for free is redundant. Eliminates this class of bug at the root.
- **Cons**: Behavior change in the controller; existing operators who explicitly set the field still work but new clusters silently get a different value. Worth checking whether the explicit field was added for a specific reason (e.g. routing through a private link, or supporting an out-of-cluster controller deployment) before removing the requirement.

### Combinations

- **(i) alone** is the minimum viable fix. The cluster stops re-breaking on upgrade.
- **(ii) + (iv)** is the recommended end state. The controller doesn't need the field; the release tarball doesn't ship a destructive template; the existing `site init` still works for whoever wants explicit control.
- **(iii)** is worth doing if there's appetite for a richer plugin-driven upgrade flow regardless of this specific bug.

## Recommendation

Two-PR sequence:

1. **PR A: Option (i)** as a fast unbreak. Maybe one screen of YAML. Lands today, the next release-upgrade run is safe.
2. **PR B: Option (iv)** as a follow-up. Probably one screen of Go in `cluster_info.go`. Once it lands, Option (ii) (dropping the file from the tarball) becomes safe to do as well, but is optional because the field becoming optional removes the destructiveness.

Option (iii) is a separate, larger conversation about plugin-driven release management and isn't blocking.

## Related

- PR #233 - fixed the workflow trigger so deploys actually attempt; this is what surfaced the underlying problem.
- PR #234 - fixed the smoke-discover job that ran into a directory-doesn't-exist-at-tag issue.
- Original investigation thread: the v0.1.11 backfill run https://github.com/Azure/unbounded/actions/runs/27172753761 timed out at `Wait for rollouts` because of this bug.

## Acceptance criteria

The next release-upgrade run for a freshly-cut tag against a cluster that was previously bootstrapped via `site init` MUST:

- Leave the `machina-config` ConfigMap's `apiServerEndpoint` field intact.
- Bring `machina-controller` to `1/1 Running` within the rollout timeout.
- Pass the `core-namespaces-ready` smoke test without manual intervention.

Bonus: a regression test (e2e or otherwise) that exercises an upgrade from a previously-bootstrapped cluster to a newer tag and verifies the ConfigMap and controller remain healthy.

Contributor guide

Open the contributing guide

Research direction

Start with the Apply manifests (upgrade) step in .github/workflows/release-upgrade.yaml and compare its recursive apply with skipPaths in cmd/kubectl-unbounded/app/site_init.go. Reproduce the upgrade against a bootstrapped cluster, then verify the next run preserves apiServerEndpoint, the machina-controller rollout reaches 1/1 Running, and core-namespaces-ready passes without manual intervention.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, go, kubernetes
Domain
ci-cd, devops, infrastructure
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.