Add HealthCheckNodePort deletion logic in Service restore.

Signed-off-by: Xun Jiang <[email protected]>
vmware-tanzu · Oct 27, 2023 · fab32c3 · fab32c3
1 parent fd8350f
commit fab32c3
Show file tree

Hide file tree

Showing 3 changed files with 226 additions and 1 deletion.
diff --git a/changelogs/unreleased/7026-blackpiglet b/changelogs/unreleased/7026-blackpiglet
@@ -0,0 +1 @@
+Add HealthCheckNodePort deletion logic for Service restore.
diff --git a/pkg/restore/service_action.go b/pkg/restore/service_action.go
@@ -66,6 +66,9 @@ func (a *ServiceAction) Execute(input *velero.RestoreItemActionExecuteInput) (*v
 		if err := deleteNodePorts(service); err != nil {
 			return nil, err
 		}
+		if err := deleteHealthCheckNodePort(service); err != nil {
+			return nil, err
+		}
 	}
 
 	res, err := runtime.DefaultUnstructuredConverter.ToUnstructured(service)
@@ -76,6 +79,68 @@ func (a *ServiceAction) Execute(input *velero.RestoreItemActionExecuteInput) (*v
 	return velero.NewRestoreItemActionExecuteOutput(&unstructured.Unstructured{Object: res}), nil
 }
 
