From 5a2e8ab8e40be6bf70bac1d1524947995944aa9e Mon Sep 17 00:00:00 2001 From: Jwalant Modi Date: Tue, 1 Oct 2024 13:33:00 +0530 Subject: [PATCH 1/2] Skip setting hostPort in Aerospike container for podOnly network and multiPodPerHost: false --- internal/controller/cluster/statefulset.go | 18 ++++++++++++----- test/cluster/network_policy_test.go | 23 ++++++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/internal/controller/cluster/statefulset.go b/internal/controller/cluster/statefulset.go index f137d828..fc0145b9 100644 --- a/internal/controller/cluster/statefulset.go +++ b/internal/controller/cluster/statefulset.go @@ -89,9 +89,10 @@ func (r *SingleClusterReconciler) createSTS( r.Log.Info("Create statefulset for AerospikeCluster", "size", replicas) - ports := getSTSContainerPort( + ports := GetSTSContainerPort( r.aeroCluster.Spec.PodSpec.MultiPodPerHost, r.aeroCluster.Spec.AerospikeConfig, + &r.aeroCluster.Spec.AerospikeNetworkPolicy, ) operatorDefinedLabels := utils.LabelsForAerospikeClusterRack( @@ -605,9 +606,10 @@ func (r *SingleClusterReconciler) updateSTSStorage( func (r *SingleClusterReconciler) updateSTSPorts( st *appsv1.StatefulSet, ) { - ports := getSTSContainerPort( + ports := GetSTSContainerPort( r.aeroCluster.Spec.PodSpec.MultiPodPerHost, r.aeroCluster.Spec.AerospikeConfig, + &r.aeroCluster.Spec.AerospikeNetworkPolicy, ) st.Spec.Template.Spec.Containers[0].Ports = ports @@ -1538,11 +1540,16 @@ func addVolumeDeviceInContainer( } } -func getSTSContainerPort( - multiPodPerHost *bool, aeroConf *asdbv1.AerospikeConfigSpec, +func GetSTSContainerPort( + multiPodPerHost *bool, aeroConf *asdbv1.AerospikeConfigSpec, aeroNetworkPolicy *asdbv1.AerospikeNetworkPolicy, ) []corev1.ContainerPort { ports := make([]corev1.ContainerPort, 0, len(defaultContainerPorts)) portNames := make([]string, 0, len(defaultContainerPorts)) + aerospikeNetworkTypePod := asdbv1.AerospikeNetworkTypePod + podOnlyNetwork := (aeroNetworkPolicy.AccessType == aerospikeNetworkTypePod && + aeroNetworkPolicy.AlternateAccessType == aerospikeNetworkTypePod) + tlsPodOnlyNetwork := (aeroNetworkPolicy.TLSAccessType == aerospikeNetworkTypePod && + aeroNetworkPolicy.TLSAlternateAccessType == aerospikeNetworkTypePod) // Sorting defaultContainerPorts to fetch map in ordered manner. // Helps reduce unnecessary sts object updates. @@ -1567,11 +1574,12 @@ func getSTSContainerPort( ContainerPort: int32(*configPort), } // Single pod per host. Enable hostPort setting + // when pod only network is not defined. // The hostPort setting applies to the Kubernetes containers. // 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 !asdbv1.GetBool(multiPodPerHost) && portInfo.exposedOnHost { + if !asdbv1.GetBool(multiPodPerHost) && portInfo.exposedOnHost && !podOnlyNetwork && !tlsPodOnlyNetwork { containerPort.HostPort = containerPort.ContainerPort } diff --git a/test/cluster/network_policy_test.go b/test/cluster/network_policy_test.go index 634772d8..d1329d39 100644 --- a/test/cluster/network_policy_test.go +++ b/test/cluster/network_policy_test.go @@ -708,6 +708,29 @@ func doTestNetworkPolicy( }, ) + It("OnlyPodNetwork: should not set the hostport in pod only network"+ + "and multiPodPerHost is false", func() { + clusterNamespacedName := getNamespacedName( + "pod-network-cluster", test.MultiClusterNs1) + + networkPolicy := asdbv1.AerospikeNetworkPolicy{ + AccessType: asdbv1.AerospikeNetworkTypePod, + AlternateAccessType: asdbv1.AerospikeNetworkTypePod, + TLSAccessType: asdbv1.AerospikeNetworkTypePod, + TLSAlternateAccessType: asdbv1.AerospikeNetworkTypePod, + } + + aeroCluster = getAerospikeClusterSpecWithNetworkPolicy( + clusterNamespacedName, &networkPolicy, multiPodPerHost, + enableTLS, + ) + ports := aerospikecluster.GetSTSContainerPort(aeroCluster.Spec.PodSpec.MultiPodPerHost, + aeroCluster.Spec.AerospikeConfig, &aeroCluster.Spec.AerospikeNetworkPolicy) + + for _, port := range ports { + Expect(port.HostPort).To(BeZero()) + } + }) // test-case valid only for multiPodPerHost true if multiPodPerHost { It("OnlyPodNetwork: should create cluster without nodePort service", func() { From 58d34416cff1969a42f50e138770e0ba76adc5ac Mon Sep 17 00:00:00 2001 From: Abhisek Dwivedi Date: Tue, 8 Oct 2024 00:14:01 +0530 Subject: [PATCH 2/2] Fix tls and non-tls network policy hostPort flow --- internal/controller/cluster/reconciler.go | 4 +++- internal/controller/cluster/statefulset.go | 27 ++++++++++++++-------- test/cluster/network_policy_test.go | 24 +++++++++++++------ 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/internal/controller/cluster/reconciler.go b/internal/controller/cluster/reconciler.go index 435fd9c4..4e58d06e 100644 --- a/internal/controller/cluster/reconciler.go +++ b/internal/controller/cluster/reconciler.go @@ -235,7 +235,7 @@ func (r *SingleClusterReconciler) Reconcile() (result ctrl.Result, recErr error) if r.IsReclusterNeeded() { if err = deployment.InfoRecluster( r.Log, - r.getClientPolicy(), allHostConns, + policy, allHostConns, ); err != nil { r.Log.Error(err, "Failed to do recluster") return reconcile.Result{}, err @@ -1041,6 +1041,8 @@ func (r *SingleClusterReconciler) IsReclusterNeeded() bool { return false } + // Check for any active-rack addition/update across all the namespaces. + // If there is any active-rack change, recluster is required. for specIdx := range r.aeroCluster.Spec.RackConfig.Racks { for statusIdx := range r.aeroCluster.Status.RackConfig.Racks { if r.aeroCluster.Spec.RackConfig.Racks[specIdx].ID == r.aeroCluster.Status.RackConfig.Racks[statusIdx].ID && diff --git a/internal/controller/cluster/statefulset.go b/internal/controller/cluster/statefulset.go index fc0145b9..caac9087 100644 --- a/internal/controller/cluster/statefulset.go +++ b/internal/controller/cluster/statefulset.go @@ -89,7 +89,7 @@ func (r *SingleClusterReconciler) createSTS( r.Log.Info("Create statefulset for AerospikeCluster", "size", replicas) - ports := GetSTSContainerPort( + ports := getSTSContainerPort( r.aeroCluster.Spec.PodSpec.MultiPodPerHost, r.aeroCluster.Spec.AerospikeConfig, &r.aeroCluster.Spec.AerospikeNetworkPolicy, @@ -606,7 +606,7 @@ func (r *SingleClusterReconciler) updateSTSStorage( func (r *SingleClusterReconciler) updateSTSPorts( st *appsv1.StatefulSet, ) { - ports := GetSTSContainerPort( + ports := getSTSContainerPort( r.aeroCluster.Spec.PodSpec.MultiPodPerHost, r.aeroCluster.Spec.AerospikeConfig, &r.aeroCluster.Spec.AerospikeNetworkPolicy, @@ -1540,18 +1540,25 @@ func addVolumeDeviceInContainer( } } -func GetSTSContainerPort( +func getSTSContainerPort( multiPodPerHost *bool, aeroConf *asdbv1.AerospikeConfigSpec, aeroNetworkPolicy *asdbv1.AerospikeNetworkPolicy, ) []corev1.ContainerPort { ports := make([]corev1.ContainerPort, 0, len(defaultContainerPorts)) portNames := make([]string, 0, len(defaultContainerPorts)) - aerospikeNetworkTypePod := asdbv1.AerospikeNetworkTypePod - podOnlyNetwork := (aeroNetworkPolicy.AccessType == aerospikeNetworkTypePod && - aeroNetworkPolicy.AlternateAccessType == aerospikeNetworkTypePod) - tlsPodOnlyNetwork := (aeroNetworkPolicy.TLSAccessType == aerospikeNetworkTypePod && - aeroNetworkPolicy.TLSAlternateAccessType == aerospikeNetworkTypePod) + podOnlyNetwork := true - // Sorting defaultContainerPorts to fetch map in ordered manner. + // Check for podOnlyNetwork for all TLS and nonTLS fields. + if svcPort := asdbv1.GetServicePort(aeroConf); svcPort != nil { + podOnlyNetwork = aeroNetworkPolicy.AccessType == asdbv1.AerospikeNetworkTypePod && + aeroNetworkPolicy.AlternateAccessType == asdbv1.AerospikeNetworkTypePod + } + + if _, tlsSvcPort := asdbv1.GetServiceTLSNameAndPort(aeroConf); tlsSvcPort != nil { + podOnlyNetwork = podOnlyNetwork && aeroNetworkPolicy.TLSAccessType == asdbv1.AerospikeNetworkTypePod && + aeroNetworkPolicy.TLSAlternateAccessType == asdbv1.AerospikeNetworkTypePod + } + + // Sorting defaultContainerPorts to fetch map in an ordered manner. // Helps reduce unnecessary sts object updates. for portName := range defaultContainerPorts { portNames = append(portNames, portName) @@ -1579,7 +1586,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 !asdbv1.GetBool(multiPodPerHost) && portInfo.exposedOnHost && !podOnlyNetwork && !tlsPodOnlyNetwork { + if !asdbv1.GetBool(multiPodPerHost) && portInfo.exposedOnHost && !podOnlyNetwork { containerPort.HostPort = containerPort.ContainerPort } diff --git a/test/cluster/network_policy_test.go b/test/cluster/network_policy_test.go index d1329d39..c47f7c34 100644 --- a/test/cluster/network_policy_test.go +++ b/test/cluster/network_policy_test.go @@ -708,8 +708,7 @@ func doTestNetworkPolicy( }, ) - It("OnlyPodNetwork: should not set the hostport in pod only network"+ - "and multiPodPerHost is false", func() { + It("PodOnlyNetwork: Should not set the hostPort", func() { clusterNamespacedName := getNamespacedName( "pod-network-cluster", test.MultiClusterNs1) @@ -724,16 +723,27 @@ func doTestNetworkPolicy( clusterNamespacedName, &networkPolicy, multiPodPerHost, enableTLS, ) - ports := aerospikecluster.GetSTSContainerPort(aeroCluster.Spec.PodSpec.MultiPodPerHost, - aeroCluster.Spec.AerospikeConfig, &aeroCluster.Spec.AerospikeNetworkPolicy) - for _, port := range ports { - Expect(port.HostPort).To(BeZero()) + err := aerospikeClusterCreateUpdate(k8sClient, aeroCluster, ctx) + Expect(err).ToNot(HaveOccurred()) + + podList, err := getPodList(aeroCluster, k8sClient) + Expect(err).ToNot(HaveOccurred()) + Expect(len(podList.Items)).ToNot(BeZero()) + + for idx := range podList.Items { + pod := &podList.Items[idx] + container := pod.Spec.Containers[0] + Expect(container.Ports).ToNot(BeNil()) + + for _, port := range container.Ports { + Expect(port.HostPort).To(BeZero()) + } } }) // test-case valid only for multiPodPerHost true if multiPodPerHost { - It("OnlyPodNetwork: should create cluster without nodePort service", func() { + It("PodOnlyNetwork: should create cluster without nodePort service", func() { clusterNamespacedName := getNamespacedName( "pod-network-cluster", test.MultiClusterNs1)