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 27, 2023
1 parent a5d3f36 commit 87488d9
Show file tree
Hide file tree
Showing 4 changed files with 163 additions and 8 deletions.
2 changes: 1 addition & 1 deletion internal/infrastructure/kubernetes/infra_client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func New(cli client.Client) *InfraClient {
}
}

func (cli *InfraClient) Create(ctx context.Context, key client.ObjectKey, current client.Object, specific client.Object, updateChecker func() bool) error {
func (cli *InfraClient) CreateOrUpdate(ctx context.Context, key client.ObjectKey, current client.Object, specific client.Object, updateChecker func() bool) error {
if err := cli.Client.Get(ctx, key, current); err != nil {
if kerrors.IsNotFound(err) {
// Create if it does not exist.
Expand Down
11 changes: 6 additions & 5 deletions internal/infrastructure/kubernetes/infra_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"context"
"reflect"

"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/resource"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -29,7 +30,7 @@ func (i *Infra) createOrUpdateServiceAccount(ctx context.Context, r ResourceRend
Name: sa.Name,
}

return i.Client.Create(ctx, key, current, sa, func() bool {
return i.Client.CreateOrUpdate(ctx, key, current, sa, func() bool {
return true
})
}
Expand All @@ -48,7 +49,7 @@ func (i *Infra) createOrUpdateConfigMap(ctx context.Context, r ResourceRender) e
Name: cm.Name,
}

return i.Client.Create(ctx, key, current, cm, func() bool {
return i.Client.CreateOrUpdate(ctx, key, current, cm, func() bool {
return !reflect.DeepEqual(cm.Data, current.Data)
})
}
Expand All @@ -67,7 +68,7 @@ func (i *Infra) createOrUpdateDeployment(ctx context.Context, r ResourceRender)
Name: deployment.Name,
}

return i.Client.Create(ctx, key, current, deployment, func() bool {
return i.Client.CreateOrUpdate(ctx, key, current, deployment, func() bool {
return !reflect.DeepEqual(deployment.Spec, current.Spec)
})
}
Expand All @@ -86,8 +87,8 @@ func (i *Infra) createOrUpdateService(ctx context.Context, r ResourceRender) err
Name: svc.Name,
}

return i.Client.Create(ctx, key, current, svc, func() bool {
return !reflect.DeepEqual(svc.Spec, current.Spec)
return i.Client.CreateOrUpdate(ctx, key, current, svc, func() bool {
return !resource.CompareSvc(svc, current)
})
}

Expand Down
11 changes: 9 additions & 2 deletions internal/infrastructure/kubernetes/resource/resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
package resource

import (
egcfgv1a1 "github.com/envoyproxy/gateway/api/config/v1alpha1"
"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"

egcfgv1a1 "github.com/envoyproxy/gateway/api/config/v1alpha1"
)

// GetSelector returns a label selector used to select resources
Expand All @@ -31,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(currentSvc, originalSvc *corev1.Service) bool {
return cmp.Equal(currentSvc.Spec.Selector, originalSvc.Spec.Selector) &&
cmp.Equal(currentSvc.Spec.Ports, originalSvc.Spec.Ports, cmpopts.IgnoreFields(corev1.ServicePort{}, "NodePort"))
}
147 changes: 147 additions & 0 deletions internal/infrastructure/kubernetes/resource/resource_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ package resource
import (
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -76,3 +79,147 @@ func TestGetSelector(t *testing.T) {
})
}
}

func TestCompareSvc(t *testing.T) {
cases := []struct {
ExpectRet bool
NewSvc *corev1.Service
OriginalSvc *corev1.Service
}{
{
// Only Spec.Ports[*].NodePort and ClusterType is different
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",
},
},
}, {
// Only Spec.Ports[*].Port is different
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",
},
},
},
{
// only Spec.ClusterIP is different, NewSvc's ClusterIP is nil build by ResourceRender
ExpectRet: true,
NewSvc: &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "my-service",
Namespace: "default",
},
Spec: corev1.ServiceSpec{
ClusterIP: "",
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{
ClusterIP: "192.168.1.1",
Ports: []corev1.ServicePort{
{
Name: "http",
Port: 80,
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 87488d9

Please sign in to comment.