Skip to content

Commit 81108f2

Browse files
ardikabsardikabs
authored
refactor: infra client CreateOrUpdate to ServerSideApply (#3134)
* refactor(infra-client): CreateOrUpdate to ServerSideApply Signed-off-by: Ardika Bagus <[email protected]> * test(infra-client): add e2e test for ServerSideApply Signed-off-by: Ardika Bagus <[email protected]> * chore: remove comment Signed-off-by: Ardika Bagus <[email protected]> * chore: fix linter Signed-off-by: Ardika Bagus <[email protected]> --------- Signed-off-by: Ardika Bagus <[email protected]>
1 parent b45c4c4 commit 81108f2

File tree

5 files changed

+198
-85
lines changed

5 files changed

+198
-85
lines changed

‎internal/infrastructure/kubernetes/infra_client.go‎

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
kerrors "k8s.io/apimachinery/pkg/api/errors"
1313
"k8s.io/apimachinery/pkg/types"
14-
"k8s.io/client-go/util/retry"
1514
"sigs.k8s.io/controller-runtime/pkg/client"
1615
)
1716

@@ -25,28 +24,13 @@ func New(cli client.Client) *InfraClient {
2524
}
2625
}
2726

28-
func (cli *InfraClient) CreateOrUpdate(ctx context.Context, key client.ObjectKey, current client.Object, specific client.Object, updateChecker func() bool) error {
29-
return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
30-
if err := cli.Client.Get(ctx, key, current); err != nil {
31-
if kerrors.IsNotFound(err) {
32-
// Create if it does not exist.
33-
if err := cli.Client.Create(ctx, specific); err != nil {
34-
return fmt.Errorf("for Create: %w", err)
35-
}
36-
}
37-
} else {
38-
// Since the client.Object does not have a specific Spec field to compare
39-
// just perform an update for now.
40-
if updateChecker() {
41-
opts := []client.PatchOption{client.ForceOwnership, client.FieldOwner("envoy-gateway")}
42-
if err := cli.Client.Patch(ctx, specific, client.Apply, opts...); err != nil {
43-
return fmt.Errorf("for Update: %w", err)
44-
}
45-
}
46-
}
27+
func (cli *InfraClient) ServerSideApply(ctx context.Context, obj client.Object) error {
28+
opts := []client.PatchOption{client.ForceOwnership, client.FieldOwner("envoy-gateway")}
29+
if err := cli.Client.Patch(ctx, obj, client.Apply, opts...); err != nil {
30+
return fmt.Errorf("failed to create/update resource with server-side apply for obj %v: %w", obj, err)
31+
}
4732

48-
return nil
49-
})
33+
return nil
5034
}
5135

5236
func (cli *InfraClient) Delete(ctx context.Context, object client.Object) error {

‎internal/infrastructure/kubernetes/infra_resource.go‎

Lines changed: 5 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,11 @@ package kubernetes
77

88
import (
99
"context"
10-
"reflect"
1110

12-
"github.com/google/go-cmp/cmp"
13-
"github.com/google/go-cmp/cmp/cmpopts"
1411
appsv1 "k8s.io/api/apps/v1"
1512
autoscalingv2 "k8s.io/api/autoscaling/v2"
1613
corev1 "k8s.io/api/core/v1"
1714
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
18-
"k8s.io/apimachinery/pkg/types"
19-
20-
"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/resource"
21-
"github.com/envoyproxy/gateway/internal/utils"
2215
)
2316

2417
// createOrUpdateServiceAccount creates a ServiceAccount in the kube api server based on the
@@ -29,14 +22,7 @@ func (i *Infra) createOrUpdateServiceAccount(ctx context.Context, r ResourceRend
2922
return err
3023
}
3124

32-
current := &corev1.ServiceAccount{}
33-
key := utils.NamespacedName(sa)
34-
35-
return i.Client.CreateOrUpdate(ctx, key, current, sa, func() bool {
36-
// the service account never changed, does not need to update
37-
// fixes https://github.com/envoyproxy/gateway/issues/1604
38-
return false
39-
})
25+
return i.Client.ServerSideApply(ctx, sa)
4026
}
4127

