From 8e11093bf38f17cf886b3c0e131b12a666bdfaff Mon Sep 17 00:00:00 2001 From: Rai Date: Fri, 3 Jul 2026 16:45:01 -0700 Subject: [PATCH 1/2] fix: bound runner setup RPCs with a configurable timeout When a runner pod dies mid-reconcile (e.g. its node reboots), the reconcile goroutine hangs forever inside a runner RPC. The runner gRPC client is created with waitForReady:true (getRunnerConnection), so an RPC on a channel that can no longer connect blocks until its context is done -- and setupTerraform's RPCs (UploadAndExtract ... Init ... SelectWorkspace) use the raw reconcile context, which has no deadline. controller-runtime will not start a new Reconcile for a key while the previous one is still running, so the affected Terraform object is never reconciled again and one of the --concurrent worker slots is leaked permanently. Wrap the setup RPCs in a context bounded by a new --runner-rpc-timeout flag (default 30m -- generous enough for provider downloads and long inits, finite so a dead runner surfaces as an error and requeues instead of hanging). This also supersedes the long-commented-out ctx30s TODO in setupTerraform. Signed-off-by: Rai --- cmd/manager/main.go | 6 ++++++ controllers/tf_controller.go | 1 + controllers/tf_controller_backend.go | 14 +++++++++++--- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index c4f275b6..283e2c93 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -91,6 +91,7 @@ func main() { rotationCheckFrequency time.Duration runnerGRPCPort int runnerCreationTimeout time.Duration + runnerRPCTimeout time.Duration runnerGRPCMaxMessageSize int allowBreakTheGlass bool clusterDomain string @@ -118,6 +119,10 @@ func main() { "The interval that the mTLS certificate rotator should check the certificate validity.") flag.IntVar(&runnerGRPCPort, "runner-grpc-port", 30000, "The port which will be exposed on the runner pod for gRPC connections.") flag.DurationVar(&runnerCreationTimeout, "runner-creation-timeout", 120*time.Second, "Timeout for creating a runner pod.") + flag.DurationVar(&runnerRPCTimeout, "runner-rpc-timeout", 30*time.Minute, + "Maximum duration for the batch of runner RPCs that set up Terraform (upload, "+ + "backend config, init, workspace select). Bounds the reconcile so a runner pod "+ + "that dies mid-RPC surfaces as an error and requeues instead of hanging forever.") flag.IntVar(&runnerGRPCMaxMessageSize, "runner-grpc-max-message-size", 4, "The maximum message size for gRPC connections in MiB.") flag.BoolVar(&allowBreakTheGlass, "allow-break-the-glass", false, "Allow break the glass mode.") flag.StringVar(&clusterDomain, "cluster-domain", "cluster.local", "The cluster domain used by the cluster.") @@ -253,6 +258,7 @@ func main() { CertRotator: rotator, RunnerGRPCPort: runnerGRPCPort, RunnerCreationTimeout: runnerCreationTimeout, + RunnerRPCTimeout: runnerRPCTimeout, RunnerGRPCMaxMessageSize: runnerGRPCMaxMessageSize, AllowBreakTheGlass: allowBreakTheGlass, ClusterDomain: clusterDomain, diff --git a/controllers/tf_controller.go b/controllers/tf_controller.go index 9254ea49..f08fa2a7 100644 --- a/controllers/tf_controller.go +++ b/controllers/tf_controller.go @@ -86,6 +86,7 @@ type TerraformReconciler struct { CertRotator *mtls.CertRotator RunnerGRPCPort int RunnerCreationTimeout time.Duration + RunnerRPCTimeout time.Duration RunnerGRPCMaxMessageSize int AllowBreakTheGlass bool ClusterDomain string diff --git a/controllers/tf_controller_backend.go b/controllers/tf_controller_backend.go index 5e298b65..5f3ce832 100644 --- a/controllers/tf_controller_backend.go +++ b/controllers/tf_controller_backend.go @@ -28,6 +28,17 @@ func (r *TerraformReconciler) backendCompletelyDisable(terraform *infrav1.Terraf func (r *TerraformReconciler) setupTerraform(ctx context.Context, patchHelper *patch.SerialPatcher, runnerClient runner.RunnerClient, terraform *infrav1.Terraform, sourceObj sourcev1.Source, revision string, reconciliationLoopID string) (*infrav1.Terraform, string, string, error) { log := ctrl.LoggerFrom(ctx) + // Bound the runner RPCs below (UploadAndExtract ... Init ... SelectWorkspace). + // The runner gRPC client is created with waitForReady:true, so an RPC on a + // channel that cannot connect (e.g. the runner pod was killed by a node reboot + // mid-reconcile) blocks until its context is done. Without a deadline the call + // blocks forever, and because controller-runtime does not start a new Reconcile + // for a key while the previous one is still running, that Terraform object is + // never reconciled again and a worker slot is leaked permanently. A generous, + // configurable ceiling turns "never" into "requeue after RunnerRPCTimeout". + ctx, cancel := context.WithTimeout(ctx, r.RunnerRPCTimeout) + defer cancel() + tfInstance := "0" tmpDir := "" @@ -48,9 +59,6 @@ func (r *TerraformReconciler) setupTerraform(ctx context.Context, patchHelper *p ), tfInstance, tmpDir, err } - // we fix timeout of UploadAndExtract to be 30s - // ctx30s, cancelCtx30s := context.WithTimeout(ctx, 30*time.Second) - // defer cancelCtx30s() uploadAndExtractReply, err := runnerClient.UploadAndExtract(ctx, &runner.UploadAndExtractRequest{ Namespace: terraform.Namespace, Name: terraform.Name, From 811e86da3096c5487340aa806d9594002c72de89 Mon Sep 17 00:00:00 2001 From: Rai Date: Fri, 3 Jul 2026 16:45:01 -0700 Subject: [PATCH 2/2] fix: reap Failed runner pods (handle stateUnknown) A runner pod in Failed phase (e.g. killed by a node reboot) matches none of the branches in reconcileRunnerPod: it has the tls-secret-name label, the label matches, it has no DeletionTimestamp, and its phase is not Running -- so podState stays stateUnknown and the switch is a no-op. The dead pod is never cleaned up, so Error runner pods accumulate and connections to them hang under waitForReady. Add a stateUnknown case that force-deletes the stale pod and creates a fresh one, mirroring the stateMustBeDeleted path. Signed-off-by: Rai --- controllers/tf_controller_runner.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/controllers/tf_controller_runner.go b/controllers/tf_controller_runner.go index 2a639f38..9d04b69f 100644 --- a/controllers/tf_controller_runner.go +++ b/controllers/tf_controller_runner.go @@ -455,6 +455,28 @@ func (r *TerraformReconciler) reconcileRunnerPod(ctx context.Context, terraform case stateRunning: // do nothing traceLog.Info("Pod is running, do nothing") + case stateUnknown: + // The pod exists but is neither Running nor Terminating — e.g. it is in + // Failed phase after its node rebooted or it was OOM-killed. It will never + // serve gRPC again, but it is not being deleted either, so left alone it + // lingers forever (and any connection to it hangs under waitForReady). + // Force-delete it and recreate a fresh one. + log.Info("runner pod in unexpected (non-running, non-terminating) state, force-deleting", "name", terraform.Name) + if err := r.Delete(ctx, &runnerPod, + client.GracePeriodSeconds(1), // force kill = 1 second + client.PropagationPolicy(metav1.DeletePropagationForeground), + ); err != nil && !errors.IsNotFound(err) { + traceLog.Error(err, "Hit an error") + return "", err + } + if err := waitForPodToBeTerminated(); err != nil { + traceLog.Error(err, "Hit an error") + return "", fmt.Errorf("failed to wait for the stale pod termination: %v", err) + } + if err := createNewPod(); err != nil { + traceLog.Error(err, "Hit an error") + return "", err + } } // wait for pod ip