diff --git a/controllers/virtualmachineservice/providers/loadbalancer_provider.go b/controllers/virtualmachineservice/providers/loadbalancer_provider.go index 44f380952..fada62315 100644 --- a/controllers/virtualmachineservice/providers/loadbalancer_provider.go +++ b/controllers/virtualmachineservice/providers/loadbalancer_provider.go @@ -17,8 +17,11 @@ import ( const ( NSXTLoadBalancer = "nsx-t-lb" + // ServiceLoadBalancerHealthCheckNodePortTagKey is the key of annotation which tell NSX to perform HTTP health check on the healthCheckNodePort. This only applies to TKG LoadBalancer type services with externalTrafficPolicy set to Local. ServiceLoadBalancerHealthCheckNodePortTagKey = "ncp/healthCheckNodePort" - NSXTServiceProxy = "nsx-t" + // ServiceLoadBalancerEndpointHealthCheckEnabledTagKey is the key of annotation which tell NSX to enable health check on the Service targetPort. + ServiceLoadBalancerEndpointHealthCheckEnabledTagKey = "nsx.vmware.com/lb-health-check" + NSXTServiceProxy = "nsx-t" // LabelServiceProxyName indicates that an alternative service proxy will implement // this Service. Copied from kubernetes pkg/proxy/apis/well_known_labels.go to avoid // k8s dependency. @@ -142,6 +145,9 @@ func (nl *NsxtLoadbalancerProvider) GetServiceAnnotations(ctx context.Context, v res[ServiceLoadBalancerHealthCheckNodePortTagKey] = healthCheckNodePortString } + if _, ok := vmService.Annotations[utils.AnnotationServiceEndpointHealthCheckEnabledKey]; ok { + res[ServiceLoadBalancerEndpointHealthCheckEnabledTagKey] = "" + } return res, nil } @@ -156,5 +162,10 @@ func (nl *NsxtLoadbalancerProvider) GetToBeRemovedServiceAnnotations(ctx context res[ServiceLoadBalancerHealthCheckNodePortTagKey] = "" } + // When endpointHealthCheckEnable is NOT present, the corresponding NSX-T + // annotation should be cleared as well + if _, ok := vmService.Annotations[utils.AnnotationServiceEndpointHealthCheckEnabledKey]; !ok { + res[ServiceLoadBalancerEndpointHealthCheckEnabledTagKey] = "" + } return res, nil } diff --git a/controllers/virtualmachineservice/providers/loadbalancer_provider_test.go b/controllers/virtualmachineservice/providers/loadbalancer_provider_test.go index d1958f5f5..0a0b17c7c 100644 --- a/controllers/virtualmachineservice/providers/loadbalancer_provider_test.go +++ b/controllers/virtualmachineservice/providers/loadbalancer_provider_test.go @@ -105,6 +105,32 @@ var _ = Describe( }) }) + Context("GetServiceAnnotations when VMService has endpointHealthCheckEnabled defined", func() { + BeforeEach(func() { + vmService = &vmopv1.VirtualMachineService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dummy-vmservice", + Namespace: dummyNamespace, + Annotations: make(map[string]string), + }, + Spec: vmopv1.VirtualMachineServiceSpec{ + Type: vmopv1.VirtualMachineServiceTypeClusterIP, + ClusterIP: "TEST", + ExternalName: "TEST", + }, + } + vmService.Annotations[utils.AnnotationServiceEndpointHealthCheckEnabledKey] = "" + lbProvider = NsxtLoadBalancerProvider() + }) + + It("should get nsx.vmware.com/lb-health-check in the annotation", func() { + vmServiceAnnotations, err := lbProvider.GetServiceAnnotations(ctx, vmService) + Expect(err).ToNot(HaveOccurred()) + Expect(vmServiceAnnotations).ToNot(BeNil()) + Expect(vmServiceAnnotations).To(HaveKey(ServiceLoadBalancerEndpointHealthCheckEnabledTagKey)) + }) + }) + Context("GetToBeRemovedServiceAnnotations when VMService does not have healthCheckNodePort defined", func() { BeforeEach(func() { vmService = &vmopv1.VirtualMachineService{ @@ -130,6 +156,31 @@ var _ = Describe( }) }) + Context("GetToBeRemovedServiceAnnotations when VMService does not have endpointHealthCheckEnabled defined", func() { + BeforeEach(func() { + vmService = &vmopv1.VirtualMachineService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dummy-vmservice", + Namespace: dummyNamespace, + Annotations: make(map[string]string), + }, + Spec: vmopv1.VirtualMachineServiceSpec{ + Type: vmopv1.VirtualMachineServiceTypeClusterIP, + ClusterIP: "TEST", + ExternalName: "TEST", + }, + } + lbProvider = NsxtLoadBalancerProvider() + }) + + It("should get nsx.vmware.com/lb-health-check in the to be removed annotation", func() { + vmServiceAnnotations, err := lbProvider.GetToBeRemovedServiceAnnotations(ctx, vmService) + Expect(err).ToNot(HaveOccurred()) + Expect(vmServiceAnnotations).ToNot(BeNil()) + Expect(vmServiceAnnotations).To(HaveKey(ServiceLoadBalancerEndpointHealthCheckEnabledTagKey)) + }) + }) + Context("GetServiceLabels when VMService have externalTrafficPolicy annotation defined", func() { BeforeEach(func() { vmService = &vmopv1.VirtualMachineService{ diff --git a/controllers/virtualmachineservice/utils/constants.go b/controllers/virtualmachineservice/utils/constants.go index 943d0d2a3..03649825c 100644 --- a/controllers/virtualmachineservice/utils/constants.go +++ b/controllers/virtualmachineservice/utils/constants.go @@ -5,5 +5,8 @@ package utils const ( AnnotationServiceExternalTrafficPolicyKey = "virtualmachineservice.vmoperator.vmware.com/service.externalTrafficPolicy" - AnnotationServiceHealthCheckNodePortKey = "virtualmachineservice.vmoperator.vmware.com/service.healthCheckNodePort" + // AnnotationServiceHealthCheckNodePortKey is the key of the annotation that is used to set HTTP health check on the TKG Service's healthCheckNodePort when the Service is LoadBalancer type and externalTrafficPolicy is Local. + AnnotationServiceHealthCheckNodePortKey = "virtualmachineservice.vmoperator.vmware.com/service.healthCheckNodePort" + // AnnotationServiceEndpointHealthCheckEnabledKey is the key of the annotation that is used to enable health check on the VMService endpoint port. This is different from AnnotationServiceHealthCheckNodePortKey. + AnnotationServiceEndpointHealthCheckEnabledKey = "virtualmachineservice.vmoperator.vmware.com/service.endpointHealthCheckEnabled" ) diff --git a/controllers/virtualmachineservice/virtualmachineservice_controller.go b/controllers/virtualmachineservice/virtualmachineservice_controller.go index 4e3cfe0ed..4f6322775 100644 --- a/controllers/virtualmachineservice/virtualmachineservice_controller.go +++ b/controllers/virtualmachineservice/virtualmachineservice_controller.go @@ -53,8 +53,9 @@ func AddToManager(ctx *pkgctx.ControllerManagerContext, mgr manager.Manager) err ) lbProviderType := pkgcfg.FromContext(ctx).LoadBalancerProvider + networkProvider := pkgcfg.FromContext(ctx).NetworkProviderType if lbProviderType == "" { - if pkgcfg.FromContext(ctx).NetworkProviderType == pkgcfg.NetworkProviderTypeNSXT { + if networkProvider == pkgcfg.NetworkProviderTypeNSXT || networkProvider == pkgcfg.NetworkProviderTypeVPC { lbProviderType = providers.NSXTLoadBalancer } } @@ -367,7 +368,7 @@ func (r *ReconcileVirtualMachineService) setServiceAnnotationsAndLabels( } // Explicitly remove vm service managed annotations if needed - for _, k := range []string{utils.AnnotationServiceExternalTrafficPolicyKey, utils.AnnotationServiceHealthCheckNodePortKey} { + for _, k := range []string{utils.AnnotationServiceExternalTrafficPolicyKey, utils.AnnotationServiceHealthCheckNodePortKey, utils.AnnotationServiceEndpointHealthCheckEnabledKey} { if _, exist := vmService.Annotations[k]; !exist { if v, exist := service.Annotations[k]; exist { ctx.Logger.V(5).Info("Removing annotation from Service", "key", k, "value", v) diff --git a/controllers/virtualmachineservice/virtualmachineservice_controller_unit_test.go b/controllers/virtualmachineservice/virtualmachineservice_controller_unit_test.go index ffe52dc00..8b6fb058a 100644 --- a/controllers/virtualmachineservice/virtualmachineservice_controller_unit_test.go +++ b/controllers/virtualmachineservice/virtualmachineservice_controller_unit_test.go @@ -899,6 +899,7 @@ func nsxtLBProviderTestsReconcile() { vmService.Spec.LoadBalancerSourceRanges = []string{"1.1.1.0/24", "2.2.2.2/28"} vmService.Annotations[utils.AnnotationServiceExternalTrafficPolicyKey] = string(corev1.ServiceExternalTrafficPolicyTypeLocal) vmService.Annotations[utils.AnnotationServiceHealthCheckNodePortKey] = "30012" + vmService.Annotations[utils.AnnotationServiceEndpointHealthCheckEnabledKey] = "" vmService.Labels[LabelServiceProxyName] = providers.NSXTServiceProxy err := reconciler.ReconcileNormal(vmServiceCtx) @@ -912,8 +913,10 @@ func nsxtLBProviderTestsReconcile() { Expect(newService.Spec.ExternalName).To(Equal(newExternalName)) Expect(newService.Spec.LoadBalancerIP).To(Equal(newLoadBalancerIP)) Expect(newService.Spec.ExternalTrafficPolicy).To(Equal(corev1.ServiceExternalTrafficPolicyTypeLocal)) + Expect(newService.Annotations[providers.ServiceLoadBalancerHealthCheckNodePortTagKey]).To(Equal("30012")) Expect(newService.Labels[LabelServiceProxyName]).To(Equal(providers.NSXTServiceProxy)) Expect(newService.Spec.LoadBalancerSourceRanges).To(Equal([]string{"1.1.1.0/24", "2.2.2.2/28"})) + Expect(newService.Annotations).To(HaveKey(providers.ServiceLoadBalancerEndpointHealthCheckEnabledTagKey)) }) It("Should update the k8s Service to match with the VirtualMachineService when LoadBalancerSourceRanges is cleared", func() { @@ -958,9 +961,37 @@ func nsxtLBProviderTestsReconcile() { Expect(newService.Spec.ExternalTrafficPolicy).To(Equal(corev1.ServiceExternalTrafficPolicyTypeCluster)) Expect(newService.Annotations).ToNot(HaveKey(utils.AnnotationServiceExternalTrafficPolicyKey)) Expect(newService.Annotations).ToNot(HaveKey(utils.AnnotationServiceHealthCheckNodePortKey)) + Expect(newService.Annotations).ToNot(HaveKey(providers.ServiceLoadBalancerHealthCheckNodePortTagKey)) Expect(newService.Labels).ToNot(HaveKey(LabelServiceProxyName)) }) + It("Should update the k8s Service to match with the VirtualMachineService when endpointHealthCheckEnabled is cleared", func() { + if service.Annotations == nil { + service.Annotations = make(map[string]string) + } + if service.Labels == nil { + service.Labels = make(map[string]string) + } + + By("applying endpointHealthCheckEnabled annotations") + service.Annotations[utils.AnnotationServiceEndpointHealthCheckEnabledKey] = "" + service.Annotations[providers.ServiceLoadBalancerEndpointHealthCheckEnabledTagKey] = "" + By("applying service-proxy label") + Expect(ctx.Client.Update(ctx, service)).To(Succeed()) + + delete(vmService.Annotations, utils.AnnotationServiceEndpointHealthCheckEnabledKey) + + err := reconciler.ReconcileNormal(vmServiceCtx) + Expect(err).ShouldNot(HaveOccurred()) + + expectEvent(ctx, ContainSubstring(virtualmachineservice.OpUpdate)) + + newService := &corev1.Service{} + Expect(ctx.Client.Get(ctx, objKey, newService)).To(Succeed()) + Expect(newService.Annotations).ToNot(HaveKey(utils.AnnotationServiceEndpointHealthCheckEnabledKey)) + Expect(newService.Annotations).ToNot(HaveKey(providers.ServiceLoadBalancerEndpointHealthCheckEnabledTagKey)) + }) + It("Should update the k8s Service to remove the provider specific annotations regarding healthCheckNodePort", func() { if service.Annotations == nil { service.Annotations = make(map[string]string)