Unverified Commit f650bf89 authored by Lemon-miaow's avatar Lemon-miaow
Browse files

fix(operator): idle auto-stop couldn't write — patch the spec, and grant the patch 

Two stacked blockers behind the frozen auto-stop, both found live after the
first two fixes let the timer finally tick:

- The stop used a whole-object Update while the same reconcile loop writes
  status; that risks clobbering a concurrent status write. Switch to the
  reaper's merge-patch pattern (spec.desiredState only; EmptySince is left for
  markStopped to clear).
- The operator Role never carried minecraftservers:patch, so the call failed
  closed with 403 (visible in the operator log as 'cannot update resource
  "minecraftservers"'). Grant patch and pin it in the RBAC scope test.

With all three layers fixed, the auto-stop path is: timer persists (schema),
wake-up fires (requeue), spec write allowed (RBAC).
parent 1c89a5ee

internal/operator/reconciler.go

+7 −2
Changes for internal/operator/reconciler.go: 7 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -185,9 +185,14 @@ func (r *Reconciler) reconcileRunning(ctx context.Context, server *v1alpha1.Mine 
				server.Status.EmptySince = &t

			} else if r.now().Time.Sub(server.Status.EmptySince.Time).Seconds() >=

				float64(server.Spec.Idle.EmptySecondsBeforeStop) {

				// Merge patch, not Update: an unrelated reconcile writes status

				// concurrently, and shipping the whole object back risks

				// clobbering it (the reaper's Stop uses the same pattern for

				// the same reason). EmptySince is deliberately left for

				// markStopped to clear once the scale-down completes.

				patch := client.MergeFrom(server.DeepCopy())

				server.Spec.DesiredState = v1alpha1.DesiredStopped

				server.Status.EmptySince = nil

				if err := r.Update(ctx, server); err != nil {

				if err := r.Patch(ctx, server, patch); err != nil {

					return ctrl.Result{}, err

				}

				return ctrl.Result{}, nil

internal/platform/rbac.go

+6 −3
Changes for internal/platform/rbac.go: 6 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -136,12 +136,15 @@ func APIBuildRole(p Params) *rbacv1.Role { 
// the manager's cache is namespace-scoped (see cmd/felis/operator.go), so a

// namespaced Role is sufficient. The operator owns StatefulSets and Services

// (Get/Create/Update — never patch or delete), writes only minecraftservers

// status (Status().Update — `update` only), and reads RCON Secrets. It never

// touches pods, PVCs, Events, or finalizers, so none appear here.

// status (Status().Update — `update` only) and patches spec.desiredState to

// Stopped for idle auto-stop (spec §8 — the one spec field it may write, using

// the same merge patch as the reaper's Stop: without the grant the auto-stop

// call fails closed with a 403), and reads RCON Secrets. It never touches pods,

// PVCs, Events, or finalizers, so none appear here.

func OperatorRole(p Params) *rbacv1.Role {

	p = p.withDefaults()

	return role(p.MinecraftNamespace, "felis-operator", ComponentOperator, []rbacv1.PolicyRule{

		rule([]string{groupFelis}, []string{"minecraftservers"}, []string{"get", "list", "watch"}),

		rule([]string{groupFelis}, []string{"minecraftservers"}, []string{"get", "list", "watch", "patch"}),

		rule([]string{groupFelis}, []string{"minecraftservers/status"}, []string{"update"}),

		rule([]string{groupApps}, []string{"statefulsets"}, []string{"get", "list", "watch", "create", "update"}),

		rule([]string{groupCore}, []string{"services"}, []string{"get", "list", "watch", "create", "update"}),

internal/platform/rbac_test.go

+5 −0
Changes for internal/platform/rbac_test.go: 5 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -149,6 +149,11 @@ func TestOperatorRole_ScopeExact(t *testing.T) { 
			t.Errorf("operator must have minecraftservers:%s (cached client)", v)

		}

	}

	// Idle auto-stop patches spec.desiredState=Stopped (spec §8). Found live:

	// without this grant the auto-stop fails closed with a 403.

	if !hasRule(op, groupFelis, "minecraftservers", "patch") {

		t.Error("operator must have minecraftservers:patch (idle auto-stop writes spec.desiredState)")

	}

	for _, v := range []string{"get", "list", "watch", "create", "update"} {

		if !hasRule(op, "apps", "statefulsets", v) {

			t.Errorf("operator must have statefulsets:%s", v)