bugfix envoyproxy#1340: not compare all svc.spec for user modified scene

Signed-off-by: spwangxp <[email protected]>
spwangxp · Apr 23, 2023 · 7ea2c83 · 7ea2c83
1 parent 690a045
commit 7ea2c83
Show file tree

Hide file tree

Showing 3 changed files with 121 additions and 1 deletion.
diff --git a/internal/infrastructure/kubernetes/applier/apply.go b/internal/infrastructure/kubernetes/applier/apply.go
@@ -8,6 +8,12 @@ package applier
 import (
 	"context"
 	"fmt"
+<<<<<<< HEAD
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+=======
+	"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/utils"
+>>>>>>> a1a01fb (bugfix #1340: not compare all svc.spec for user modified scene)
 	"reflect"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -122,7 +128,13 @@ func (i *Instance) CreateOrUpdateService(ctx context.Context, svc *corev1.Servic
 		}
 	} else {
 		// Update if current value is different.
-		if !reflect.DeepEqual(svc.Spec, current.Spec) {
+<<<<<<< HEAD
+		// Only compare the selector and ports(not include nodePort) in case user have modified for some scene.
+		if !cmp.Equal(svc.Spec.Selector, current.Spec.Selector) &&
+			!cmp.Equal(svc.Spec.Ports, current.Spec.Ports, cmpopts.IgnoreFields(corev1.ServicePort{}, "NodePort")) {
+=======
+		if !utils.CompareSvc(svc, current) {
+>>>>>>> a1a01fb (bugfix #1340: not compare all svc.spec for user modified scene)
 			svc.ResourceVersion = current.ResourceVersion
 			svc.UID = current.UID
 			if err := i.Client.Update(ctx, svc); err != nil {

diff --git a/internal/infrastructure/kubernetes/utils/utils.go b/internal/infrastructure/kubernetes/utils/utils.go
@@ -6,6 +6,8 @@
 package utils
 
 import (
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -30,3 +32,9 @@ func ExpectedServiceSpec(serviceType *egcfgv1a1.ServiceType) corev1.ServiceSpec
 	}
 	return serviceSpec
 }
+
+// CompareSvc Only compare the selector and ports(not include nodePort) in case user have modified for some scene.
+func CompareSvc(newSvc, originalSvc *corev1.Service) bool {
+	return cmp.Equal(newSvc.Spec.Selector, originalSvc.Spec.Selector) &&
+		cmp.Equal(newSvc.Spec.Ports, originalSvc.Spec.Ports, cmpopts.IgnoreFields(corev1.ServicePort{}, "NodePort"))
+}
diff --git a/internal/infrastructure/kubernetes/utils/utils_test.go b/internal/infrastructure/kubernetes/utils/utils_test.go
@@ -6,6 +6,8 @@
 package utils
 
 import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -76,3 +78,101 @@ func TestGetSelector(t *testing.T) {
 		})
 	}
 }
+
+func TestCompareSvc(t *testing.T) {
+	cases := []struct {
+		ExpectRet   bool
+		NewSvc      *corev1.Service
+		OriginalSvc *corev1.Service
+	}{
+		{
+			ExpectRet: true,
+			NewSvc: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "my-service",
+					Namespace: "default",
+				},
+				Spec: corev1.ServiceSpec{
+					Ports: []corev1.ServicePort{
+						{
+							Name:       "http",
+							Port:       80,
+							NodePort:   30000,
+							TargetPort: intstr.FromInt(8080),
+						},
+					},
+					Selector: map[string]string{
+						"app": "my-app",
+					},
+					Type: "NodePort",
+				},
+			},
+			OriginalSvc: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "my-service",
+					Namespace: "default",
+				},
+				Spec: corev1.ServiceSpec{
+					Ports: []corev1.ServicePort{
+						{
+							Name:       "http",
+							Port:       80,
+							NodePort:   30001,
+							TargetPort: intstr.FromInt(8080),
+						},
+					},
+					Selector: map[string]string{
+						"app": "my-app",
+					},
+					Type: "ClusterIP",
+				},
+			},
+		}, {
+			ExpectRet: false,
+			NewSvc: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "my-service",
+					Namespace: "default",
+				},
+				Spec: corev1.ServiceSpec{
+					Ports: []corev1.ServicePort{
+						{
+							Name:       "http",
+							Port:       80,
+							TargetPort: intstr.FromInt(8080),
+						},
+					},
+					Selector: map[string]string{
+						"app": "my-app",
+					},
+					Type: "ClusterIP",
+				},
+			},
+			OriginalSvc: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "my-service",
+					Namespace: "default",
+				},
+				Spec: corev1.ServiceSpec{
+					Ports: []corev1.ServicePort{
+						{
+							Name:       "http",
+							Port:       90,
+							TargetPort: intstr.FromInt(8080),
+						},
+					},
+					Selector: map[string]string{
+						"app": "my-app",
+					},
+					Type: "ClusterIP",
+				},
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run("", func(t *testing.T) {
+			assert.Equal(t, tc.ExpectRet, CompareSvc(tc.NewSvc, tc.OriginalSvc), "expectedCompareSvcReturn(%v)", tc.ExpectRet)
+		})
+	}
+}