diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index bf269af..8d35214 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -58,6 +58,42 @@ jobs: run: | helm lint charts/gpucellpool helm template gpucellpool charts/gpucellpool --namespace gpucellpool-system > /dev/null + # The release workflow pushes the git tag verbatim (vX.Y.Z) while appVersion + # carries no leading v, so a default install once asked for a tag that had + # never been published and failed on its own image. Assert the two agree. + - name: Default image tag matches what the release workflow publishes + run: | + want="v$(grep '^appVersion:' charts/gpucellpool/Chart.yaml | awk '{print $2}' | tr -d '\"')" + got=$(helm template gpucellpool charts/gpucellpool \ + | grep -oE 'gpucellpool/manager:[^"]+' | head -1 | cut -d: -f2) + echo "appVersion implies $want; chart renders $got" + test "$want" = "$got" + + # Both of these shipped broken in v0.1.0 and neither could fail a test that + # ran with admin credentials: the operator RENDERS a per-cell bootstrap + # Secret, and its recorder writes through events.k8s.io. A read-only secrets + # grant meant no chart-installed pool could ever create a cell; a + # core-group-only events grant meant every event was silently rejected. + - name: Chart RBAC permits what the operator actually does + run: | + helm template gpucellpool charts/gpucellpool --show-only templates/rbac.yaml > /tmp/rbac.yaml + python3 -c ' + import sys, yaml + need = {"secrets": {"create", "update"}, "events": {"create"}} + have = {k: set() for k in need} + for doc in yaml.safe_load_all(open("/tmp/rbac.yaml")): + for rule in (doc or {}).get("rules", []): + if "events" in rule.get("resources", []) and "events.k8s.io" not in rule.get("apiGroups", []): + continue + for res in rule.get("resources", []): + if res in have: + have[res] |= set(rule.get("verbs", [])) + for res, verbs in need.items(): + missing = verbs - have[res] + assert not missing, "%s missing %s (have %s)" % (res, sorted(missing), sorted(have[res])) + print("rbac ok:", {k: sorted(v) for k, v in have.items()}) + ' + # The chart ships a copy of the CRD; helm upgrade never updates crds/, so a # stale copy would install an old schema on a fresh cluster and silently # drop fields. diff --git a/charts/gpucellpool/rules.yaml b/charts/gpucellpool/rules.yaml index e53b924..ac0f0e8 100644 --- a/charts/gpucellpool/rules.yaml +++ b/charts/gpucellpool/rules.yaml @@ -1,18 +1,21 @@ - apiGroups: - "" resources: - - events + - secrets verbs: - create - - patch + - get + - list + - update + - watch - apiGroups: - "" + - events.k8s.io resources: - - secrets + - events verbs: - - get - - list - - watch + - create + - patch - apiGroups: - bootstrap.cluster.x-k8s.io resources: diff --git a/charts/gpucellpool/templates/deployment.yaml b/charts/gpucellpool/templates/deployment.yaml index c9a1355..627f7ce 100644 --- a/charts/gpucellpool/templates/deployment.yaml +++ b/charts/gpucellpool/templates/deployment.yaml @@ -24,7 +24,13 @@ spec: type: RuntimeDefault containers: - name: manager - image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" + {{- /* + The release workflow pushes the git tag verbatim (v0.1.0), while appVersion + carries no leading v (0.1.0, and the workflow asserts that). Defaulting to + appVersion alone therefore asked for a tag that does not exist: a default + install of the released chart failed with ImagePullBackOff on its own image. + */}} + image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default (printf "v%s" .Chart.AppVersion) }}" imagePullPolicy: {{ .Values.image.pullPolicy }} args: - --health-probe-bind-address=:8081 diff --git a/charts/gpucellpool/templates/monitoring/servicemonitor.yaml b/charts/gpucellpool/templates/monitoring/servicemonitor.yaml index cce1218..a8c7cb5 100644 --- a/charts/gpucellpool/templates/monitoring/servicemonitor.yaml +++ b/charts/gpucellpool/templates/monitoring/servicemonitor.yaml @@ -31,6 +31,14 @@ spec: - port: metrics path: /metrics interval: {{ $sm.interval | default "30s" }} + # Every gpucell_* series carries pool and namespace labels describing the POOL. + # Prometheus overwrites exposed labels with target labels by default, so our + # namespace was being replaced by the operator's own namespace and ours pushed + # to exported_namespace — measured against a live Prometheus. Every alert then + # named the wrong namespace. honorLabels keeps the describing labels, which is + # exactly what kube-prometheus-stack does for kube-state-metrics, for exactly + # this reason. + honorLabels: true {{- if .Values.metrics.secure }} # The manager serves HTTPS with a self-signed certificate, so verification # is skipped. It does not authenticate scrapers — see issue #11 and the diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 83a4a25..b8a6835 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -7,18 +7,21 @@ rules: - apiGroups: - "" resources: - - events + - secrets verbs: - create - - patch + - get + - list + - update + - watch - apiGroups: - "" + - events.k8s.io resources: - - secrets + - events verbs: - - get - - list - - watch + - create + - patch - apiGroups: - bootstrap.cluster.x-k8s.io resources: diff --git a/internal/controller/controller.go b/internal/controller/controller.go index 02be661..89c0fbd 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -62,8 +62,20 @@ type GPUCellPoolReconciler struct { // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters,verbs=get;list;watch // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=kubeswiftmachines,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=bootstrap.cluster.x-k8s.io,resources=*,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch +// Secrets need create+update, not only read: the operator RENDERS a per-cell +// bootstrap Secret from the user's join template (reconcileBootstrapSecret). A +// read-only grant let the chart install, pass every test that used admin +// credentials, and then fail on the first cell with "secrets is forbidden". +// Delete is deliberately absent — the per-cell Secret carries the pool's +// ownerRef and is garbage-collected. +// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;update +// Events need BOTH groups. The core-group grant alone looks right and is +// useless: the recorder writes through the events.k8s.io API, so every event +// this operator emits was rejected with "events.events.k8s.io is forbidden" — +// visible only in the manager log, never to the operator reading `kubectl +// describe gpucellpool`. The core grant stays for clients that still read there. // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch +// +kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patch // Reconcile implements the loop in docs/design/gpucellpool-reconciliation.md §4. func (r *GPUCellPoolReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {