Copilot commented on code in PR #894:
URL: https://github.com/apache/solr-operator/pull/894#discussion_r4508756805
##########
controllers/solrcloud_controller.go:
##########
@@ -703,6 +725,22 @@ func (r *SolrCloudReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
return requeueOrNot, serviceCleanupError
}
+ // Keep the replica placement plugin in sync with the spec, once there
is a pod to talk to
+ newStatus.AppliedReplicaPlacementFactory =
instance.Status.AppliedReplicaPlacementFactory
Review Comment:
The applied hash is carried forward even when the SolrCloud changes
ZooKeeper ensemble or chroot. After the pods connect to a fresh target, an
unchanged `replicaPlacementFactory` still matches this old hash, so
reconciliation skips Solr and the new cluster never receives the configured
plugin. Scope the cached application state to the ZooKeeper target, and only
record success after applying against pods connected to that target.
##########
controllers/util/solr_placement_util.go:
##########
@@ -0,0 +1,106 @@
+/*
+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 util
+
+import (
+ "context"
+ "crypto/md5"
+ "encoding/json"
+ "fmt"
+
+ solr "github.com/apache/solr-operator/api/v1beta1"
+ "github.com/apache/solr-operator/controllers/util/solr_api"
+ "github.com/go-logr/logr"
+)
+
+// MinSolrVersionForClusterPlugins is the first Solr major version with the
/api/cluster/plugin API.
+const MinSolrVersionForClusterPlugins = 9
+
+// DesiredReplicaPlacementPlugin returns the cluster plugin that the
SolrCloud's spec asks for, and a
+// hash identifying it. Both are empty when the SolrCloud configures no
placement plugin.
+func DesiredReplicaPlacementPlugin(solrCloud *solr.SolrCloud) (plugin
*solr_api.SolrClusterPlugin, hash string, err error) {
+ if solrCloud.Spec.ReplicaPlacementFactory == nil {
+ return nil, "", nil
+ }
+
+ plugin = &solr_api.SolrClusterPlugin{
+ Name: solr_api.PlacementPluginName,
+ Class: solrCloud.Spec.ReplicaPlacementFactory.Class,
+ }
+ if config := solrCloud.Spec.ReplicaPlacementFactory.Config; config !=
nil && len(config.Raw) > 0 {
+ // Round-trip the config so that the hash does not change with
insignificant formatting
+ var normalized interface{}
+ if err = json.Unmarshal(config.Raw, &normalized); err != nil {
Review Comment:
Decoding into `interface{}` converts JSON numbers to `float64`, so
9007199254740993 becomes 9007199254740992 in the submitted configuration. Those
distinct values also produce the same hash, preventing an update when switching
between them. Normalize with a `json.Decoder` using `UseNumber()`, retain
rejection of trailing invalid JSON, and add a large-integer regression test.
##########
controllers/solrcloud_controller.go:
##########
@@ -703,6 +725,22 @@ func (r *SolrCloudReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
return requeueOrNot, serviceCleanupError
}
+ // Keep the replica placement plugin in sync with the spec, once there
is a pod to talk to
+ newStatus.AppliedReplicaPlacementFactory =
instance.Status.AppliedReplicaPlacementFactory
+ if instance.Spec.ReplicaPlacementFactory != nil &&
!instance.SolrVersionAtLeast(util.MinSolrVersionForClusterPlugins) {
+ r.Recorder.Eventf(instance, corev1.EventTypeWarning,
"ReplicaPlacementPluginUnsupported",
+ "Ignoring replicaPlacementFactory, which requires Solr
%d or later", util.MinSolrVersionForClusterPlugins)
+ } else if hasReadyPod {
Review Comment:
The Solr-version guard only applies while `replicaPlacementFactory` is
present. If a previously managed Solr 9 cloud is rolled back to Solr 8 and the
section is removed, its nonempty applied hash enters this branch and repeatedly
calls the unsupported cluster-plugin API every 15 seconds. Gate the removal
path on the supported version too, retaining the hash until a supported Solr
version can remove the plugin.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
