kubeslice / kubeslice/kubeslice-controller

Bug: ReconcileCluster silently drops cluster from reconciliation on transient API error

Open
#323 0 comments 0 reactions 1 assignee Claimed by @richiesebastian View on GitHub
bug
Dominant language
Go
Stars
73
Forks
48
Avg merge
2d 21h
Merged PRs (30d)
8

Description

### 📜 Description

`ReconcileCluster` in `service/cluster_service.go` at lines 72-78 calls `util.GetResourceIfExist` to look up the project namespace, but never checks the returned error. The error from line 72 is silently discarded.

```go
// service/cluster_service.go, lines 70-78
// Step 0: check if cluster is in project namespace
projectNs := &corev1.Namespace{}
found, err = util.GetResourceIfExist(ctx, client.ObjectKey{
Name: req.Namespace,
}, projectNs)
if !found || !c.checkForProjectNamespace(projectNs) {
logger.Infof("Created Cluster %v is not in project namespace. Returning from reconciliation loop.", req.NamespacedName)
return ctrl.Result{}, nil
}
```

If the API server returns a transient error (network timeout, etcd leader election, brief unavailability), `GetResourceIfExist` returns `false, err`. The code ignores `err`, sees `found == false`, and returns `ctrl.Result{}, nil`.

In controller-runtime, returning a nil error with an empty `ctrl.Result{}` means "reconciliation succeeded, do not requeue." The controller will never retry this cluster unless an external event (another update to the Cluster resource) triggers a new reconciliation. The cluster is effectively orphaned from all further processing: no finalizers, no secrets, no RBAC, no default slice operations.

This is a regression from the correct pattern used just above at lines 55-57 for the cluster lookup itself:

```go
// Lines 55-57 - correct pattern
found, err := util.GetResourceIfExist(ctx, req.NamespacedName, cluster)
if err != nil {
return ctrl.Result{}, err
}
```

### 👟 Reproduction steps

1. Register a Cluster in a project namespace:

apiVersion: controller.kubeslice.io/v1alpha1
kind: Cluster
metadata:
name: worker-1
namespace: kubeslice-my-project
spec:
networkInterface: eth0
clusterProperty:
geoLocation:
cloudProvider: gcp
cloudRegion: us-central1

2. Induce a transient API server failure during the namespace lookup. This can occur naturally during:
- etcd leader failover
- API server rolling restart
- Network partition between controller and API server
- Resource quota or rate limiting on the kube-apiserver

3. The reconciler hits the `GetResourceIfExist` call at line 72, which returns `false, err`.

4. The error is ignored. The reconciler logs "Created Cluster worker-1 is not in project namespace" (a misleading message since the real issue was an API error, not a missing namespace) and returns nil.

5. The cluster is never reconciled again until something externally modifies the Cluster resource.

### 👍 Expected behavior

The reconciler should check the error from `GetResourceIfExist` and return it. In controller-runtime, returning a non-nil error causes an exponential backoff requeue, which is the correct behavior for transient failures. The cluster should be retried automatically.

### 👎 Actual Behavior

The reconciler returns `ctrl.Result{}, nil`, signaling successful completion. The cluster is permanently dropped from reconciliation. No error is surfaced in logs or metrics. The only log line is a misleading info message claiming the cluster is not in a project namespace.

Downstream effects of the dropped reconciliation:
- Cluster secret is never created or rotated
- Worker cluster RBAC (ServiceAccount, RoleBinding) is never provisioned
- Default slice operations are never triggered
- Cluster health monitoring is never initialized

The cluster appears "stuck" in whatever state it was in before the transient failure.

### 🐚 Relevant log output

```shell
No error is logged. The only output during the failure is the misleading info message:

INFO Created Cluster kubeslice-my-project/worker-1 is not in project namespace. Returning from reconciliation loop.

This message is indistinguishable from a genuinely misconfigured cluster, making the bug extremely difficult to diagnose in production. The actual API error from GetResourceIfExist is silently discarded.
```

### Version

master branch (latest HEAD as of 2026-05-08). The bug exists in all releases containing the namespace check in ReconcileCluster. Affected release branches include release-shimla, release-udaipur, and release-varanasi.

### 🖥️ What operating system are you seeing the problem on?

_No response_

### ✅ Proposed Solution

Add an error check between the `GetResourceIfExist` call and the `!found` check. Apply at `service/cluster_service.go` lines 74-75:

Current code (lines 72-78):

found, err = util.GetResourceIfExist(ctx, client.ObjectKey{
Name: req.Namespace,
}, projectNs)
if !found || !c.checkForProjectNamespace(projectNs) {
logger.Infof("Created Cluster %v is not in project namespace. Returning from reconciliation loop.", req.NamespacedName)
return ctrl.Result{}, nil
}

Fixed code:

found, err = util.GetResourceIfExist(ctx, client.ObjectKey{
Name: req.Namespace,
}, projectNs)
if err != nil {
return ctrl.Result{}, err
}
if !found || !c.checkForProjectNamespace(projectNs) {
logger.Infof("Created Cluster %v is not in project namespace. Returning from reconciliation loop.", req.NamespacedName)
return ctrl.Result{}, nil
}

This matches the pattern already used at lines 55-57 in the same function. Returning the error causes controller-runtime to requeue with exponential backoff, ensuring the cluster is retried after the transient failure resolves.

### 👀 Have you spent some time to check if this issue has been raised before?

- [x] I checked and didn't find any similar issue

### Code of Conduct

- [x] I agree to follow this project's Code of Conduct

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.