apache / apache/solr-operator

Changing `spec.solrAddressability.podPort` deletes every pod without migrating replicas

Open
#847 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
283
Forks
148
PR merge metrics
No merged PRs in 30d

Description

# Changing `spec.solrAddressability.podPort` deletes every pod without migrating replicas

## Environment

- solr-operator built from `ed5c5c7d28a4c1189d19f581259e05385c0d4b20`
- Solr 9.7.0
- Kubernetes: kind
- SolrCloud: 2 pods, `dataStorage: ephemeral`, `updateStrategy.method: Managed`

## What happened

I edited one field on a healthy SolrCloud:

```yaml
spec:
solrAddressability:
podPort: 8983 # -> 8984
```

That is a pod template change, so the operator correctly started a managed rolling update. But it deleted the pods **without migrating any replicas first**.

The operator log shows the delete happening five times:

```
ManagedUpdateSelector ... Deleting solr pod for update ... "pod": "t4-solrcloud-1"
```

and zero migration activity for those pods — no `Migrating all replicas off of pod before deletion`, no `ReplicaMigrationStarted` event, no `move-replicas-*` async request. Nothing was logged as an error. As far as the operator was concerned the rolling update succeeded.

## Where the source code is wrong

A running pod registers its Solr node name in ZooKeeper using the `podPort` value from the pod template at creation time. After the CR is updated, the pod remains registered as `...:8983_solr`.

On each reconcile, the operator constructs the lookup key from the current CR using [`SolrNodeName`](https://github.com/apache/solr-operator/blob/ed5c5c7d28a4c1189d19f581259e05385c0d4b20/controllers/util/solr_update_util.go#L527-L529). After the update, this function returns `...:8984_solr`. The registered node name and the operator's lookup key therefore no longer match.

So the lookup in [`PodContents`](https://github.com/apache/solr-operator/blob/ed5c5c7d28a4c1189d19f581259e05385c0d4b20/controllers/util/solr_update_util.go#L112-L115) misses every running pod:

```go
// controllers/util/solr_update_util.go:112-115
contents, isInClusterState = state.NodeContents[SolrNodeName(cloud, podName)]
```

That failed lookup is immediately converted to `podHasReplicas=false`:

```go
// controllers/util/solr_update_util.go:118-121
func (state NodeReplicaState) PodHasReplicas(cloud *solr.SolrCloud, podName string) bool {
contents, isInClusterState := state.PodContents(cloud, podName)
return isInClusterState && contents.replicas > 0
}
```

[`EvictReplicasForPodIfNecessary`](https://github.com/apache/solr-operator/blob/ed5c5c7d28a4c1189d19f581259e05385c0d4b20/controllers/util/solr_update_util.go#L551-L602) treats this `false` as proof that there are no replicas. It skips `REPLACENODE` and allows the pod to be deleted:

```go
// controllers/util/solr_update_util.go:569-591
if podHasReplicas {
// Submit new Replace Node request
...
} else {
canDeletePod = true
}
```

[`DeletePodForUpdate`](https://github.com/apache/solr-operator/blob/ed5c5c7d28a4c1189d19f581259e05385c0d4b20/controllers/solr_pod_lifecycle_util.go#L49-L112) then deletes the pod without rechecking the cluster state:

```go
// controllers/solr_pod_lifecycle_util.go:90-106
if podHasReplicas {
requeueAfterDuration = time.Millisecond * 10
} else {
deletePod = true
}
...
if deletePod {
err = r.Delete(ctx, pod, client.Preconditions{UID: &pod.UID})
}
```

The bug is that "node name not found" is treated as "pod has no replicas." After `podPort` changes, the lookup key is wrong, but the operator still skips migration and deletes the pod.

Because the same edit is a pod template change, this repeats for every pod in the StatefulSet.

## Expected behavior

A missed lookup should not be treated as proof that the pod has no replicas. The operator should distinguish between these cases:

- The pod is present in the cluster state and has no replicas: it is safe to delete.
- The pod is not found in the cluster state: do not delete it until its replica state can be determined.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with controllers/util/solr_update_util.go, especially SolrNodeName, PodContents, PodHasReplicas, and EvictReplicasForPodIfNecessary, then trace the call from controllers/solr_pod_lifecycle_util.go in DeletePodForUpdate. Reproduce or test the podPort change path and ensure a missing cluster-state node is not treated as replica-free; done means replicas are migrated or deletion is deferred until their state is known.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, kubernetes
Domain
distributed-systems, search
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.