This is an automated email from the ASF dual-hosted git repository.
houston 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 1fbc6aa Remove unused ingress and services during reconcile (#674)
1fbc6aa is described below
commit 1fbc6aad60c1e96fe4f0387d58acefb4ac4bd519
Author: Jan Høydahl <[email protected]>
AuthorDate: Thu Jan 16 00:23:29 2025 +0100
Remove unused ingress and services during reconcile (#674)
Co-authored-by: Jan Høydahl <[email protected]>
Co-authored-by: Jason Gerlowski <[email protected]>
Co-authored-by: Houston Putman <[email protected]>
---
controllers/controller_utils_test.go | 25 +++-
controllers/solrcloud_controller.go | 99 ++++++++++++++--
controllers/solrcloud_controller_backup_test.go | 2 +
controllers/solrcloud_controller_ingress_test.go | 87 ++++++++++++++
controllers/solrcloud_controller_test.go | 2 +-
controllers/util/common.go | 12 +-
controllers/util/solr_util.go | 26 +++--
helm/solr-operator/Chart.yaml | 7 ++
tests/e2e/resource_utils_test.go | 116 +++++++------------
tests/e2e/solrcloud_ingress_test.go | 138 +++++++++++++++++++++++
10 files changed, 417 insertions(+), 97 deletions(-)
diff --git a/controllers/controller_utils_test.go
b/controllers/controller_utils_test.go
index 561ccac..6cdb14f 100644
--- a/controllers/controller_utils_test.go
+++ b/controllers/controller_utils_test.go
@@ -241,7 +241,7 @@ func expectService(ctx context.Context, parentResource
client.Object, serviceNam
func expectServiceWithChecks(ctx context.Context, parentResource
client.Object, serviceName string, selectorLables map[string]string, isHeadless
bool, additionalChecks func(Gomega, *corev1.Service), additionalOffset ...int)
*corev1.Service {
service := &corev1.Service{}
EventuallyWithOffset(resolveOffset(additionalOffset), func(g Gomega) {
- Expect(k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), service)).To(Succeed(), "Expected Service does not exist")
+ g.Expect(k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), service)).To(Succeed(), "Expected Service does not exist")
g.Expect(service.Spec.Selector).To(Equal(selectorLables),
"Service is not pointing to the correct Pods.")
@@ -275,7 +275,7 @@ func expectServiceWithChecks(ctx context.Context,
parentResource client.Object,
func expectServiceWithConsistentChecks(ctx context.Context, parentResource
client.Object, serviceName string, selectorLables map[string]string, isHeadless
bool, additionalChecks func(Gomega, *corev1.Service), additionalOffset ...int)
*corev1.Service {
service := &corev1.Service{}
ConsistentlyWithOffset(resolveOffset(additionalOffset), func(g Gomega) {
- Expect(k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), service)).To(Succeed(), "Expected Service does not exist")
+ g.Expect(k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), service)).To(Succeed(), "Expected Service does not exist")
g.Expect(service.Spec.Selector).To(Equal(selectorLables),
"Service is not pointing to the correct Pods.")
@@ -299,10 +299,23 @@ func expectNoService(ctx context.Context, parentResource
client.Object, serviceN
}).Should(MatchError("services \""+serviceName+"\" not found"),
message, "Service exists when it should not")
}
+func eventuallyExpectNoService(ctx context.Context, parentResource
client.Object, serviceName string, message string, additionalOffset ...int) {
+ EventuallyWithOffset(resolveOffset(additionalOffset), func() error {
+ return k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), &corev1.Service{})
+ }).Should(MatchError("services \""+serviceName+"\" not found"),
message, "Service exists when it should not")
+}
+
func expectNoServices(ctx context.Context, parentResource client.Object,
message string, serviceNames []string, additionalOffset ...int) {
ConsistentlyWithOffset(resolveOffset(additionalOffset), func(g Gomega) {
for _, serviceName := range serviceNames {
- g.Expect(k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), &corev1.Service{})).To(MatchError("services \""+serviceName+"\"
not found"), message)
+ g.Expect(k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), &corev1.Service{})).To(MatchError("services \""+serviceName+"\"
not found"), message, "service", serviceName)
+ }
+ }).Should(Succeed())
+}
+func eventuallyExpectNoServices(ctx context.Context, parentResource
client.Object, message string, serviceNames []string, additionalOffset ...int) {
+ EventuallyWithOffset(resolveOffset(additionalOffset), func(g Gomega) {
+ for _, serviceName := range serviceNames {
+ g.Expect(k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), &corev1.Service{})).To(MatchError("services \""+serviceName+"\"
not found"), message, "service", serviceName)
}
}).Should(Succeed())
}
@@ -356,6 +369,12 @@ func expectNoIngress(ctx context.Context, parentResource
client.Object, ingressN
}).Should(MatchError("ingresses.networking.k8s.io \""+ingressName+"\"
not found"), "Ingress exists when it should not")
}
+func eventuallyExpectNoIngress(ctx context.Context, parentResource
client.Object, ingressName string, additionalOffset ...int) {
+ EventuallyWithOffset(resolveOffset(additionalOffset), func() error {
+ return k8sClient.Get(ctx, resourceKey(parentResource,
ingressName), &netv1.Ingress{})
+ }).Should(MatchError("ingresses.networking.k8s.io \""+ingressName+"\"
not found"), "Ingress exists when it should not")
+}
+
func expectPodDisruptionBudget(ctx context.Context, parentResource
client.Object, podDisruptionBudgetName string, selector *metav1.LabelSelector,
maxUnavailable intstr.IntOrString, additionalOffset ...int)
*policyv1.PodDisruptionBudget {
return expectPodDisruptionBudgetWithChecks(ctx, parentResource,
podDisruptionBudgetName, selector, maxUnavailable, nil,
resolveOffset(additionalOffset))
}
diff --git a/controllers/solrcloud_controller.go
b/controllers/solrcloud_controller.go
index 9940ff9..affc220 100644
--- a/controllers/solrcloud_controller.go
+++ b/controllers/solrcloud_controller.go
@@ -326,7 +326,6 @@ func (r *SolrCloudReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
}
var statefulSet *appsv1.StatefulSet
-
if !blockReconciliationOfStatefulSet {
// Generate StatefulSet that should exist
expectedStatefulSet := util.GenerateStatefulSet(instance,
&newStatus, hostNameIpMap, reconcileConfigInfo, tls, security)
@@ -380,12 +379,12 @@ func (r *SolrCloudReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
}
} else {
// If we are blocking the reconciliation of the statefulSet, we
still want to find information about it.
- err = r.Get(ctx, types.NamespacedName{Name:
instance.StatefulSetName(), Namespace: instance.Namespace}, statefulSet)
- if err != nil {
- if !errors.IsNotFound(err) {
- return requeueOrNot, err
- } else {
+ statefulSet = &appsv1.StatefulSet{}
+ if e := r.Get(ctx, types.NamespacedName{Name:
instance.StatefulSetName(), Namespace: instance.Namespace}, statefulSet); e !=
nil {
+ if errors.IsNotFound(e) {
statefulSet = nil
+ } else {
+ err = e
}
}
}
@@ -424,6 +423,17 @@ func (r *SolrCloudReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
if err != nil {
return requeueOrNot, err
}
+ } else {
+ // If ingress exists, delete it
+ foundIngress := &netv1.Ingress{}
+ err = r.Get(ctx, types.NamespacedName{Name:
instance.CommonIngressName(), Namespace: instance.GetNamespace()}, foundIngress)
+ if err == nil {
+ err = r.Delete(ctx, foundIngress)
+ if err != nil {
+ return requeueOrNot, err
+ }
+ logger.Info("Deleted Ingress")
+ }
}
// *********************************************************
@@ -638,6 +648,12 @@ func (r *SolrCloudReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
}
}
+ // Remove unused services if necessary
+ err = r.cleanupUnconfiguredServices(ctx, instance, podList, logger)
+ if err != nil && !errors.IsNotFound(err) {
+ return requeueOrNot, err
+ }
+
if !reflect.DeepEqual(instance.Status, newStatus) {
logger.Info("Updating SolrCloud Status", "status", newStatus)
oldInstance := instance.DeepCopy()
@@ -651,7 +667,76 @@ func (r *SolrCloudReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
return requeueOrNot, err
}
-// InitializePods Ensure that all SolrCloud Pods are initialized
+// cleanupUnconfiguredServices remove services that are no longer defined by
the SolrCloud resource, and no longer in use by pods.
+// This does not currently include removing per-node services that are no
longer in use because the SolrCloud has been scaled down.
+func (r *SolrCloudReconciler) cleanupUnconfiguredServices(ctx context.Context,
solrCloud *solrv1beta1.SolrCloud, podList []corev1.Pod, logger logr.Logger)
(err error) {
+ var onlyServiceTypeInUse string
+ if solrCloud.UsesHeadlessService() {
+ onlyServiceTypeInUse = util.HeadlessServiceType
+ } else if solrCloud.UsesIndividualNodeServices() {
+ onlyServiceTypeInUse = util.PerNodeServiceType
+ } else {
+ // This should never happen, there are only 2 options
+ return
+ }
+ for _, pod := range podList {
+ if pod.Annotations == nil &&
pod.Annotations[util.ServiceTypeAnnotation] != "" {
+ if onlyServiceTypeInUse !=
pod.Annotations[util.ServiceTypeAnnotation] {
+ // Only remove services if all pods are using
the same, and configured, type of service.
+ // Otherwise, we are in transition between
service types and need to wait to delete anything.
+ return
+ }
+ } else {
+ // If we have a pod missing this annotation, then it
has not been fully updated to a supported Operator version.
+ // We don't have the information, so assume both
serviceTypes are in use, and don't remove anything.
+ return
+ }
+ }
+
+ // If we are at this point, then we can assume we are completely
transitioned and can delete the unused services
+ if solrCloud.UsesHeadlessService() {
+ err = r.deleteServicesOfType(ctx, solrCloud,
util.PerNodeServiceType, logger)
+ } else if solrCloud.UsesIndividualNodeServices() {
+ err = r.deleteServicesOfType(ctx, solrCloud,
util.HeadlessServiceType, logger)
+ }
+ return
+}
+
+func (r *SolrCloudReconciler) deleteServicesOfType(ctx context.Context,
solrCloud *solrv1beta1.SolrCloud, serviceType string, logger logr.Logger) (err
error) {
+ foundServices := &corev1.ServiceList{}
+ selectorLabels := solrCloud.SharedLabels()
+ selectorLabels[util.ServiceTypeAnnotation] = serviceType
+
+ serviceSelector := labels.SelectorFromSet(selectorLabels)
+ listOps := &client.ListOptions{
+ Namespace: solrCloud.Namespace,
+ LabelSelector: serviceSelector,
+ }
+
+ if err = r.List(ctx, foundServices, listOps); err != nil {
+ logger.Error(err, "Error listing services for SolrCloud",
"serviceType", serviceType)
+ return
+ }
+
+ for _, headlessService := range foundServices.Items {
+ if e := r.deleteService(ctx, &headlessService, logger); e !=
nil {
+ // Don't break, just add the error for later
+ err = e
+ }
+ }
+ return
+}
+
+func (r *SolrCloudReconciler) deleteService(ctx context.Context, service
*corev1.Service, logger logr.Logger) (err error) {
+ logger.Info("Deleting Service for SolrCloud", "Service", service.Name)
+ err = r.Client.Delete(ctx, service)
+ if err != nil && !errors.IsNotFound(err) {
+ logger.Error(err, "Error deleting unused Service for
SolrCloud", "Service", service.Name)
+ }
+ return
+}
+
+// initializePods Ensure that all SolrCloud Pods are initialized
func (r *SolrCloudReconciler) initializePods(ctx context.Context, solrCloud
*solrv1beta1.SolrCloud, statefulSet *appsv1.StatefulSet, logger logr.Logger)
(podSelector labels.Selector, podList []corev1.Pod, err error) {
foundPods := &corev1.PodList{}
selectorLabels := solrCloud.SharedLabels()
diff --git a/controllers/solrcloud_controller_backup_test.go
b/controllers/solrcloud_controller_backup_test.go
index a0be8d5..7f223fc 100644
--- a/controllers/solrcloud_controller_backup_test.go
+++ b/controllers/solrcloud_controller_backup_test.go
@@ -98,6 +98,7 @@ var _ = FDescribe("SolrCloud controller - Backup
Repositories", func() {
Expect(statefulSet.Spec.Template.Annotations).To(Equal(util.MergeLabelsOrAnnotations(testPodAnnotations,
map[string]string{
"solr.apache.org/solrXmlMd5":
solrXmlMd5,
util.SolrBackupRepositoriesAnnotation:
"test-repo",
+ util.ServiceTypeAnnotation:
util.HeadlessServiceType,
})), "Incorrect pod annotations")
// Env Variable Tests
@@ -321,6 +322,7 @@ var _ = FDescribe("SolrCloud controller - Backup
Repositories", func() {
Expect(statefulSet.Spec.Template.Annotations).To(Equal(map[string]string{
"solr.apache.org/solrXmlMd5":
fmt.Sprintf("%x", md5.Sum([]byte(configMap.Data["solr.xml"]))),
util.SolrBackupRepositoriesAnnotation:
"another,test-repo",
+ util.ServiceTypeAnnotation:
util.HeadlessServiceType,
}), "Incorrect pod annotations")
})
})
diff --git a/controllers/solrcloud_controller_ingress_test.go
b/controllers/solrcloud_controller_ingress_test.go
index 8a616a9..87fb57b 100644
--- a/controllers/solrcloud_controller_ingress_test.go
+++ b/controllers/solrcloud_controller_ingress_test.go
@@ -163,6 +163,92 @@ var _ = FDescribe("SolrCloud controller - Ingress", func()
{
})
})
+ FContext("Full Ingress - Switch off after creation", func() {
+ BeforeEach(func() {
+ solrCloud.Spec.SolrAddressability =
solrv1beta1.SolrAddressabilityOptions{
+ External: &solrv1beta1.ExternalAddressability{
+ Method: solrv1beta1.Ingress,
+ UseExternalAddress: true,
+ DomainName: testDomain,
+ NodePortOverride: 100,
+ },
+ PodPort: 3000,
+ CommonServicePort: 4000,
+ }
+ })
+ FIt("has the correct resources", func(ctx context.Context) {
+ By("testing the Solr StatefulSet")
+ statefulSet := expectStatefulSet(ctx, solrCloud,
solrCloud.StatefulSetName())
+ // Pod Annotations test
+
Expect(statefulSet.Spec.Template.Annotations).To(HaveKeyWithValue(util.ServiceTypeAnnotation,
util.PerNodeServiceType), "Since external address is used for advertising, the
perNode service should be specified in the pod annotations.")
+
+ By("testing the Solr Common Service")
+ commonService := expectService(ctx, solrCloud,
solrCloud.CommonServiceName(), statefulSet.Spec.Selector.MatchLabels, false)
+
Expect(commonService.Annotations).To(Equal(testCommonServiceAnnotations),
"Incorrect common service annotations")
+
Expect(commonService.Spec.Ports[0].Name).To(Equal("solr-client"), "Wrong port
name on common Service")
+
Expect(commonService.Spec.Ports[0].Port).To(Equal(int32(4000)), "Wrong port on
common Service")
+
Expect(commonService.Spec.Ports[0].TargetPort.StrVal).To(Equal("solr-client"),
"Wrong podPort name on common Service")
+
+ By("ensuring the Solr Headless Service does not exist")
+ expectNoService(ctx, solrCloud,
solrCloud.HeadlessServiceName(), "Headless service shouldn't exist, but it
does.")
+
+ By("making sure the individual Solr Node Services exist
and route correctly")
+ nodeNames := solrCloud.GetAllSolrPodNames()
+ Expect(nodeNames).To(HaveLen(replicas), "SolrCloud has
incorrect number of nodeNames.")
+ for _, nodeName := range nodeNames {
+ service := expectService(ctx, solrCloud,
nodeName, util.MergeLabelsOrAnnotations(statefulSet.Spec.Selector.MatchLabels,
map[string]string{"statefulset.kubernetes.io/pod-name": nodeName}), false)
+ expectedNodeServiceLabels :=
util.MergeLabelsOrAnnotations(solrCloud.SharedLabelsWith(solrCloud.Labels),
map[string]string{"service-type": "external"})
+
Expect(service.Labels).To(Equal(util.MergeLabelsOrAnnotations(expectedNodeServiceLabels,
testNodeServiceLabels)), "Incorrect node '"+nodeName+"' service labels")
+
Expect(service.Annotations).To(Equal(testNodeServiceAnnotations), "Incorrect
node '"+nodeName+"' service annotations")
+
Expect(service.Spec.Ports[0].Name).To(Equal("solr-client"), "Wrong port name on
common Service")
+
Expect(service.Spec.Ports[0].Port).To(Equal(int32(100)), "Wrong port on node
Service")
+
Expect(service.Spec.Ports[0].TargetPort.StrVal).To(Equal("solr-client"), "Wrong
podPort name on node Service")
+ }
+
+ By("making sure Ingress was created correctly")
+ ingress := expectIngress(ctx, solrCloud,
solrCloud.CommonIngressName())
+
Expect(ingress.Labels).To(Equal(util.MergeLabelsOrAnnotations(solrCloud.SharedLabelsWith(solrCloud.Labels),
testIngressLabels)), "Incorrect ingress labels")
+
Expect(ingress.Annotations).To(Equal(ingressLabelsWithDefaults(testIngressAnnotations)),
"Incorrect ingress annotations")
+ Expect(ingress.Spec.IngressClassName).To(Not(BeNil()),
"Ingress class name should not be nil")
+
Expect(*ingress.Spec.IngressClassName).To(Equal(testIngressClass), "Incorrect
ingress class name")
+ testIngressRules(solrCloud, ingress, true, replicas,
4000, 100, testDomain)
+
+ By("making sure the node addresses in the Status are
correct")
+ expectSolrCloudStatusWithChecks(ctx, solrCloud, func(g
Gomega, found *solrv1beta1.SolrCloudStatus) {
+
g.Expect(found.InternalCommonAddress).To(Equal("http://"+solrCloud.CommonServiceName()+"."+solrCloud.Namespace+":4000"),
"Wrong internal common address in status")
+
g.Expect(found.ExternalCommonAddress).To(Not(BeNil()), "External common address
in status should not be nil")
+
g.Expect(*found.ExternalCommonAddress).To(Equal("http://"+solrCloud.Namespace+"-"+solrCloud.Name+"-solrcloud"+"."+testDomain),
"Wrong external common address in status")
+ })
+
+ By("Turning off node external addressability and making
sure the node services are deleted")
+ expectSolrCloudWithChecks(ctx, solrCloud, func(g
Gomega, found *solrv1beta1.SolrCloud) {
+
found.Spec.SolrAddressability.External.UseExternalAddress = false
+
found.Spec.SolrAddressability.External.HideNodes = true
+ g.Expect(k8sClient.Update(ctx,
found)).To(Succeed(), "Couldn't update the solrCloud to not advertise the nodes
externally.")
+ })
+
+ // Since node external addressability is off, but
common external addressability is on, the ingress should exist, but the node
services should not
+ eventuallyExpectNoServices(ctx, solrCloud, "Node
services shouldn't exist after the update, but they do.", nodeNames)
+
+ expectIngress(ctx, solrCloud,
solrCloud.CommonIngressName())
+
+ headlessService := expectService(ctx, solrCloud,
solrCloud.HeadlessServiceName(), statefulSet.Spec.Selector.MatchLabels, true)
+
Expect(headlessService.Annotations).To(Equal(testHeadlessServiceAnnotations),
"Incorrect headless service annotations")
+
Expect(headlessService.Spec.Ports[0].Name).To(Equal("solr-client"), "Wrong port
name on common Service")
+
Expect(headlessService.Spec.Ports[0].Port).To(Equal(int32(3000)), "Wrong port
on headless Service")
+
Expect(headlessService.Spec.Ports[0].TargetPort.StrVal).To(Equal("solr-client"),
"Wrong podPort name on headless Service")
+
Expect(headlessService.Spec.Ports[0].Protocol).To(Equal(corev1.ProtocolTCP),
"Wrong protocol on headless Service")
+
Expect(headlessService.Spec.Ports[0].AppProtocol).To(BeNil(), "AppProtocol on
headless Service should be nil when not running with TLS")
+
+ By("Turning off common external addressability and
making sure the ingress is deleted")
+ expectSolrCloudWithChecks(ctx, solrCloud, func(g
Gomega, found *solrv1beta1.SolrCloud) {
+ found.Spec.SolrAddressability.External = nil
+ g.Expect(k8sClient.Update(ctx,
found)).To(Succeed(), "Couldn't update the solrCloud to remove external
addressability")
+ })
+ eventuallyExpectNoIngress(ctx, solrCloud,
solrCloud.CommonIngressName())
+ })
+ })
+
FContext("Hide Nodes from external connections - Using default ingress
class", func() {
ingressClass := &netv1.IngressClass{
ObjectMeta: metav1.ObjectMeta{
@@ -206,6 +292,7 @@ var _ = FDescribe("SolrCloud controller - Ingress", func() {
By("testing the Solr StatefulSet")
statefulSet := expectStatefulSet(ctx, solrCloud,
solrCloud.StatefulSetName())
+
Expect(statefulSet.Spec.Template.Annotations).To(HaveKeyWithValue(util.ServiceTypeAnnotation,
util.HeadlessServiceType), "Since external address is not used for
advertising, the headless service should be specified in the pod annotations.")
Expect(statefulSet.Spec.Template.Spec.Containers).To(HaveLen(1), "Solr
StatefulSet requires a container.")
diff --git a/controllers/solrcloud_controller_test.go
b/controllers/solrcloud_controller_test.go
index 1bd01ff..5dca7f0 100644
--- a/controllers/solrcloud_controller_test.go
+++ b/controllers/solrcloud_controller_test.go
@@ -280,7 +280,7 @@ var _ = FDescribe("SolrCloud controller - General", func() {
Expect(statefulSet.Spec.Template.ObjectMeta.Annotations).To(HaveKey(util.SolrScheduledRestartAnnotation),
"Pod Template does not have scheduled restart annotation when it should")
// Remove the annotation when we know that it exists,
we don't know the exact value so we can't check it below.
delete(statefulSet.Spec.Template.Annotations,
util.SolrScheduledRestartAnnotation)
-
Expect(statefulSet.Spec.Template.Annotations).To(Equal(util.MergeLabelsOrAnnotations(map[string]string{"solr.apache.org/solrXmlMd5":
fmt.Sprintf("%x", md5.Sum([]byte(configMap.Data["solr.xml"])))},
testPodAnnotations)), "Incorrect pod annotations")
+
Expect(statefulSet.Spec.Template.Annotations).To(Equal(util.MergeLabelsOrAnnotations(map[string]string{util.ServiceTypeAnnotation:
util.HeadlessServiceType, "solr.apache.org/solrXmlMd5": fmt.Sprintf("%x",
md5.Sum([]byte(configMap.Data["solr.xml"])))}, testPodAnnotations)), "Incorrect
pod annotations")
Expect(statefulSet.Spec.Template.Spec.NodeSelector).To(Equal(testNodeSelectors),
"Incorrect pod node selectors")
Expect(statefulSet.Spec.Template.Spec.Containers[0].LivenessProbe,
testProbeLivenessNonDefaults, "Incorrect Liveness Probe")
diff --git a/controllers/util/common.go b/controllers/util/common.go
index 9882519..7b2e5c9 100644
--- a/controllers/util/common.go
+++ b/controllers/util/common.go
@@ -431,8 +431,9 @@ func CopyPodTemplates(from, to *corev1.PodTemplateSpec,
basePath string, logger
requireUpdate = CopyPodVolumes(&from.Spec.Volumes, &to.Spec.Volumes,
basePath+"Spec.Volumes", logger) || requireUpdate
if !DeepEqualWithNils(to.Spec.HostAliases, from.Spec.HostAliases) {
- requireUpdate = true
+ hostAliasUpdate := false
if to.Spec.HostAliases == nil {
+ hostAliasUpdate = true
to.Spec.HostAliases = from.Spec.HostAliases
} else {
// Do not remove aliases that are no longer used.
@@ -443,19 +444,22 @@ func CopyPodTemplates(from, to *corev1.PodTemplateSpec,
basePath string, logger
if fromAlias.Hostnames[0] ==
toAlias.Hostnames[0] {
found = true
if !DeepEqualWithNils(toAlias,
fromAlias) {
- requireUpdate = true
+ hostAliasUpdate = true
to.Spec.HostAliases[i]
= fromAlias
break
}
}
}
if !found {
- requireUpdate = true
+ hostAliasUpdate = true
to.Spec.HostAliases =
append(to.Spec.HostAliases, fromAlias)
}
}
}
- logger.Info("Update required because field changed", "field",
basePath+"Spec.HostAliases", "from", to.Spec.HostAliases, "to",
from.Spec.HostAliases)
+ if hostAliasUpdate {
+ requireUpdate = true
+ logger.Info("Update required because field changed",
"field", basePath+"Spec.HostAliases", "from", to.Spec.HostAliases, "to",
from.Spec.HostAliases)
+ }
}
if !DeepEqualWithNils(to.Spec.ImagePullSecrets,
from.Spec.ImagePullSecrets) {
diff --git a/controllers/util/solr_util.go b/controllers/util/solr_util.go
index db4c408..478145a 100644
--- a/controllers/util/solr_util.go
+++ b/controllers/util/solr_util.go
@@ -52,6 +52,10 @@ const (
SolrXmlFile = "solr.xml"
LogXmlMd5Annotation = "solr.apache.org/logXmlMd5"
LogXmlFile = "log4j2.xml"
+ ServiceTypeAnnotation = "service-type" //
"solr.apache.org/serviceType"
+ HeadlessServiceType = "headless"
+ PerNodeServiceType = "external"
+ CommonServiceType = "common"
// Protected StatefulSet annotations
// These are to be saved on a statefulSet update
@@ -125,6 +129,16 @@ func GenerateStatefulSet(solrCloud *solr.SolrCloud,
solrCloudStatus *solr.SolrCl
}
shareProcessNamespace = customPodOptions.ShareProcessNamespace
}
+ if podAnnotations == nil {
+ podAnnotations = make(map[string]string, 1)
+ }
+ var serviceType string
+ if solrCloud.UsesHeadlessService() {
+ serviceType = HeadlessServiceType
+ } else if solrCloud.UsesIndividualNodeServices() {
+ serviceType = PerNodeServiceType
+ }
+ podAnnotations[ServiceTypeAnnotation] = serviceType
// The isNotStopped readiness gate will always be used for
managedUpdates
podReadinessGates := []corev1.PodReadinessGate{
@@ -413,9 +427,6 @@ func GenerateStatefulSet(solrCloud *solr.SolrCloud,
solrCloudStatus *solr.SolrCl
// Did the user provide a custom log config?
if reconcileConfigInfo[LogXmlFile] != "" {
if reconcileConfigInfo[LogXmlMd5Annotation] != "" {
- if podAnnotations == nil {
- podAnnotations = make(map[string]string, 1)
- }
podAnnotations[LogXmlMd5Annotation] =
reconcileConfigInfo[LogXmlMd5Annotation]
}
@@ -432,9 +443,6 @@ func GenerateStatefulSet(solrCloud *solr.SolrCloud,
solrCloudStatus *solr.SolrCl
// track the MD5 of the custom solr.xml in the pod spec annotations,
// so we get a rolling restart when the configMap changes
if reconcileConfigInfo[SolrXmlMd5Annotation] != "" {
- if podAnnotations == nil {
- podAnnotations = make(map[string]string, 1)
- }
podAnnotations[SolrXmlMd5Annotation] =
reconcileConfigInfo[SolrXmlMd5Annotation]
}
@@ -912,7 +920,7 @@ func getAppProtocol(solrCloud *solr.SolrCloud) *string {
// solrCloud: SolrCloud instance
func GenerateCommonService(solrCloud *solr.SolrCloud) *corev1.Service {
labels := solrCloud.SharedLabelsWith(solrCloud.GetLabels())
- labels["service-type"] = "common"
+ labels[ServiceTypeAnnotation] = CommonServiceType
selectorLabels := solrCloud.SharedLabels()
selectorLabels["technology"] = solr.SolrTechnologyLabel
@@ -964,7 +972,7 @@ func GenerateCommonService(solrCloud *solr.SolrCloud)
*corev1.Service {
// solrCloud: SolrCloud instance
func GenerateHeadlessService(solrCloud *solr.SolrCloud) *corev1.Service {
labels := solrCloud.SharedLabelsWith(solrCloud.GetLabels())
- labels["service-type"] = "headless"
+ labels[ServiceTypeAnnotation] = HeadlessServiceType
selectorLabels := solrCloud.SharedLabels()
selectorLabels["technology"] = solr.SolrTechnologyLabel
@@ -1019,7 +1027,7 @@ func GenerateHeadlessService(solrCloud *solr.SolrCloud)
*corev1.Service {
// nodeName: string node
func GenerateNodeService(solrCloud *solr.SolrCloud, nodeName string)
*corev1.Service {
labels := solrCloud.SharedLabelsWith(solrCloud.GetLabels())
- labels["service-type"] = "external"
+ labels[ServiceTypeAnnotation] = PerNodeServiceType
selectorLabels := solrCloud.SharedLabels()
selectorLabels["technology"] = solr.SolrTechnologyLabel
diff --git a/helm/solr-operator/Chart.yaml b/helm/solr-operator/Chart.yaml
index 2509c43..ec5275f 100644
--- a/helm/solr-operator/Chart.yaml
+++ b/helm/solr-operator/Chart.yaml
@@ -134,6 +134,13 @@ annotations:
url: https://github.com/apache/solr-operator/issues/489
- name: Github PR
url: https://github.com/apache/solr-operator/pull/743
+ - kind: fixed
+ description: The operator will now delete ingress and per-node services
when external ingress is disabled.
+ links:
+ - name: Github Issue
+ url: https://github.com/apache/solr-operator/issues/673
+ - name: Github PR
+ url: https://github.com/apache/solr-operator/pull/674
artifacthub.io/images: |
- name: solr-operator
image: apache/solr-operator:v0.9.0-prerelease
diff --git a/tests/e2e/resource_utils_test.go b/tests/e2e/resource_utils_test.go
index a58904c..31b88c6 100644
--- a/tests/e2e/resource_utils_test.go
+++ b/tests/e2e/resource_utils_test.go
@@ -346,6 +346,27 @@ func expectNoPodNow(ctx context.Context, parentResource
client.Object, podName s
).To(MatchError("pods \""+podName+"\" not found"), "Pod exists when it
should not")
}
+func expectPodNow(ctx context.Context, parentResource client.Object, podName
string, additionalOffset ...int) *corev1.Pod {
+ pod := &corev1.Pod{}
+ ExpectWithOffset(
+ resolveOffset(additionalOffset),
+ k8sClient.Get(ctx, resourceKey(parentResource, podName), pod),
+ ).To(Succeed(), "Pod should exist", "name", podName, "namespace",
parentResource.GetNamespace())
+ return pod
+}
+
+func expectPodWithChecks(ctx context.Context, parentResource client.Object,
podName string, additionalChecks func(Gomega, *corev1.Pod), additionalOffset
...int) *corev1.Pod {
+ pod := &corev1.Pod{}
+ EventuallyWithOffset(resolveOffset(additionalOffset), func(g Gomega) {
+ g.Expect(k8sClient.Get(ctx, resourceKey(parentResource,
podName), pod)).To(Succeed(), "Expected Pod does not exist")
+
+ if additionalChecks != nil {
+ additionalChecks(g, pod)
+ }
+ }).Should(Succeed())
+ return pod
+}
+
func expectService(ctx context.Context, parentResource client.Object,
serviceName string, selectorLables map[string]string, isHeadless bool,
additionalOffset ...int) *corev1.Service {
return expectServiceWithChecks(ctx, parentResource, serviceName,
selectorLables, isHeadless, nil, resolveOffset(additionalOffset))
}
@@ -353,7 +374,7 @@ func expectService(ctx context.Context, parentResource
client.Object, serviceNam
func expectServiceWithChecks(ctx context.Context, parentResource
client.Object, serviceName string, selectorLables map[string]string, isHeadless
bool, additionalChecks func(Gomega, *corev1.Service), additionalOffset ...int)
*corev1.Service {
service := &corev1.Service{}
EventuallyWithOffset(resolveOffset(additionalOffset), func(g Gomega) {
- Expect(k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), service)).To(Succeed(), "Expected Service does not exist")
+ g.Expect(k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), service)).To(Succeed(), "Expected Service does not exist")
g.Expect(service.Spec.Selector).To(Equal(selectorLables),
"Service is not pointing to the correct Pods.")
@@ -367,27 +388,13 @@ func expectServiceWithChecks(ctx context.Context,
parentResource client.Object,
additionalChecks(g, service)
}
}).Should(Succeed())
-
- By("recreating the Service after it is deleted")
- ExpectWithOffset(resolveOffset(additionalOffset), k8sClient.Delete(ctx,
service)).To(Succeed())
- EventuallyWithOffset(
- resolveOffset(additionalOffset),
- func() (types.UID, error) {
- newResource := &corev1.Service{}
- err := k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), newResource)
- if err != nil {
- return "", err
- }
- return newResource.UID, nil
- }).Should(And(Not(BeEmpty()), Not(Equal(service.UID))), "New
Service, with new UID, not created.")
-
return service
}
func expectServiceWithConsistentChecks(ctx context.Context, parentResource
client.Object, serviceName string, selectorLables map[string]string, isHeadless
bool, additionalChecks func(Gomega, *corev1.Service), additionalOffset ...int)
*corev1.Service {
service := &corev1.Service{}
ConsistentlyWithOffset(resolveOffset(additionalOffset), func(g Gomega) {
- Expect(k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), service)).To(Succeed(), "Expected Service does not exist")
+ g.Expect(k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), service)).To(Succeed(), "Expected Service does not exist")
g.Expect(service.Spec.Selector).To(Equal(selectorLables),
"Service is not pointing to the correct Pods.")
@@ -411,10 +418,23 @@ func expectNoService(ctx context.Context, parentResource
client.Object, serviceN
}).Should(MatchError("services \""+serviceName+"\" not found"),
message, "Service exists when it should not")
}
+func eventuallyExpectNoService(ctx context.Context, parentResource
client.Object, serviceName string, message string, additionalOffset ...int) {
+ EventuallyWithOffset(resolveOffset(additionalOffset), func() error {
+ return k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), &corev1.Service{})
+ }).Should(MatchError("services \""+serviceName+"\" not found"),
message, "Service exists when it should not")
+}
+
func expectNoServices(ctx context.Context, parentResource client.Object,
message string, serviceNames []string, additionalOffset ...int) {
ConsistentlyWithOffset(resolveOffset(additionalOffset), func(g Gomega) {
for _, serviceName := range serviceNames {
- g.Expect(k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), &corev1.Service{})).To(MatchError("services \""+serviceName+"\"
not found"), message)
+ g.Expect(k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), &corev1.Service{})).To(MatchError("services \""+serviceName+"\"
not found"), message, "service", serviceName)
+ }
+ }).Should(Succeed())
+}
+func eventuallyExpectNoServices(ctx context.Context, parentResource
client.Object, message string, serviceNames []string, additionalOffset ...int) {
+ EventuallyWithOffset(resolveOffset(additionalOffset), func(g Gomega) {
+ for _, serviceName := range serviceNames {
+ g.Expect(k8sClient.Get(ctx, resourceKey(parentResource,
serviceName), &corev1.Service{})).To(MatchError("services \""+serviceName+"\"
not found"), message, "service", serviceName)
}
}).Should(Succeed())
}
@@ -432,20 +452,6 @@ func expectIngressWithChecks(ctx context.Context,
parentResource client.Object,
additionalChecks(g, ingress)
}
}).Should(Succeed())
-
- By("recreating the Ingress after it is deleted")
- ExpectWithOffset(resolveOffset(additionalOffset), k8sClient.Delete(ctx,
ingress)).To(Succeed())
- EventuallyWithOffset(
- resolveOffset(additionalOffset),
- func() (types.UID, error) {
- newResource := &netv1.Ingress{}
- err := k8sClient.Get(ctx, resourceKey(parentResource,
ingressName), newResource)
- if err != nil {
- return "", err
- }
- return newResource.UID, nil
- }).Should(And(Not(BeEmpty()), Not(Equal(ingress.UID))), "New
Ingress, with new UID, not created.")
-
return ingress
}
@@ -468,6 +474,12 @@ func expectNoIngress(ctx context.Context, parentResource
client.Object, ingressN
}).Should(MatchError("ingresses.networking.k8s.io \""+ingressName+"\"
not found"), "Ingress exists when it should not")
}
+func eventuallyExpectNoIngress(ctx context.Context, parentResource
client.Object, ingressName string, additionalOffset ...int) {
+ EventuallyWithOffset(resolveOffset(additionalOffset), func() error {
+ return k8sClient.Get(ctx, resourceKey(parentResource,
ingressName), &netv1.Ingress{})
+ }).Should(MatchError("ingresses.networking.k8s.io \""+ingressName+"\"
not found"), "Ingress exists when it should not")
+}
+
func expectPodDisruptionBudget(ctx context.Context, parentResource
client.Object, podDisruptionBudgetName string, selector *metav1.LabelSelector,
maxUnavailable intstr.IntOrString, additionalOffset ...int)
*policyv1.PodDisruptionBudget {
return expectPodDisruptionBudgetWithChecks(ctx, parentResource,
podDisruptionBudgetName, selector, maxUnavailable, nil,
resolveOffset(additionalOffset))
}
@@ -485,20 +497,6 @@ func expectPodDisruptionBudgetWithChecks(ctx
context.Context, parentResource cli
additionalChecks(g, podDisruptionBudget)
}
}).Should(Succeed())
-
- By("recreating the PodDisruptionBudget after it is deleted")
- ExpectWithOffset(resolveOffset(additionalOffset), k8sClient.Delete(ctx,
podDisruptionBudget)).To(Succeed())
- EventuallyWithOffset(
- resolveOffset(additionalOffset),
- func() (types.UID, error) {
- newResource := &policyv1.PodDisruptionBudget{}
- err := k8sClient.Get(ctx, resourceKey(parentResource,
podDisruptionBudgetName), newResource)
- if err != nil {
- return "", err
- }
- return newResource.UID, nil
- }).Should(And(Not(BeEmpty()),
Not(Equal(podDisruptionBudget.UID))), "New PodDisruptionBudget, with new UID,
not created.")
-
return podDisruptionBudget
}
@@ -518,20 +516,6 @@ func expectConfigMapWithChecks(ctx context.Context,
parentResource client.Object
additionalChecks(g, configMap)
}
}).Should(Succeed())
-
- By("recreating the ConfigMap after it is deleted")
- ExpectWithOffset(resolveOffset(additionalOffset), k8sClient.Delete(ctx,
configMap)).To(Succeed())
- EventuallyWithOffset(
- resolveOffset(additionalOffset),
- func() (types.UID, error) {
- newResource := &corev1.ConfigMap{}
- err := k8sClient.Get(ctx, resourceKey(parentResource,
configMapName), newResource)
- if err != nil {
- return "", err
- }
- return newResource.UID, nil
- }).Should(And(Not(BeEmpty()), Not(Equal(configMap.UID))), "New
ConfigMap, with new UID, not created.")
-
return configMap
}
@@ -574,20 +558,6 @@ func expectDeploymentWithChecks(ctx context.Context,
parentResource client.Objec
additionalChecks(g, deployment)
}
}).Should(Succeed())
-
- By("recreating the Deployment after it is deleted")
- ExpectWithOffset(resolveOffset(additionalOffset), k8sClient.Delete(ctx,
deployment)).To(Succeed())
- EventuallyWithOffset(
- resolveOffset(additionalOffset),
- func() (types.UID, error) {
- newResource := &appsv1.Deployment{}
- err := k8sClient.Get(ctx, resourceKey(parentResource,
deploymentName), newResource)
- if err != nil {
- return "", err
- }
- return newResource.UID, nil
- }).Should(And(Not(BeEmpty()), Not(Equal(deployment.UID))), "New
Deployment, with new UID, not created.")
-
return deployment
}
diff --git a/tests/e2e/solrcloud_ingress_test.go
b/tests/e2e/solrcloud_ingress_test.go
new file mode 100644
index 0000000..e9a45ab
--- /dev/null
+++ b/tests/e2e/solrcloud_ingress_test.go
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+ "context"
+ solrv1beta1 "github.com/apache/solr-operator/api/v1beta1"
+ "github.com/apache/solr-operator/controllers/util"
+ . "github.com/onsi/ginkgo/v2"
+ . "github.com/onsi/gomega"
+ corev1 "k8s.io/api/core/v1"
+)
+
+var _ = FDescribe("E2E - SolrCloud - Ingress", func() {
+ var (
+ solrCloud *solrv1beta1.SolrCloud
+ )
+
+ BeforeEach(func() {
+ solrCloud = generateBaseSolrCloud(1)
+ solrCloud.Spec.SolrAddressability =
solrv1beta1.SolrAddressabilityOptions{
+ External: &solrv1beta1.ExternalAddressability{
+ Method: solrv1beta1.Ingress,
+ UseExternalAddress: true,
+ DomainName: testDomain,
+ },
+ }
+ })
+
+ JustBeforeEach(func(ctx context.Context) {
+ By("creating the SolrCloud")
+ Expect(k8sClient.Create(ctx, solrCloud)).To(Succeed())
+
+ DeferCleanup(func(ctx context.Context) {
+ cleanupTest(ctx, solrCloud)
+ })
+
+ By("Waiting for the SolrCloud to come up healthy")
+ solrCloud = expectSolrCloudToBeReady(ctx, solrCloud)
+
+ By("creating a first Solr Collection")
+ createAndQueryCollection(ctx, solrCloud, "basic", 1, 1)
+ })
+
+ FContext("Can Remove Ingress and Services when changing
addressability", func() {
+
+ FIt("Can adapt to changing needs", func(ctx context.Context) {
+ By("testing the Solr StatefulSet")
+ statefulSet := expectStatefulSet(ctx, solrCloud,
solrCloud.StatefulSetName())
+ // Pod Annotations test
+
Expect(statefulSet.Spec.Template.Annotations).To(HaveKeyWithValue(util.ServiceTypeAnnotation,
util.PerNodeServiceType), "Since external address is used for advertising, the
perNode service should be specified in the pod annotations.")
+
+ By("testing the Solr Common Service")
+ expectService(ctx, solrCloud,
solrCloud.CommonServiceName(), statefulSet.Spec.Selector.MatchLabels, false)
+
+ By("ensuring the Solr Headless Service does not exist")
+ expectNoService(ctx, solrCloud,
solrCloud.HeadlessServiceName(), "Headless service shouldn't exist, but it
does.")
+
+ By("making sure the individual Solr Node Services exist
and route correctly")
+ nodeNames := solrCloud.GetAllSolrPodNames()
+ Expect(nodeNames).To(HaveLen(1), "SolrCloud has
incorrect number of nodeNames.")
+ for _, nodeName := range nodeNames {
+ expectService(ctx, solrCloud, nodeName,
util.MergeLabelsOrAnnotations(statefulSet.Spec.Selector.MatchLabels,
map[string]string{"statefulset.kubernetes.io/pod-name": nodeName}), false)
+ }
+
+ By("making sure Ingress was created correctly")
+ expectIngress(ctx, solrCloud,
solrCloud.CommonIngressName())
+
+ By("Turning off node external addressability and making
sure the node services are deleted")
+ expectSolrCloudWithChecks(ctx, solrCloud, func(g
Gomega, found *solrv1beta1.SolrCloud) {
+
found.Spec.SolrAddressability.External.UseExternalAddress = false
+
found.Spec.SolrAddressability.External.HideNodes = true
+ g.Expect(k8sClient.Update(ctx,
found)).To(Succeed(), "Couldn't update the solrCloud to not advertise the nodes
externally.")
+ })
+
+ // Since node external addressability is off, but
common external addressability is on, the ingress should exist, but the node
services should not
+ expectIngress(ctx, solrCloud,
solrCloud.CommonIngressName())
+
+ expectService(ctx, solrCloud,
solrCloud.HeadlessServiceName(), statefulSet.Spec.Selector.MatchLabels, true)
+
+ eventuallyExpectNoServices(ctx, solrCloud, "Node
services shouldn't exist after the update, but they do.", nodeNames)
+ pod := expectPodNow(ctx, solrCloud,
solrCloud.GetSolrPodName(0))
+
Expect(pod.Annotations).To(HaveKeyWithValue(util.ServiceTypeAnnotation,
util.HeadlessServiceType))
+
+ By("Turning off common external addressability and
making sure the ingress is deleted")
+ expectSolrCloudWithChecks(ctx, solrCloud, func(g
Gomega, found *solrv1beta1.SolrCloud) {
+ found.Spec.SolrAddressability.External = nil
+ g.Expect(k8sClient.Update(ctx,
found)).To(Succeed(), "Couldn't update the solrCloud to remove external
addressability")
+ })
+ eventuallyExpectNoIngress(ctx, solrCloud,
solrCloud.CommonIngressName())
+
+ By("Turning back on common external addressability and
making sure the headless service is deleted")
+ expectSolrCloudWithChecks(ctx, solrCloud, func(g
Gomega, found *solrv1beta1.SolrCloud) {
+ found.Spec.SolrAddressability =
solrv1beta1.SolrAddressabilityOptions{
+ External:
&solrv1beta1.ExternalAddressability{
+ Method:
solrv1beta1.Ingress,
+ UseExternalAddress: true,
+ DomainName: testDomain,
+ },
+ }
+ g.Expect(k8sClient.Update(ctx,
found)).To(Succeed(), "Couldn't update the solrCloud to add external
addressability")
+ })
+ expectIngress(ctx, solrCloud,
solrCloud.CommonIngressName())
+
+ By("testing the Solr Common Service")
+ expectService(ctx, solrCloud,
solrCloud.CommonServiceName(), statefulSet.Spec.Selector.MatchLabels, false)
+
+ By("making sure the individual Solr Node Services are
created")
+
+ Expect(nodeNames).To(HaveLen(1), "SolrCloud has
incorrect number of nodeNames.")
+ for _, nodeName := range nodeNames {
+ expectService(ctx, solrCloud, nodeName,
util.MergeLabelsOrAnnotations(statefulSet.Spec.Selector.MatchLabels,
map[string]string{"statefulset.kubernetes.io/pod-name": nodeName}), false)
+ }
+
+ By("ensuring the Solr Headless Service is deleted after
the pod is updated")
+ expectPodWithChecks(ctx, solrCloud,
solrCloud.GetSolrPodName(0), func(g Gomega, pod *corev1.Pod) {
+
g.Expect(pod.Annotations).To(HaveKeyWithValue(util.ServiceTypeAnnotation,
util.PerNodeServiceType))
+ })
+ expectNoService(ctx, solrCloud,
solrCloud.HeadlessServiceName(), "Headless service shouldn't exist, but it
does.")
+
+ })
+ })
+})