4228
// createOrUpdateConfigMap creates a ConfigMap in the Kube api server based on the provided
@@ -50,15 +36,8 @@ func (i *Infra) createOrUpdateConfigMap(ctx context.Context, r ResourceRender) e
5036
if cm == nil {
5137
return nil
5238
}
53-
current := &corev1.ConfigMap{}
54-
key := types.NamespacedName{
55-
Namespace: cm.Namespace,
56-
Name: cm.Name,
57-
}
5839

59-
return i.Client.CreateOrUpdate(ctx, key, current, cm, func() bool {
60-
return !reflect.DeepEqual(cm.Data, current.Data)
61-
})
40+
return i.Client.ServerSideApply(ctx, cm)
6241
}
6342

6443
// createOrUpdateDeployment creates a Deployment in the kube api server based on the provided
@@ -69,25 +48,7 @@ func (i *Infra) createOrUpdateDeployment(ctx context.Context, r ResourceRender)
6948
return err
7049
}
7150

72-
current := &appsv1.Deployment{}
73-
key := types.NamespacedName{
74-
Namespace: deployment.Namespace,
75-
Name: deployment.Name,
76-
}
77-
78-
hpa, err := r.HorizontalPodAutoscaler()
79-
if err != nil {
80-
return err
81-
}
82-
83-
var opts cmp.Options
84-
if hpa != nil {
85-
opts = append(opts, cmpopts.IgnoreFields(appsv1.DeploymentSpec{}, "Replicas"))
86-
}
87-
88-
return i.Client.CreateOrUpdate(ctx, key, current, deployment, func() bool {
89-
return !cmp.Equal(current.Spec, deployment.Spec, opts...)
90-
})
51+
return i.Client.ServerSideApply(ctx, deployment)
9152
}
9253

9354
// createOrUpdateHPA creates HorizontalPodAutoscaler object in the kube api server based on
@@ -105,15 +66,7 @@ func (i *Infra) createOrUpdateHPA(ctx context.Context, r ResourceRender) error {
10566
return i.deleteHPA(ctx, r)
10667
}
10768

108-
current := &autoscalingv2.HorizontalPodAutoscaler{}
109-
key := types.NamespacedName{
110-
Namespace: hpa.Namespace,
111-
Name: hpa.Name,
112-
}
113-
114-
return i.Client.CreateOrUpdate(ctx, key, current, hpa, func() bool {
115-
return !cmp.Equal(hpa.Spec, current.Spec)
116-
})
69+
return i.Client.ServerSideApply(ctx, hpa)
11770
}
11871

11972
// createOrUpdateRateLimitService creates a Service in the kube api server based on the provided ResourceRender,
@@ -124,15 +77,7 @@ func (i *Infra) createOrUpdateService(ctx context.Context, r ResourceRender) err
12477
return err
12578
}
12679

127-
current := &corev1.Service{}
128-
key := types.NamespacedName{
129-
Namespace: svc.Namespace,
130-
Name: svc.Name,
131-
}
132-
133-
return i.Client.CreateOrUpdate(ctx, key, current, svc, func() bool {
134-
return !resource.CompareSvc(svc, current)
135-
})
80+
return i.Client.ServerSideApply(ctx, svc)
13681
}
13782

13883
// deleteServiceAccount deletes the ServiceAccount in the kube api server, if it exists.

‎internal/infrastructure/kubernetes/proxy_deployment_test.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,11 @@ func TestCreateOrUpdateProxyDeployment(t *testing.T) {
136136
}
137137