+func deleteHealthCheckNodePort(service *corev1api.Service) error {
+	// Check service type and external traffic policy setting,
+	// if the setting is not applicable for HealthCheckNodePort, return early.
+	if service.Spec.ExternalTrafficPolicy != corev1api.ServiceExternalTrafficPolicyTypeLocal ||
+		service.Spec.Type != corev1api.ServiceTypeLoadBalancer {
+		return nil
+	}
+
+	// HealthCheckNodePort is already 0, return.
+	if service.Spec.HealthCheckNodePort == 0 {
+		return nil
+	}
+
+	// Search HealthCheckNodePort from annotation.
+	lastAppliedConfig, ok := service.Annotations[annotationLastAppliedConfig]
+	if ok {
+		appliedServiceUnstructured := new(map[string]interface{})
+		if err := json.Unmarshal([]byte(lastAppliedConfig), appliedServiceUnstructured); err != nil {
+			return errors.WithStack(err)
+		}
+
+		healthCheckNodePort, exist, err := unstructured.NestedFloat64(*appliedServiceUnstructured, "spec", "healthCheckNodePort")
+		if err != nil {
+			return errors.WithStack(err)
+		}
+
+		// Found healthCheckNodePort in lastAppliedConfig annotation,
+		// and the value is not 0. No need to delete, return.
+		if exist && healthCheckNodePort != 0 {
+			return nil
+		}
+	}
+
+	// Search HealthCheckNodePort from ManagedFields.
+	for _, entry := range service.GetManagedFields() {
+		if entry.FieldsV1 == nil {
+			continue
+		}
+		fields := new(map[string]interface{})
+		if err := json.Unmarshal(entry.FieldsV1.Raw, fields); err != nil {
+			return errors.WithStack(err)
+		}
+
+		_, exist, err := unstructured.NestedMap(*fields, "f:spec", "f:healthCheckNodePort")
+		if err != nil {
+			return errors.WithStack(err)
+		}
+		if !exist {
+			continue
+		}
+		// Found healthCheckNodePort in ManagedFields,
+		// and the value is not 0. No need to delete, return.
+		return nil
+	}
+
+	// Cannot find HealthCheckNodePort from Annotation and
+	// ManagedFields, which means it's auto-generated. Delete it.
+	service.Spec.HealthCheckNodePort = 0
+
+	return nil
+}
+
 func deleteNodePorts(service *corev1api.Service) error {
 	if service.Spec.Type == corev1api.ServiceTypeExternalName {
 		return nil

diff --git a/pkg/restore/service_action_test.go b/pkg/restore/service_action_test.go
@@ -36,7 +36,8 @@ import (
 func svcJSON(ports ...corev1api.ServicePort) string {
 	svc := corev1api.Service{
 		Spec: corev1api.ServiceSpec{
-			Ports: ports,
+			HealthCheckNodePort: 8080,
+			Ports:               ports,
 		},
 	}
 
@@ -486,6 +487,164 @@ func TestServiceActionExecute(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "If PreserveNodePorts is True in restore spec then HealthCheckNodePort always preserved.",
+			obj: corev1api.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "svc-1",
+				},
+				Spec: corev1api.ServiceSpec{
+					HealthCheckNodePort:   8080,
+					ExternalTrafficPolicy: corev1api.ServiceExternalTrafficPolicyTypeLocal,
+					Type:                  corev1api.ServiceTypeLoadBalancer,
+					Ports: []corev1api.ServicePort{
+						{
+							Name:     "http",
+							Port:     80,
+							NodePort: 8080,
+						},
+						{
+							Name:     "hepsiburada",
+							NodePort: 9025,
+						},
+					},
+				},
+			},
+			restore: builder.ForRestore(api.DefaultNamespace, "").PreserveNodePorts(true).Result(),
+			expectedRes: corev1api.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "svc-1",
+				},
+				Spec: corev1api.ServiceSpec{
+					HealthCheckNodePort:   8080,
+					ExternalTrafficPolicy: corev1api.ServiceExternalTrafficPolicyTypeLocal,
+					Type:                  corev1api.ServiceTypeLoadBalancer,
+					Ports: []corev1api.ServicePort{
+						{
+							Name:     "http",
+							Port:     80,
+							NodePort: 8080,
+						},
+						{
+							Name:     "hepsiburada",
+							NodePort: 9025,
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "If PreserveNodePorts is False in restore spec then HealthCheckNodePort should be cleaned.",
+			obj: corev1api.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "svc-1",
+				},
+				Spec: corev1api.ServiceSpec{
+					HealthCheckNodePort:   8080,
+					ExternalTrafficPolicy: corev1api.ServiceExternalTrafficPolicyTypeLocal,
+					Type:                  corev1api.ServiceTypeLoadBalancer,
+				},
+			},
+			restore: builder.ForRestore(api.DefaultNamespace, "").PreserveNodePorts(false).Result(),
+			expectedRes: corev1api.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "svc-1",
+				},
+				Spec: corev1api.ServiceSpec{
+					HealthCheckNodePort:   0,
+					ExternalTrafficPolicy: corev1api.ServiceExternalTrafficPolicyTypeLocal,
+					Type:                  corev1api.ServiceTypeLoadBalancer,
+				},
+			},
+		},
+		{
+			name: "If PreserveNodePorts is false in restore spec, but service is not expected, then HealthCheckNodePort should be kept.",
+			obj: corev1api.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "svc-1",
+				},
+				Spec: corev1api.ServiceSpec{
+					HealthCheckNodePort:   8080,
+					ExternalTrafficPolicy: corev1api.ServiceExternalTrafficPolicyTypeCluster,
+					Type:                  corev1api.ServiceTypeLoadBalancer,
+				},
+			},
+			restore: builder.ForRestore(api.DefaultNamespace, "").PreserveNodePorts(false).Result(),
+			expectedRes: corev1api.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "svc-1",
+				},
+				Spec: corev1api.ServiceSpec{
+					HealthCheckNodePort:   8080,
+					ExternalTrafficPolicy: corev1api.ServiceExternalTrafficPolicyTypeCluster,
+					Type:                  corev1api.ServiceTypeLoadBalancer,
+				},
+			},
+		},
+		{
+			name: "If PreserveNodePorts is false in restore spec, but HealthCheckNodePort can be found in Annotation, then it should be kept.",
+			obj: corev1api.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "svc-1",
+					Annotations: map[string]string{annotationLastAppliedConfig: svcJSON()},
+				},
+				Spec: corev1api.ServiceSpec{
+					HealthCheckNodePort:   8080,
+					ExternalTrafficPolicy: corev1api.ServiceExternalTrafficPolicyTypeLocal,
+					Type:                  corev1api.ServiceTypeLoadBalancer,
+				},
+			},
+			restore: builder.ForRestore(api.DefaultNamespace, "").PreserveNodePorts(false).Result(),
+			expectedRes: corev1api.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "svc-1",
+					Annotations: map[string]string{annotationLastAppliedConfig: svcJSON()},
+				},
+				Spec: corev1api.ServiceSpec{
+					HealthCheckNodePort:   8080,
+					ExternalTrafficPolicy: corev1api.ServiceExternalTrafficPolicyTypeLocal,
+					Type:                  corev1api.ServiceTypeLoadBalancer,
+				},
+			},
+		},
+		{
+			name: "If PreserveNodePorts is false in restore spec, but HealthCheckNodePort can be found in ManagedFields, then it should be kept.",
+			obj: corev1api.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "svc-1",
+					ManagedFields: []metav1.ManagedFieldsEntry{
+						{
+							FieldsV1: &metav1.FieldsV1{
+								Raw: []byte(`{"f:spec":{"f:healthCheckNodePort":{}}}`),
+							},
+						},
+					},
+				},
+				Spec: corev1api.ServiceSpec{
+					HealthCheckNodePort:   8080,
+					ExternalTrafficPolicy: corev1api.ServiceExternalTrafficPolicyTypeLocal,
+					Type:                  corev1api.ServiceTypeLoadBalancer,
+				},
+			},
+			restore: builder.ForRestore(api.DefaultNamespace, "").PreserveNodePorts(false).Result(),
+			expectedRes: corev1api.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "svc-1",
+					ManagedFields: []metav1.ManagedFieldsEntry{
+						{
+							FieldsV1: &metav1.FieldsV1{
+								Raw: []byte(`{"f:spec":{"f:healthCheckNodePort":{}}}`),
+							},
+						},
+					},
+				},
+				Spec: corev1api.ServiceSpec{
+					HealthCheckNodePort:   8080,
+					ExternalTrafficPolicy: corev1api.ServiceExternalTrafficPolicyTypeLocal,
+					Type:                  corev1api.ServiceTypeLoadBalancer,
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {