This is an automated email from the ASF dual-hosted git repository.
janhoy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr-operator.git
The following commit(s) were added to refs/heads/main by this push:
new c09922f Always retry a failed cluster operation (#876)
c09922f is described below
commit c09922fd90b9541f88f8ea0d6f91d4d8d62f3be3
Author: Jan Høydahl <[email protected]>
AuthorDate: Thu Nov 5 23:25:10 2026 +0100
Always retry a failed cluster operation (#876)
---
controllers/solrcloud_clusterop_requeue_test.go | 272 ++++++++++++++++++++++++
controllers/solrcloud_controller.go | 17 +-
helm/solr-operator/Chart.yaml | 7 +
3 files changed, 294 insertions(+), 2 deletions(-)
diff --git a/controllers/solrcloud_clusterop_requeue_test.go
b/controllers/solrcloud_clusterop_requeue_test.go
new file mode 100644
index 0000000..b2edc01
--- /dev/null
+++ b/controllers/solrcloud_clusterop_requeue_test.go
@@ -0,0 +1,272 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package controllers
+
+import (
+ "context"
+ "encoding/json"
+ "errors"
+ "net/http"
+ "testing"
+ "time"
+
+ solrv1beta1 "github.com/apache/solr-operator/api/v1beta1"
+ "github.com/apache/solr-operator/controllers/util"
+ "github.com/apache/solr-operator/controllers/util/solr_api"
+ "github.com/go-logr/logr"
+ zkApi "github.com/pravega/zookeeper-operator/api/v1beta1"
+ appsv1 "k8s.io/api/apps/v1"
+ corev1 "k8s.io/api/core/v1"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "k8s.io/apimachinery/pkg/runtime"
+ "k8s.io/apimachinery/pkg/types"
+ clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+ "k8s.io/client-go/tools/record"
+ "k8s.io/utils/pointer"
+ ctrl "sigs.k8s.io/controller-runtime"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+ "sigs.k8s.io/controller-runtime/pkg/client/fake"
+ "sigs.k8s.io/controller-runtime/pkg/log"
+)
+
+// errSolrUnreachable simulates the transient loss of connectivity to Solr
that stalled cluster
+// operations before they were guaranteed to be requeued.
+var errSolrUnreachable = errors.New("simulated i/o timeout talking to Solr")
+
+type unreachableSolrTransport struct{}
+
+func (unreachableSolrTransport) RoundTrip(*http.Request) (*http.Response,
error) {
+ return nil, errSolrUnreachable
+}
+
+// makeSolrUnreachable forces every Solr HTTP call to fail. The failing client
goes in the mTLS slot,
+// which the Solr API calls prefer over the default client. Only main.go ever
fills that slot, so it
+// is nil in tests and resetting it to nil restores exactly what was there
before.
+func makeSolrUnreachable(t *testing.T) {
+ t.Helper()
+ solr_api.SetMTLSHttpClient(&http.Client{Transport:
unreachableSolrTransport{}})
+ t.Cleanup(func() {
+ solr_api.SetMTLSHttpClient(nil)
+ })
+}
+
+// clusterOpFixture builds the state of a SolrCloud with 4 running pods, a
requested size of 3 pods
+// and the given cluster operation already locked in on its StatefulSet.
+func clusterOpFixture(t *testing.T, clusterOp SolrClusterOp)
(*SolrCloudReconciler, *solrv1beta1.SolrCloud, *appsv1.StatefulSet) {
+ t.Helper()
+
+ scheme := runtime.NewScheme()
+ if err := clientgoscheme.AddToScheme(scheme); err != nil {
+ t.Fatal(err)
+ }
+ if err := solrv1beta1.AddToScheme(scheme); err != nil {
+ t.Fatal(err)
+ }
+ if err := zkApi.AddToScheme(scheme); err != nil {
+ t.Fatal(err)
+ }
+
+ instance := &solrv1beta1.SolrCloud{
+ ObjectMeta: metav1.ObjectMeta{Name: "example", Namespace:
"default"},
+ Spec: solrv1beta1.SolrCloudSpec{
+ Replicas: pointer.Int32(3),
+ ZookeeperRef: &solrv1beta1.ZookeeperRef{
+ ConnectionInfo:
&solrv1beta1.ZookeeperConnectionInfo{
+ InternalConnectionString:
"zoo-headless:2181",
+ ChRoot: "/example",
+ },
+ },
+ },
+ }
+ // Apply the defaults up-front, otherwise the first reconcile would
just persist them and requeue.
+ instance.WithDefaults(logr.Discard())
+
+ statefulSet := &appsv1.StatefulSet{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: instance.StatefulSetName(),
+ Namespace: instance.Namespace,
+ UID: "statefulset-uid",
+ Annotations: map[string]string{
+ util.ClusterOpsLockAnnotation:
clusterOpAnnotation(t, clusterOp),
+ },
+ },
+ Spec: appsv1.StatefulSetSpec{
+ Replicas: pointer.Int32(4),
+ Selector: &metav1.LabelSelector{MatchLabels:
solrPodLabels(instance)},
+ },
+ Status: appsv1.StatefulSetStatus{Replicas: 4, ReadyReplicas: 4,
UpdatedReplicas: 4},
+ }
+
+ objects := []client.Object{instance, statefulSet}
+ // One pod per StatefulSet replica, i.e. including the pod that is
about to be scaled down.
+ for i := 0; i < int(*statefulSet.Spec.Replicas); i++ {
+ objects = append(objects, readySolrPod(instance, statefulSet,
instance.GetSolrPodName(i)))
+ }
+
+ r := &SolrCloudReconciler{
+ Client: fake.NewClientBuilder().
+ WithScheme(scheme).
+ WithObjects(objects...).
+ WithStatusSubresource(&solrv1beta1.SolrCloud{},
&appsv1.StatefulSet{}, &corev1.Pod{}).
+ Build(),
+ Scheme: scheme,
+ Recorder: record.NewFakeRecorder(64),
+ }
+ return r, instance, statefulSet
+}
+
+func clusterOpAnnotation(t *testing.T, op SolrClusterOp) string {
+ t.Helper()
+ bytes, err := json.Marshal(op)
+ if err != nil {
+ t.Fatal(err)
+ }
+ return string(bytes)
+}
+
+// solrPodLabels returns the labels that the operator uses to find the pods of
a SolrCloud.
+func solrPodLabels(instance *solrv1beta1.SolrCloud) map[string]string {
+ labels := instance.SharedLabels()
+ labels["technology"] = solrv1beta1.SolrTechnologyLabel
+ return labels
+}
+
+// readySolrPod builds a ready Solr pod owned by the given StatefulSet.
+func readySolrPod(instance *solrv1beta1.SolrCloud, statefulSet
*appsv1.StatefulSet, podName string) *corev1.Pod {
+ return &corev1.Pod{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: podName,
+ Namespace: instance.Namespace,
+ Labels: solrPodLabels(instance),
+ OwnerReferences: []metav1.OwnerReference{{UID:
statefulSet.UID, Name: statefulSet.Name}},
+ },
+ Spec: corev1.PodSpec{
+ Containers: []corev1.Container{{Name:
util.SolrNodeContainer, Image: instance.Spec.SolrImage.ToImageName()}},
+ },
+ Status: corev1.PodStatus{
+ Conditions: []corev1.PodCondition{{Type:
corev1.PodReady, Status: corev1.ConditionTrue}},
+ ContainerStatuses: []corev1.ContainerStatus{{Name:
util.SolrNodeContainer, Started: pointer.Bool(true)}},
+ },
+ }
+}
+
+// TestClusterOpRequeuedWhenSolrIsUnreachable verifies that a cluster
operation that fails because
+// Solr cannot be reached is requeued, so that the operation continues once
Solr is reachable again.
+// Regression test for https://github.com/apache/solr-operator/issues/707,
where a transiently failed
+// scale down was never retried because nothing triggered another reconcile.
+func TestClusterOpRequeuedWhenSolrIsUnreachable(t *testing.T) {
+ makeSolrUnreachable(t)
+ r, instance, statefulSet := clusterOpFixture(t, SolrClusterOp{
+ Operation: ScaleDownLock,
+ Metadata: "3",
+ LastStartTime: metav1.Now(),
+ })
+
+ ctx := log.IntoContext(context.Background(), logr.Discard())
+ result, err := r.Reconcile(ctx, ctrl.Request{NamespacedName:
types.NamespacedName{Name: instance.Name, Namespace: instance.Namespace}})
+ if err != nil {
+ t.Fatalf("did not expect the reconcile to fail, got %v", err)
+ }
+ if result.RequeueAfter <= 0 {
+ t.Errorf("expected the failed scale-down to be requeued, got
%+v", result)
+ }
+
+ // The operation should still hold the lock, so that it continues where
it left off.
+ if err = r.Get(ctx, client.ObjectKeyFromObject(statefulSet),
statefulSet); err != nil {
+ t.Fatal(err)
+ }
+ clusterOp, err := GetCurrentClusterOp(statefulSet)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if clusterOp == nil || clusterOp.Operation != ScaleDownLock {
+ t.Errorf("expected the scale-down lock to still be held, got
%+v", clusterOp)
+ }
+ if *statefulSet.Spec.Replicas != 4 {
+ t.Errorf("expected the StatefulSet to not be scaled down while
Solr is unreachable, got %d replicas", *statefulSet.Spec.Replicas)
+ }
+}
+
+// TestClusterOpQueuedForRetryAfterTimeoutWhenSolrIsUnreachable verifies that
a cluster operation
+// that keeps failing is eventually given up on, queued for a later retry and
its lock released, so
+// that other cluster operations are not blocked forever.
+func TestClusterOpQueuedForRetryAfterTimeoutWhenSolrIsUnreachable(t
*testing.T) {
+ makeSolrUnreachable(t)
+ r, instance, statefulSet := clusterOpFixture(t, SolrClusterOp{
+ Operation: ScaleDownLock,
+ Metadata: "3",
+ LastStartTime: metav1.NewTime(time.Now().Add(-2 * time.Minute)),
+ })
+
+ ctx := log.IntoContext(context.Background(), logr.Discard())
+ if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName:
types.NamespacedName{Name: instance.Name, Namespace: instance.Namespace}}); err
!= nil {
+ t.Fatalf("did not expect the reconcile to fail, got %v", err)
+ }
+
+ if err := r.Get(ctx, client.ObjectKeyFromObject(statefulSet),
statefulSet); err != nil {
+ t.Fatal(err)
+ }
+ if clusterOp, err := GetCurrentClusterOp(statefulSet); err != nil {
+ t.Fatal(err)
+ } else if clusterOp != nil {
+ t.Errorf("expected the scale-down lock to be released after the
operation timed out, got %+v", clusterOp)
+ }
+ queue, err := GetClusterOpRetryQueue(statefulSet)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(queue) != 1 || queue[0].Operation != ScaleDownLock {
+ t.Errorf("expected the scale-down to be queued for a later
retry, got %+v", queue)
+ }
+}
+
+// TestFailingLongRunningClusterOpQueuedForRetryAfterAMinute verifies that a
cluster operation that
+// is allowed to run for a long time (here a PVC expansion) is queued for a
later retry as soon as it
+// has been failing for a minute, instead of holding its lock for the full
long timeout.
+func TestFailingLongRunningClusterOpQueuedForRetryAfterAMinute(t *testing.T) {
+ r, instance, statefulSet := clusterOpFixture(t, SolrClusterOp{
+ Operation: PvcExpansionLock,
+ Metadata: "not-a-size",
+ LastStartTime: metav1.NewTime(time.Now().Add(-2 * time.Minute)),
+ })
+
+ ctx := log.IntoContext(context.Background(), logr.Discard())
+ result, err := r.Reconcile(ctx, ctrl.Request{NamespacedName:
types.NamespacedName{Name: instance.Name, Namespace: instance.Namespace}})
+ if err != nil {
+ t.Fatalf("did not expect the reconcile to fail, got %v", err)
+ }
+ if result.RequeueAfter <= 0 {
+ t.Errorf("expected the failed PVC expansion to be requeued, got
%+v", result)
+ }
+
+ if err = r.Get(ctx, client.ObjectKeyFromObject(statefulSet),
statefulSet); err != nil {
+ t.Fatal(err)
+ }
+ if clusterOp, opErr := GetCurrentClusterOp(statefulSet); opErr != nil {
+ t.Fatal(opErr)
+ } else if clusterOp != nil {
+ t.Errorf("expected the PVC expansion lock to be released after
a minute of errors, got %+v", clusterOp)
+ }
+ queue, err := GetClusterOpRetryQueue(statefulSet)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(queue) != 1 || queue[0].Operation != PvcExpansionLock {
+ t.Errorf("expected the PVC expansion to be queued for a later
retry, got %+v", queue)
+ }
+}
diff --git a/controllers/solrcloud_controller.go
b/controllers/solrcloud_controller.go
index cc0b249..203d6e1 100644
--- a/controllers/solrcloud_controller.go
+++ b/controllers/solrcloud_controller.go
@@ -510,7 +510,9 @@ func (r *SolrCloudReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
case PvcExpansionLock:
operationComplete, retryLaterDuration, err =
handlePvcExpansion(ctx, r, instance, statefulSet, clusterOp, logger)
// PVC expansion (the controller-side volume resize)
can take a long time on some provisioners,
- // so it should use the long requeue timeout rather
than being preempted after a minute.
+ // so a slow-but-progressing expansion should use the
long requeue timeout rather than being
+ // preempted after a minute. An expansion that is
erroring is still queued for a later retry
+ // after a minute, like any other failing operation.
shortTimeoutForRequeue = false
default:
operationFound = false
@@ -521,6 +523,10 @@ func (r *SolrCloudReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
err = clearClusterOpLockWithPatch(ctx, r, statefulSet,
"clusterOp not supported", logger)
}
if operationFound {
+ // The error returned by the cluster operation is not
returned by the reconcile, because the
+ // clusterOp has its own retry mechanism (it will be
queued for a later retry if it keeps failing).
+ // The error is still needed below, to decide whether
the operation should be queued for a later retry.
+ clusterOpError := err
err = nil
if operationComplete {
if nextClusterOperation == nil {
@@ -545,7 +551,7 @@ func (r *SolrCloudReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
clusterOpRuntime :=
time.Since(clusterOp.LastStartTime.Time)
queueForLaterReason := ""
- if err != nil && clusterOpRuntime > time.Minute
{
+ if clusterOpError != nil && clusterOpRuntime >
time.Minute {
queueForLaterReason = "hit an error
during operation"
} else if shortTimeoutForRequeue &&
clusterOpRuntime > time.Minute {
queueForLaterReason = "timed out during
operation (1 minutes)"
@@ -568,6 +574,13 @@ func (r *SolrCloudReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
"Pausing cluster operation %q
because it %s; it has been queued to be retried later",
string(clusterOp.Operation), queueForLaterReason)
}
}
+
+ // A failed cluster operation must always be retried,
otherwise a transient error (e.g. Solr
+ // being temporarily unreachable) would stall the
operation forever, since nothing else is
+ // guaranteed to trigger another reconcile for this
SolrCloud.
+ if clusterOpError != nil && retryLaterDuration == 0 {
+ retryLaterDuration = time.Second * 5
+ }
}
} else if opErr == nil {
if clusterOpQueue, opErr :=
GetClusterOpRetryQueue(statefulSet); opErr == nil {
diff --git a/helm/solr-operator/Chart.yaml b/helm/solr-operator/Chart.yaml
index 65737b2..1fc22f4 100644
--- a/helm/solr-operator/Chart.yaml
+++ b/helm/solr-operator/Chart.yaml
@@ -155,6 +155,13 @@ annotations:
url: https://solr.apache.org/docs/9_11_0/changes/Changes.html
- name: Solr Security News
url: https://solr.apache.org/security-news.html
+ - kind: fixed
+ description: A cluster operation (scale up/down, rolling update, replica
balancing, PVC expansion) that fails, e.g. because Solr is temporarily
unreachable, is now always retried instead of stalling the SolrCloud until
something else triggers a reconcile
+ links:
+ - name: Github Issue
+ url: https://github.com/apache/solr-operator/issues/707
+ - name: Github PR
+ url: https://github.com/apache/solr-operator/pull/876
artifacthub.io/images: |
- name: solr-operator
image: apache/solr-operator:v0.10.0-prerelease