138138
func TestDeleteProxyDeployment(t *testing.T) {
139-
cli := fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).WithObjects().Build()
139+
cli := fakeclient.NewClientBuilder().
140+
WithScheme(envoygateway.GetScheme()).
141+
WithObjects().
142+
WithInterceptorFuncs(interceptorFunc).
143+
Build()
140144
cfg, err := config.New()
141145
require.NoError(t, err)
142146

‎test/cel-validation/clienttrafficpolicy_test.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,18 @@ package celvalidation
1111
import (
1212
"context"
1313
"fmt"
14-
"k8s.io/apimachinery/pkg/api/resource"
1514
"strings"
1615
"testing"
1716
"time"
1817

19-
egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
18+
"k8s.io/apimachinery/pkg/api/resource"
19+
2020
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2121
"k8s.io/utils/ptr"
2222
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
2323
gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
24+
25+
egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
2426
)
2527

2628
func TestClientTrafficPolicyTarget(t *testing.T) {

‎test/e2e/tests/gateway_infra_resource.go‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
// Copyright Envoy Gateway Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
// The full text of the Apache license is available in the LICENSE file at
4+
// the root of the repo.
5+
6+
//go:build e2e
7+
// +build e2e
8+
9+
package tests
10+
11+
import (
12+
"context"
13+
"sync"
14+
"testing"
15+
"time"
16+
17+
"github.com/stretchr/testify/require"
18+
appsv1 "k8s.io/api/apps/v1"
19+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
20+
"k8s.io/apimachinery/pkg/labels"
21+
"sigs.k8s.io/controller-runtime/pkg/client"
22+
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
23+
"sigs.k8s.io/gateway-api/conformance/utils/suite"
24+
25+
"github.com/envoyproxy/gateway/internal/utils"
26+
)
27+
28+
func init() {
29+
ConformanceTests = append(ConformanceTests, GatewayInfraResourceTest)
30+
}
31+
32+
var GatewayInfraResourceTest = suite.ConformanceTest{
33+
ShortName: "GatewayInfraResourceTest",
34+
Description: "Gateway Infra Resource E2E Test",
35+
Test: func(t *testing.T, suite *suite.ConformanceTestSuite) {
36+
gatewayTypeMeta := metav1.TypeMeta{
37+
Kind: "Gateway",
38+
APIVersion: "gateway.networking.k8s.io/v1",
39+
}
40+
gatewayObjMeta := metav1.ObjectMeta{
41+
Name: "e2e-test-infra",
42+
Namespace: "envoy-gateway-system",
43+
}
44+
45+
labelSelector := labels.SelectorFromSet(labels.Set{"gateway.envoyproxy.io/owning-gateway-name": gatewayObjMeta.Name})
46+
47+
var awaitOperation sync.WaitGroup
48+
49+
t.Run("create gateway", func(t *testing.T) {
50+
awaitOperation.Add(1)
51+
52+
newGatewayObj := &gwapiv1.Gateway{
53+
TypeMeta: gatewayTypeMeta,
54+
ObjectMeta: gatewayObjMeta,
55+
Spec: gwapiv1.GatewaySpec{
56+
GatewayClassName: gwapiv1.ObjectName(suite.GatewayClassName),
57+
Listeners: []gwapiv1.Listener{
58+
{
59+
Name: "http",
60+
Port: 8000,
61+
Protocol: "HTTP",
62+
},
63+
{
64+
Name: "my-tcp",
65+
Port: 5432,
66+
Protocol: "TCP",
67+
},
68+
},
69+
},
70+
}
71+
72+
err := suite.Client.Patch(context.TODO(), newGatewayObj, client.Apply, client.ForceOwnership, client.FieldOwner("e2e-test"))
73+
require.NoError(t, err)
74+
75+
<-time.After(time.Millisecond * 300)
76+
77+
var gatewayDeploymentList appsv1.DeploymentList
78+
err = suite.Client.List(context.TODO(), &gatewayDeploymentList, &client.ListOptions{
79+
LabelSelector: labelSelector,
80+
Namespace: gatewayObjMeta.Namespace,
81+
})
82+
require.NoError(t, err)
83+
require.Len(t, gatewayDeploymentList.Items, 1)
84+
85+
awaitOperation.Done()
86+
})
87+
88+
awaitOperation.Wait()
89+
t.Run("update gateway - listener changes", func(t *testing.T) {
90+
awaitOperation.Add(1)
91+
92+
newListenerTCPName := "custom-tcp"
93+
newListenerHTTPPort := int32(8001)
94+
95+
changedGatewayObj := &gwapiv1.Gateway{
96+
TypeMeta: gatewayTypeMeta,
97+
ObjectMeta: gatewayObjMeta,
98+
Spec: gwapiv1.GatewaySpec{
99+
GatewayClassName: gwapiv1.ObjectName(suite.GatewayClassName),
100+
Listeners: []gwapiv1.Listener{
101+
{
102+
Name: "http",
103+
Port: gwapiv1.PortNumber(newListenerHTTPPort),
104+
Protocol: "HTTP",
105+
},
106+
{
107+
Name: gwapiv1.SectionName(newListenerTCPName),
108+
Port: 5432,
109+
Protocol: "TCP",
110+
},
111+
},
112+
},
113+
}
114+
115+
err := suite.Client.Patch(context.TODO(), changedGatewayObj, client.Apply, client.ForceOwnership, client.FieldOwner("e2e-test"))
116+
require.NoError(t, err)
117+
118+
<-time.After(time.Millisecond * 300)
119+
var gatewayDeploymentList appsv1.DeploymentList
120+
err = suite.Client.List(context.TODO(), &gatewayDeploymentList, &client.ListOptions{
121+
LabelSelector: labelSelector,
122+
Namespace: gatewayObjMeta.Namespace,
123+
})
124+
require.NoError(t, err)
125+
require.Len(t, gatewayDeploymentList.Items, 1)
126+
127+
gatewayDeployment := gatewayDeploymentList.Items[0]
128+
129+
for _, container := range gatewayDeployment.Spec.Template.Spec.Containers {
130+
var isTCPPortNameMatch, isHTTPPortNumberMatch bool
131+
132+
hashedPortName := utils.GetHashedName(newListenerTCPName, 6)
133+
if container.Name == "envoy" {
134+
for _, port := range container.Ports {
135+
if port.Name == hashedPortName {
136+
isTCPPortNameMatch = true
137+
}
138+
139+
if port.ContainerPort == newListenerHTTPPort {
140+
isHTTPPortNumberMatch = true
141+
}
142+
}
143+
144+
if !isTCPPortNameMatch {
145+
t.Errorf("container expected TCP port name '%v' is not found", hashedPortName)
146+
}
147+
148+
if !isHTTPPortNumberMatch {
149+
t.Errorf("container expected HTTP port number '%v' is not found", hashedPortName)
150+
}
151+
}
152+
}
153+
154+
awaitOperation.Done()
155+
})
156+
157+
awaitOperation.Wait()
158+
t.Run("delete gateway", func(t *testing.T) {
159+
gwObj := &gwapiv1.Gateway{
160+
TypeMeta: gatewayTypeMeta,
161+
ObjectMeta: gatewayObjMeta,
162+
}
163+
164+
err := suite.Client.Delete(context.TODO(), gwObj)
165+
require.NoError(t, err)
166+
167+
<-time.After(time.Millisecond * 300)
168+
169+
var gatewayDeploymentList appsv1.DeploymentList
170+
err = suite.Client.List(context.TODO(), &gatewayDeploymentList, &client.ListOptions{
171+
LabelSelector: labelSelector,
172+
Namespace: gatewayObjMeta.Namespace,
173+
})
174+
require.NoError(t, err)
175+
require.Empty(t, gatewayDeploymentList.Items)
176+
})
177+
},
178+
}

0 commit comments

Comments
 (0)