Skip to content

Commit

Permalink
bugfix #1340: not compare all svc.spec for user modified scene
Browse files Browse the repository at this point in the history
Signed-off-by: spwangxp <wangshengpeng@cestc.cn>
  • Loading branch information
spwangxp committed Apr 23, 2023
1 parent 690a045 commit 79b9cf8
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 1 deletion.
3 changes: 2 additions & 1 deletion internal/infrastructure/kubernetes/applier/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package applier
import (
"context"
"fmt"
"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/utils"
"reflect"

appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -122,7 +123,7 @@ func (i *Instance) CreateOrUpdateService(ctx context.Context, svc *corev1.Servic
}
} else {
// Update if current value is different.
if !reflect.DeepEqual(svc.Spec, current.Spec) {
if !utils.CompareSvc(svc, current) {
svc.ResourceVersion = current.ResourceVersion
svc.UID = current.UID
if err := i.Client.Update(ctx, svc); err != nil {
Expand Down
8 changes: 8 additions & 0 deletions internal/infrastructure/kubernetes/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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"))
}
100 changes: 100 additions & 0 deletions internal/infrastructure/kubernetes/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
})
}
}

0 comments on commit 79b9cf8

Please sign in to comment.