Skip to content

Commit

Permalink
[KO-295] Changing MultiPodPerHost from bool to bool pointer
Browse files Browse the repository at this point in the history
  • Loading branch information
tanmayja committed Feb 28, 2024
1 parent 2dae26c commit 2884e45
Show file tree
Hide file tree
Showing 18 changed files with 61 additions and 41 deletions.
4 changes: 2 additions & 2 deletions api/v1/aerospikecluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ type AerospikePodSpec struct { //nolint:govet // for readability
// The container port will be exposed to the external network at <hostIP>:<hostPort>,
// 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.
MultiPodPerHost bool `json:"multiPodPerHost,omitempty"`
MultiPodPerHost *bool `json:"multiPodPerHost,omitempty"`

// HostNetwork enables host networking for the pod.
// To enable hostNetwork multiPodPerHost must be false.
Expand Down Expand Up @@ -594,7 +594,7 @@ type AerospikeClusterStatusSpec struct { //nolint:govet // for readability
// 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.
// Deprecated: MultiPodPerHost is now part of podSpec
MultiPodPerHost bool `json:"multiPodPerHost,omitempty"`
MultiPodPerHost *bool `json:"multiPodPerHost,omitempty"`
// Storage specify persistent storage to use for the Aerospike pods.
Storage AerospikeStorageSpec `json:"storage,omitempty"`
// AerospikeAccessControl has the Aerospike roles and users definitions.
Expand Down
9 changes: 5 additions & 4 deletions api/v1/aerospikecluster_validating_webhook.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2021.
Copyright 2024.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -27,18 +27,19 @@ import (
"regexp"
"strings"

lib "github.com/aerospike/aerospike-management-lib"
validate "github.com/asaskevich/govalidator"
"github.com/go-logr/logr"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/utils/ptr"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

internalerrors "github.com/aerospike/aerospike-kubernetes-operator/errors"
lib "github.com/aerospike/aerospike-management-lib"
"github.com/aerospike/aerospike-management-lib/asconfig"
"github.com/aerospike/aerospike-management-lib/deployment"
)
Expand Down Expand Up @@ -104,7 +105,7 @@ func (c *AerospikeCluster) ValidateUpdate(oldObj runtime.Object) (admission.Warn
}

// MultiPodPerHost cannot be updated
if c.Spec.PodSpec.MultiPodPerHost != old.Spec.PodSpec.MultiPodPerHost {
if !ptr.Equal(c.Spec.PodSpec.MultiPodPerHost, old.Spec.PodSpec.MultiPodPerHost) {
return nil, fmt.Errorf("cannot update MultiPodPerHost setting")
}

Expand Down Expand Up @@ -1993,7 +1994,7 @@ func isPathParentOrSame(dir1, dir2 string) bool {
}

func (c *AerospikeCluster) validatePodSpec() error {
if c.Spec.PodSpec.HostNetwork && c.Spec.PodSpec.MultiPodPerHost {
if c.Spec.PodSpec.HostNetwork && ptr.Deref(c.Spec.PodSpec.MultiPodPerHost, false) {
return fmt.Errorf("host networking cannot be enabled with multi pod per host")
}

Expand Down
5 changes: 5 additions & 0 deletions api/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion controllers/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ 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"
Expand Down Expand Up @@ -256,7 +257,7 @@ func (r *SingleClusterReconciler) getBaseConfData(rack *asdbv1.Rack) (map[string

initTemplateInput := initializeTemplateInput{
WorkDir: workDir,
MultiPodPerHost: r.aeroCluster.Spec.PodSpec.MultiPodPerHost,
MultiPodPerHost: ptr.Deref(r.aeroCluster.Spec.PodSpec.MultiPodPerHost, false),
NetworkPolicy: r.aeroCluster.Spec.AerospikeNetworkPolicy,
PodPort: servicePortParam,
PodTLSPort: serviceTLSPortParam,
Expand Down
3 changes: 2 additions & 1 deletion controllers/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ 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"
Expand Down Expand Up @@ -550,7 +551,7 @@ func (r *SingleClusterReconciler) cleanupPods(
}

// Try to delete corresponding pod service if it was created
if r.aeroCluster.Spec.PodSpec.MultiPodPerHost {
if ptr.Deref(r.aeroCluster.Spec.PodSpec.MultiPodPerHost, false) {
// Remove service for pod
// TODO: make it more robust, what if it fails
if err := r.deletePodService(
Expand Down
3 changes: 1 addition & 2 deletions controllers/rack.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,8 +559,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 r.aeroCluster.Spec.PodSpec.MultiPodPerHost &&
!podServiceNeeded(r.aeroCluster.Spec.PodSpec.MultiPodPerHost, &r.aeroCluster.Spec.AerospikeNetworkPolicy) {
if !podServiceNeeded(r.aeroCluster.Spec.PodSpec.MultiPodPerHost, &r.aeroCluster.Spec.AerospikeNetworkPolicy) {
if err := r.cleanupDanglingPodServices(rackState); err != nil {
return reconcileError(err)
}
Expand Down
5 changes: 3 additions & 2 deletions controllers/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ 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"
Expand Down Expand Up @@ -400,8 +401,8 @@ func (r *SingleClusterReconciler) cleanupDanglingPodServices(rackState *RackStat
return nil
}

func podServiceNeeded(multiPodPerHost bool, networkPolicy *asdbv1.AerospikeNetworkPolicy) bool {
if !multiPodPerHost || networkPolicy == nil {
func podServiceNeeded(multiPodPerHost *bool, networkPolicy *asdbv1.AerospikeNetworkPolicy) bool {
if !ptr.Deref(multiPodPerHost, false) || networkPolicy == nil {
return false
}

Expand Down
7 changes: 4 additions & 3 deletions controllers/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

Expand Down Expand Up @@ -801,7 +802,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 !r.aeroCluster.Spec.PodSpec.MultiPodPerHost {
if !ptr.Deref(r.aeroCluster.Spec.PodSpec.MultiPodPerHost, false) {
if affinity.PodAntiAffinity == nil {
affinity.PodAntiAffinity = &corev1.PodAntiAffinity{}
}
Expand Down Expand Up @@ -1460,7 +1461,7 @@ func addVolumeDeviceInContainer(
}

func getSTSContainerPort(
multiPodPerHost bool, aeroConf *asdbv1.AerospikeConfigSpec,
multiPodPerHost *bool, aeroConf *asdbv1.AerospikeConfigSpec,
) []corev1.ContainerPort {
ports := make([]corev1.ContainerPort, 0, len(defaultContainerPorts))

Expand All @@ -1482,7 +1483,7 @@ func getSTSContainerPort(
// The container port will be exposed to the external network at <hostIP>:<hostPort>,
// 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 (!multiPodPerHost) && portInfo.exposedOnHost {
if !ptr.Deref(multiPodPerHost, false) && portInfo.exposedOnHost {
containerPort.HostPort = containerPort.ContainerPort
}

Expand Down
3 changes: 2 additions & 1 deletion test/access_control_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"

as "github.com/aerospike/aerospike-client-go/v6"
asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1"
Expand Down Expand Up @@ -2133,7 +2134,7 @@ func getAerospikeClusterSpecWithAccessControl(
},
},
PodSpec: asdbv1.AerospikePodSpec{
MultiPodPerHost: true,
MultiPodPerHost: ptr.To(true),
},
AerospikeConfig: &asdbv1.AerospikeConfigSpec{
Value: aerospikeConfSpec.getSpec(),
Expand Down
3 changes: 2 additions & 1 deletion test/aero_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ 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"
Expand Down Expand Up @@ -61,7 +62,7 @@ func newAsConn(
tlsName := getServiceTLSName(aeroCluster)

networkType := asdbv1.AerospikeNetworkType(*defaultNetworkType)
if aeroCluster.Spec.PodSpec.MultiPodPerHost && networkType != asdbv1.AerospikeNetworkTypePod &&
if ptr.Deref(aeroCluster.Spec.PodSpec.MultiPodPerHost, false) && networkType != asdbv1.AerospikeNetworkTypePod &&
networkType != asdbv1.AerospikeNetworkTypeCustomInterface {
svc, err := getServiceForPod(pod, k8sClient)
if err != nil {
Expand Down
19 changes: 10 additions & 9 deletions test/cluster_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"

asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1"
Expand Down Expand Up @@ -778,7 +779,7 @@ func createAerospikeClusterPost460(
},

PodSpec: asdbv1.AerospikePodSpec{
MultiPodPerHost: true,
MultiPodPerHost: ptr.To(true),
},
OperatorClientCertSpec: &asdbv1.AerospikeOperatorClientCertSpec{
AerospikeOperatorCertSource: asdbv1.AerospikeOperatorCertSource{
Expand Down Expand Up @@ -841,7 +842,7 @@ func createAerospikeClusterPost560(
},

PodSpec: asdbv1.AerospikePodSpec{
MultiPodPerHost: true,
MultiPodPerHost: ptr.To(true),
},
OperatorClientCertSpec: &asdbv1.AerospikeOperatorClientCertSpec{
AerospikeOperatorCertSource: asdbv1.AerospikeOperatorCertSource{
Expand Down Expand Up @@ -965,7 +966,7 @@ func createDummyAerospikeClusterWithRFAndStorage(
},

PodSpec: asdbv1.AerospikePodSpec{
MultiPodPerHost: true,
MultiPodPerHost: ptr.To(true),
AerospikeInitContainerSpec: &asdbv1.AerospikeInitContainerSpec{},
},

Expand Down Expand Up @@ -1033,7 +1034,7 @@ func createDummyAerospikeCluster(
},

PodSpec: asdbv1.AerospikePodSpec{
MultiPodPerHost: true,
MultiPodPerHost: ptr.To(true),
AerospikeInitContainerSpec: &asdbv1.AerospikeInitContainerSpec{},
},

Expand Down Expand Up @@ -1224,7 +1225,7 @@ func createBasicTLSCluster(
},

PodSpec: asdbv1.AerospikePodSpec{
MultiPodPerHost: true,
MultiPodPerHost: ptr.To(true),
},

OperatorClientCertSpec: &asdbv1.AerospikeOperatorClientCertSpec{
Expand Down Expand Up @@ -1258,7 +1259,7 @@ func createSSDStorageCluster(
multiPodPerHost bool,
) *asdbv1.AerospikeCluster {
aeroCluster := createBasicTLSCluster(clusterNamespacedName, size)
aeroCluster.Spec.PodSpec.MultiPodPerHost = multiPodPerHost
aeroCluster.Spec.PodSpec.MultiPodPerHost = &multiPodPerHost
aeroCluster.Spec.Storage.Volumes = append(
aeroCluster.Spec.Storage.Volumes, []asdbv1.VolumeSpec{
{
Expand Down Expand Up @@ -1289,7 +1290,7 @@ func createHDDAndDataInMemStorageCluster(
multiPodPerHost bool,
) *asdbv1.AerospikeCluster {
aeroCluster := createBasicTLSCluster(clusterNamespacedName, size)
aeroCluster.Spec.PodSpec.MultiPodPerHost = multiPodPerHost
aeroCluster.Spec.PodSpec.MultiPodPerHost = &multiPodPerHost
aeroCluster.Spec.Storage.Volumes = append(
aeroCluster.Spec.Storage.Volumes, []asdbv1.VolumeSpec{
{
Expand Down Expand Up @@ -1328,7 +1329,7 @@ func createDataInMemWithoutPersistentStorageCluster(
multiPodPerHost bool,
) *asdbv1.AerospikeCluster {
aeroCluster := createBasicTLSCluster(clusterNamespacedName, size)
aeroCluster.Spec.PodSpec.MultiPodPerHost = multiPodPerHost
aeroCluster.Spec.PodSpec.MultiPodPerHost = &multiPodPerHost
aeroCluster.Spec.AerospikeConfig.Value["namespaces"] = []interface{}{
map[string]interface{}{
"name": "test",
Expand Down Expand Up @@ -1528,7 +1529,7 @@ func getNonRootPodSpec() asdbv1.AerospikePodSpec {

return asdbv1.AerospikePodSpec{
HostNetwork: false,
MultiPodPerHost: true,
MultiPodPerHost: ptr.To(true),
SecurityContext: &corev1.PodSecurityContext{
RunAsUser: &id,
RunAsGroup: &id,
Expand Down
12 changes: 7 additions & 5 deletions test/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/utils/ptr"

asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1"
"github.com/aerospike/aerospike-kubernetes-operator/pkg/utils"
Expand Down Expand Up @@ -175,7 +176,7 @@ func clusterWithMaxIgnorablePod(ctx goctx.Context) {
By("Set MaxIgnorablePod and Rolling restart cluster")
aeroCluster, err = getCluster(k8sClient, ctx, clusterNamespacedName)
Expect(err).ToNot(HaveOccurred())
val := intstr.FromInt(1)
val := intstr.FromInt32(1)
aeroCluster.Spec.RackConfig.MaxIgnorablePods = &val
aeroCluster.Spec.AerospikeConfig.Value["service"].(map[string]interface{})["proto-fd-max"] = int64(18000)
err = updateCluster(k8sClient, ctx, aeroCluster)
Expand Down Expand Up @@ -250,7 +251,7 @@ func clusterWithMaxIgnorablePod(ctx goctx.Context) {
By("Delete rack with id 2")
aeroCluster, err = getCluster(k8sClient, ctx, clusterNamespacedName)
Expect(err).ToNot(HaveOccurred())
val := intstr.FromInt(1)
val := intstr.FromInt32(1)
aeroCluster.Spec.RackConfig.MaxIgnorablePods = &val
aeroCluster.Spec.RackConfig.Racks = getDummyRackConf(1)
err = updateCluster(k8sClient, ctx, aeroCluster)
Expand Down Expand Up @@ -290,7 +291,7 @@ func clusterWithMaxIgnorablePod(ctx goctx.Context) {
By("Set MaxIgnorablePod and Rolling restart by removing namespace")
aeroCluster, err = getCluster(k8sClient, ctx, clusterNamespacedName)
Expect(err).ToNot(HaveOccurred())
val := intstr.FromInt(1)
val := intstr.FromInt32(1)
aeroCluster.Spec.RackConfig.MaxIgnorablePods = &val
nsList := aeroCluster.Spec.AerospikeConfig.Value["namespaces"].([]interface{})
nsList = nsList[:len(nsList)-1]
Expand Down Expand Up @@ -344,7 +345,7 @@ func deployClusterForMaxIgnorablePods(ctx goctx.Context, clusterNamespacedName t
racks := getDummyRackConf(1, 2)
aeroCluster.Spec.RackConfig = asdbv1.RackConfig{
Namespaces: []string{scNamespace}, Racks: racks}
aeroCluster.Spec.PodSpec.MultiPodPerHost = false
aeroCluster.Spec.PodSpec.MultiPodPerHost = ptr.To(false)
err := deployCluster(k8sClient, ctx, aeroCluster)
Expect(err).ToNot(HaveOccurred())
}
Expand Down Expand Up @@ -979,7 +980,8 @@ func UpdateClusterTest(ctx goctx.Context) {
)
Expect(err).ToNot(HaveOccurred())

aeroCluster.Spec.PodSpec.MultiPodPerHost = !aeroCluster.Spec.PodSpec.MultiPodPerHost
multiPodPerHost := !*aeroCluster.Spec.PodSpec.MultiPodPerHost
aeroCluster.Spec.PodSpec.MultiPodPerHost = &multiPodPerHost

err = k8sClient.Update(ctx, aeroCluster)
Expect(err).Should(HaveOccurred())
Expand Down
5 changes: 3 additions & 2 deletions test/host_network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
"k8s.io/utils/ptr"

asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1"
)
Expand All @@ -26,7 +27,7 @@ var _ = Describe(
clusterNamespacedName, 2, latestImage,
)
aeroCluster.Spec.PodSpec.HostNetwork = true
aeroCluster.Spec.PodSpec.MultiPodPerHost = true
aeroCluster.Spec.PodSpec.MultiPodPerHost = ptr.To(true)

It(
"Should not work with MultiPodPerHost enabled", func() {
Expand All @@ -38,7 +39,7 @@ var _ = Describe(
It(
"Should verify hostNetwork flag updates", func() {
By("Deploying cluster, Should not advertise node address when off")
aeroCluster.Spec.PodSpec.MultiPodPerHost = false
aeroCluster.Spec.PodSpec.MultiPodPerHost = ptr.To(false)
aeroCluster.Spec.PodSpec.HostNetwork = false

err := deployCluster(k8sClient, ctx, aeroCluster)
Expand Down
Loading

0 comments on commit 2884e45

Please sign in to comment.