From e9a6b01d8005414b3e416a8ef465c2193b17d742 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Wed, 13 Mar 2024 19:25:55 +0530 Subject: [PATCH] addressing comments --- api/v1/aerospikecluster_validating_webhook.go | 2 +- api/v1/utils.go | 6 ++++++ controllers/configmap.go | 3 +-- controllers/pod.go | 3 +-- controllers/rack.go | 3 +-- controllers/service.go | 3 +-- controllers/statefulset.go | 5 ++--- test/aero_info.go | 3 +-- test/network_policy_test.go | 2 +- 9 files changed, 15 insertions(+), 15 deletions(-) diff --git a/api/v1/aerospikecluster_validating_webhook.go b/api/v1/aerospikecluster_validating_webhook.go index 3c189524d..fe340aee2 100644 --- a/api/v1/aerospikecluster_validating_webhook.go +++ b/api/v1/aerospikecluster_validating_webhook.go @@ -1999,7 +1999,7 @@ func isPathParentOrSame(dir1, dir2 string) bool { } func (c *AerospikeCluster) validatePodSpec() error { - if c.Spec.PodSpec.HostNetwork && ptr.Deref(c.Spec.PodSpec.MultiPodPerHost, false) { + if c.Spec.PodSpec.HostNetwork && GetBool(c.Spec.PodSpec.MultiPodPerHost) { return fmt.Errorf("host networking cannot be enabled with multi pod per host") } diff --git a/api/v1/utils.go b/api/v1/utils.go index ff11d8281..786d392ef 100644 --- a/api/v1/utils.go +++ b/api/v1/utils.go @@ -8,6 +8,7 @@ import ( "strings" v1 "k8s.io/api/core/v1" + "k8s.io/utils/ptr" internalerrors "github.com/aerospike/aerospike-kubernetes-operator/errors" lib "github.com/aerospike/aerospike-management-lib" @@ -494,3 +495,8 @@ func getContainerNames(containers []v1.Container) []string { return containerNames } + +// GetBool returns the value of the given bool pointer. If the pointer is nil, it returns false. +func GetBool(boolPtr *bool) bool { + return ptr.Deref(boolPtr, false) +} diff --git a/controllers/configmap.go b/controllers/configmap.go index 189515f3f..c4a88f9b3 100644 --- a/controllers/configmap.go +++ b/controllers/configmap.go @@ -17,7 +17,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1" @@ -257,7 +256,7 @@ func (r *SingleClusterReconciler) getBaseConfData(rack *asdbv1.Rack) (map[string initTemplateInput := initializeTemplateInput{ WorkDir: workDir, - MultiPodPerHost: ptr.Deref(r.aeroCluster.Spec.PodSpec.MultiPodPerHost, false), + MultiPodPerHost: asdbv1.GetBool(r.aeroCluster.Spec.PodSpec.MultiPodPerHost), NetworkPolicy: r.aeroCluster.Spec.AerospikeNetworkPolicy, PodPort: servicePortParam, PodTLSPort: serviceTLSPortParam, diff --git a/controllers/pod.go b/controllers/pod.go index 7cff12b39..9509986b9 100644 --- a/controllers/pod.go +++ b/controllers/pod.go @@ -15,7 +15,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1" @@ -601,7 +600,7 @@ func (r *SingleClusterReconciler) cleanupPods( } // Try to delete corresponding pod service if it was created - if ptr.Deref(r.aeroCluster.Spec.PodSpec.MultiPodPerHost, false) { + if asdbv1.GetBool(r.aeroCluster.Spec.PodSpec.MultiPodPerHost) { // Remove service for pod // TODO: make it more robust, what if it fails if err := r.deletePodService( diff --git a/controllers/rack.go b/controllers/rack.go index 60b5a2569..087b56fef 100644 --- a/controllers/rack.go +++ b/controllers/rack.go @@ -13,7 +13,6 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1" @@ -572,7 +571,7 @@ func (r *SingleClusterReconciler) reconcileRack( // Safe check to delete all dangling pod services which are no longer required // There won't be any case of dangling pod service with MultiPodPerHost false, so ignore that case - if ptr.Deref(r.aeroCluster.Spec.PodSpec.MultiPodPerHost, false) && + if asdbv1.GetBool(r.aeroCluster.Spec.PodSpec.MultiPodPerHost) && !podServiceNeeded(r.aeroCluster.Spec.PodSpec.MultiPodPerHost, &r.aeroCluster.Spec.AerospikeNetworkPolicy) { if err := r.cleanupDanglingPodServices(rackState); err != nil { return reconcileError(err) diff --git a/controllers/service.go b/controllers/service.go index e2bd4d5c1..9d12e583e 100644 --- a/controllers/service.go +++ b/controllers/service.go @@ -11,7 +11,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1" @@ -402,7 +401,7 @@ func (r *SingleClusterReconciler) cleanupDanglingPodServices(rackState *RackStat } func podServiceNeeded(multiPodPerHost *bool, networkPolicy *asdbv1.AerospikeNetworkPolicy) bool { - if !ptr.Deref(multiPodPerHost, false) || networkPolicy == nil { + if !asdbv1.GetBool(multiPodPerHost) || networkPolicy == nil { return false } diff --git a/controllers/statefulset.go b/controllers/statefulset.go index 9076ae946..58b958d70 100644 --- a/controllers/statefulset.go +++ b/controllers/statefulset.go @@ -18,7 +18,6 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/util/retry" - "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -857,7 +856,7 @@ func (r *SingleClusterReconciler) updateSTSSchedulingPolicy( // Set our rules in PodAntiAffinity // only enable in production, so it can be used in 1 node clusters while debugging (minikube) - if !ptr.Deref(r.aeroCluster.Spec.PodSpec.MultiPodPerHost, false) { + if !asdbv1.GetBool(r.aeroCluster.Spec.PodSpec.MultiPodPerHost) { if affinity.PodAntiAffinity == nil { affinity.PodAntiAffinity = &corev1.PodAntiAffinity{} } @@ -1572,7 +1571,7 @@ func getSTSContainerPort( // The container port will be exposed to the external network at :, // where the hostIP is the IP address of the Kubernetes node where // the container is running and the hostPort is the port requested by the user - if !ptr.Deref(multiPodPerHost, false) && portInfo.exposedOnHost { + if !asdbv1.GetBool(multiPodPerHost) && portInfo.exposedOnHost { containerPort.HostPort = containerPort.ContainerPort } diff --git a/test/aero_info.go b/test/aero_info.go index fafdbfc92..1251d04d5 100644 --- a/test/aero_info.go +++ b/test/aero_info.go @@ -14,7 +14,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" as "github.com/aerospike/aerospike-client-go/v6" @@ -62,7 +61,7 @@ func newAsConn( tlsName := getServiceTLSName(aeroCluster) networkType := asdbv1.AerospikeNetworkType(*defaultNetworkType) - if ptr.Deref(aeroCluster.Spec.PodSpec.MultiPodPerHost, false) && networkType != asdbv1.AerospikeNetworkTypePod && + if asdbv1.GetBool(aeroCluster.Spec.PodSpec.MultiPodPerHost) && networkType != asdbv1.AerospikeNetworkTypePod && networkType != asdbv1.AerospikeNetworkTypeCustomInterface { svc, err := getServiceForPod(pod, k8sClient) if err != nil { diff --git a/test/network_policy_test.go b/test/network_policy_test.go index efc46a2ea..3a1ba41bc 100644 --- a/test/network_policy_test.go +++ b/test/network_policy_test.go @@ -1207,7 +1207,7 @@ func getExpectedServicePortForPod( var port int32 if (networkType != asdbv1.AerospikeNetworkTypePod && networkType != asdbv1.AerospikeNetworkTypeCustomInterface) && - ptr.Deref(aeroCluster.Spec.PodSpec.MultiPodPerHost, false) { + asdbv1.GetBool(aeroCluster.Spec.PodSpec.MultiPodPerHost) { svc, err := getServiceForPod(pod, k8sClient) if err != nil { return 0, fmt.Errorf("error getting service port: %v", err)