From af3cb8a3e2d64fdbdb889de644cac2428e09a11b Mon Sep 17 00:00:00 2001 From: Xun Jiang Date: Fri, 20 Oct 2023 13:25:55 +0800 Subject: [PATCH] Add HealthCheckNodePort deletion logic. Signed-off-by: Xun Jiang --- pkg/restore/service_action.go | 65 ++++++++++++ pkg/restore/service_action_test.go | 161 ++++++++++++++++++++++++++++- 2 files changed, 225 insertions(+), 1 deletion(-) diff --git a/pkg/restore/service_action.go b/pkg/restore/service_action.go index 2dbac3a894..50b720a9cf 100644 --- 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.NestedInt64(*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.NestedInt64(*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 index d80cc1c450..7009236b05 100644 --- 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.ServiceExternalTrafficPolicyTypeCluster, + 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.ServiceExternalTrafficPolicyTypeCluster, + 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.ServiceExternalTrafficPolicyTypeCluster, + 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.ServiceExternalTrafficPolicyTypeCluster, + Type: corev1api.ServiceTypeLoadBalancer, + }, + }, + }, } for _, test := range tests {