Two logic issues: service cleanup condition and balance replica condition

[oyjhl]

I noticed two small logic issues.

1. cleanupUnconfiguredServices() condition looks inverted

Relevant code:

solr-operator/controllers/solrcloud_controller.go

Lines 683 to 716 in ca9d3c5

	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 {

if pod.Annotations == nil && pod.Annotations[util.ServiceTypeAnnotation] != "" {

This condition looks inconsistent. If pod.Annotations == nil, then reading pod.Annotations[...] does not make sense. If pod.Annotations != nil, the left side is false, so the code falls into the else branch, but the else branch comment says it is for pods missing the annotation.

2. BalanceReplicasForCluster() comment and code do not match

Relevant code:

solr-operator/controllers/util/solr_scale_util.go

Lines 35 to 41 in ca9d3c5

	func BalanceReplicasForCluster(ctx context.Context, solrCloud *solr.SolrCloud, statefulSet *appsv1.StatefulSet, balanceReason string, balanceCmdUniqueId string, logger logr.Logger) (balanceComplete bool, requestInProgress bool, retryLaterDuration time.Duration, err error) {
	logger = logger.WithValues("balanceReason", balanceReason)
	// If the Cloud has 1 or zero pods, there is no reason to balance replicas.
	if statefulSet.Spec.Replicas == nil || *statefulSet.Spec.Replicas < 1 {
	balanceComplete = true
	} else {
	requestId := "balance-replicas-" + balanceCmdUniqueId

// If the Cloud has 1 or zero pods, there is no reason to balance replicas.
if statefulSet.Spec.Replicas == nil || *statefulSet.Spec.Replicas < 1 {
    balanceComplete = true
}

The comment says “1 or zero pods”, but the condition only matches < 1, not <= 1.

[HoustonPutman]

Yeah both of these look good, feel free to make a PR to fix them!