From ac0f611350cd8bc6a299aff439916c07efd73c7c Mon Sep 17 00:00:00 2001 From: Yuneui Jeong Date: Tue, 5 Mar 2024 14:40:04 +0900 Subject: [PATCH 1/5] Delete unused status keys in gatewayapi-runner Signed-off-by: Yuneui Jeong --- internal/gatewayapi/runner/runner.go | 168 ++++++++++++++++++++++++++- 1 file changed, 163 insertions(+), 5 deletions(-) diff --git a/internal/gatewayapi/runner/runner.go b/internal/gatewayapi/runner/runner.go index ca72bbfe1e3..553a7cd17fe 100644 --- a/internal/gatewayapi/runner/runner.go +++ b/internal/gatewayapi/runner/runner.go @@ -9,6 +9,7 @@ import ( "context" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" v1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/envoyproxy/gateway/api/v1alpha1" @@ -56,15 +57,21 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { // so when a delete is triggered, delete all IR keys if update.Delete || val == nil { r.deleteAllIRKeys() + r.deleteAllStatusKeys() return } - var curKeys, newKeys []string + // IR keys for watchable + var curIRKeys, newIRKeys []string + // Get current IR keys for key := range r.InfraIR.LoadAll() { - curKeys = append(curKeys, key) + curIRKeys = append(curIRKeys, key) } + // Get the current DeletableStatus which manages status keys to be deleted + deletableStatus := r.getDeletableStatus() + for _, resources := range *val { // Translate and publish IRs. t := &gatewayapi.Translator{ @@ -95,7 +102,7 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { errChan <- err } else { r.InfraIR.Store(key, val) - newKeys = append(newKeys, key) + newIRKeys = append(newIRKeys, key) } } @@ -114,61 +121,75 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { gateway := gateway key := utils.NamespacedName(gateway) r.ProviderResources.GatewayStatuses.Store(key, &gateway.Status) + delete(deletableStatus.GatewayStatusKeys, key) } for _, httpRoute := range result.HTTPRoutes { httpRoute := httpRoute key := utils.NamespacedName(httpRoute) r.ProviderResources.HTTPRouteStatuses.Store(key, &httpRoute.Status) + delete(deletableStatus.HTTPRouteStatusKeys, key) } for _, grpcRoute := range result.GRPCRoutes { grpcRoute := grpcRoute key := utils.NamespacedName(grpcRoute) r.ProviderResources.GRPCRouteStatuses.Store(key, &grpcRoute.Status) + delete(deletableStatus.GRPCRouteStatusKeys, key) } for _, tlsRoute := range result.TLSRoutes { tlsRoute := tlsRoute key := utils.NamespacedName(tlsRoute) r.ProviderResources.TLSRouteStatuses.Store(key, &tlsRoute.Status) + delete(deletableStatus.TLSRouteStatusKeys, key) } for _, tcpRoute := range result.TCPRoutes { tcpRoute := tcpRoute key := utils.NamespacedName(tcpRoute) r.ProviderResources.TCPRouteStatuses.Store(key, &tcpRoute.Status) + delete(deletableStatus.TCPRouteStatusKeys, key) } for _, udpRoute := range result.UDPRoutes { udpRoute := udpRoute key := utils.NamespacedName(udpRoute) r.ProviderResources.UDPRouteStatuses.Store(key, &udpRoute.Status) + delete(deletableStatus.UDPRouteStatusKeys, key) } for _, clientTrafficPolicy := range result.ClientTrafficPolicies { clientTrafficPolicy := clientTrafficPolicy key := utils.NamespacedName(clientTrafficPolicy) r.ProviderResources.ClientTrafficPolicyStatuses.Store(key, &clientTrafficPolicy.Status) + delete(deletableStatus.ClientTrafficPolicyStatusKeys, key) } for _, backendTrafficPolicy := range result.BackendTrafficPolicies { backendTrafficPolicy := backendTrafficPolicy key := utils.NamespacedName(backendTrafficPolicy) r.ProviderResources.BackendTrafficPolicyStatuses.Store(key, &backendTrafficPolicy.Status) + delete(deletableStatus.BackendTrafficPolicyStatusKeys, key) } for _, securityPolicy := range result.SecurityPolicies { securityPolicy := securityPolicy key := utils.NamespacedName(securityPolicy) r.ProviderResources.SecurityPolicyStatuses.Store(key, &securityPolicy.Status) + delete(deletableStatus.SecurityPolicyStatusKeys, key) } for _, backendTLSPolicy := range result.BackendTLSPolicies { backendTLSPolicy := backendTLSPolicy key := utils.NamespacedName(backendTLSPolicy) r.ProviderResources.BackendTLSPolicyStatuses.Store(key, &backendTLSPolicy.Status) + delete(deletableStatus.BackendTLSPolicyStatusKeys, key) } } - // Delete keys + + // Delete IR keys // There is a 1:1 mapping between infra and xds IR keys - delKeys := getIRKeysToDelete(curKeys, newKeys) + delKeys := getIRKeysToDelete(curIRKeys, newIRKeys) for _, key := range delKeys { r.InfraIR.Delete(key) r.XdsIR.Delete(key) } + + // Delete status keys + r.deleteStatusKeys(deletableStatus) }, ) r.Logger.Info("shutting down") @@ -182,6 +203,143 @@ func (r *Runner) deleteAllIRKeys() { } } +type DeletableStatus struct { + GatewayStatusKeys map[types.NamespacedName]bool + HTTPRouteStatusKeys map[types.NamespacedName]bool + GRPCRouteStatusKeys map[types.NamespacedName]bool + TLSRouteStatusKeys map[types.NamespacedName]bool + TCPRouteStatusKeys map[types.NamespacedName]bool + UDPRouteStatusKeys map[types.NamespacedName]bool + + ClientTrafficPolicyStatusKeys map[types.NamespacedName]bool + BackendTrafficPolicyStatusKeys map[types.NamespacedName]bool + SecurityPolicyStatusKeys map[types.NamespacedName]bool + BackendTLSPolicyStatusKeys map[types.NamespacedName]bool +} + +func (r *Runner) getDeletableStatus() *DeletableStatus { + // Maps storing status keys to be deleted + ds := &DeletableStatus{ + GatewayStatusKeys: make(map[types.NamespacedName]bool), + HTTPRouteStatusKeys: make(map[types.NamespacedName]bool), + GRPCRouteStatusKeys: make(map[types.NamespacedName]bool), + TLSRouteStatusKeys: make(map[types.NamespacedName]bool), + TCPRouteStatusKeys: make(map[types.NamespacedName]bool), + UDPRouteStatusKeys: make(map[types.NamespacedName]bool), + + ClientTrafficPolicyStatusKeys: make(map[types.NamespacedName]bool), + BackendTrafficPolicyStatusKeys: make(map[types.NamespacedName]bool), + SecurityPolicyStatusKeys: make(map[types.NamespacedName]bool), + BackendTLSPolicyStatusKeys: make(map[types.NamespacedName]bool), + } + + // Get current status keys + for key := range r.ProviderResources.GatewayStatuses.LoadAll() { + ds.GatewayStatusKeys[key] = true + } + for key := range r.ProviderResources.HTTPRouteStatuses.LoadAll() { + ds.HTTPRouteStatusKeys[key] = true + } + for key := range r.ProviderResources.GRPCRouteStatuses.LoadAll() { + ds.GRPCRouteStatusKeys[key] = true + } + for key := range r.ProviderResources.TLSRouteStatuses.LoadAll() { + ds.TLSRouteStatusKeys[key] = true + } + for key := range r.ProviderResources.TCPRouteStatuses.LoadAll() { + ds.TCPRouteStatusKeys[key] = true + } + for key := range r.ProviderResources.UDPRouteStatuses.LoadAll() { + ds.UDPRouteStatusKeys[key] = true + } + + for key := range r.ProviderResources.ClientTrafficPolicyStatuses.LoadAll() { + ds.ClientTrafficPolicyStatusKeys[key] = true + } + for key := range r.ProviderResources.BackendTrafficPolicyStatuses.LoadAll() { + ds.BackendTrafficPolicyStatusKeys[key] = true + } + for key := range r.ProviderResources.SecurityPolicyStatuses.LoadAll() { + ds.SecurityPolicyStatusKeys[key] = true + } + for key := range r.ProviderResources.BackendTLSPolicyStatuses.LoadAll() { + ds.BackendTLSPolicyStatusKeys[key] = true + } + + return ds +} + +func (r *Runner) deleteStatusKeys(ds *DeletableStatus) { + for key := range ds.GatewayStatusKeys { + r.ProviderResources.GatewayStatuses.Delete(key) + } + for key := range ds.HTTPRouteStatusKeys { + r.ProviderResources.HTTPRouteStatuses.Delete(key) + } + for key := range ds.GRPCRouteStatusKeys { + r.ProviderResources.GRPCRouteStatuses.Delete(key) + } + for key := range ds.TLSRouteStatusKeys { + r.ProviderResources.TLSRouteStatuses.Delete(key) + } + for key := range ds.TCPRouteStatusKeys { + r.ProviderResources.TCPRouteStatuses.Delete(key) + } + for key := range ds.UDPRouteStatusKeys { + r.ProviderResources.UDPRouteStatuses.Delete(key) + } + + for key := range ds.ClientTrafficPolicyStatusKeys { + r.ProviderResources.ClientTrafficPolicyStatuses.Delete(key) + } + for key := range ds.BackendTrafficPolicyStatusKeys { + r.ProviderResources.BackendTrafficPolicyStatuses.Delete(key) + } + for key := range ds.SecurityPolicyStatusKeys { + r.ProviderResources.SecurityPolicyStatuses.Delete(key) + } + for key := range ds.BackendTLSPolicyStatusKeys { + r.ProviderResources.BackendTLSPolicyStatuses.Delete(key) + } +} + +// deleteAllStatusKeys deletes all status keys stored by the subscriber. +func (r *Runner) deleteAllStatusKeys() { + // Fields of GatewayAPIStatuses + for key := range r.ProviderResources.GatewayStatuses.LoadAll() { + r.ProviderResources.GatewayStatuses.Delete(key) + } + for key := range r.ProviderResources.HTTPRouteStatuses.LoadAll() { + r.ProviderResources.HTTPRouteStatuses.Delete(key) + } + for key := range r.ProviderResources.GRPCRouteStatuses.LoadAll() { + r.ProviderResources.GRPCRouteStatuses.Delete(key) + } + for key := range r.ProviderResources.TLSRouteStatuses.LoadAll() { + r.ProviderResources.TLSRouteStatuses.Delete(key) + } + for key := range r.ProviderResources.TCPRouteStatuses.LoadAll() { + r.ProviderResources.TCPRouteStatuses.Delete(key) + } + for key := range r.ProviderResources.UDPRouteStatuses.LoadAll() { + r.ProviderResources.UDPRouteStatuses.Delete(key) + } + + // Fields of PolicyStatuses + for key := range r.ProviderResources.ClientTrafficPolicyStatuses.LoadAll() { + r.ProviderResources.ClientTrafficPolicyStatuses.Delete(key) + } + for key := range r.ProviderResources.BackendTrafficPolicyStatuses.LoadAll() { + r.ProviderResources.BackendTrafficPolicyStatuses.Delete(key) + } + for key := range r.ProviderResources.SecurityPolicyStatuses.LoadAll() { + r.ProviderResources.SecurityPolicyStatuses.Delete(key) + } + for key := range r.ProviderResources.BackendTLSPolicyStatuses.LoadAll() { + r.ProviderResources.BackendTLSPolicyStatuses.Delete(key) + } +} + // getIRKeysToDelete returns the list of IR keys to delete // based on the difference between the current keys and the // new keys parameters passed to the function. From a1bcbd8d5e637815508eeefc3a2b57aa00a805d4 Mon Sep 17 00:00:00 2001 From: Yuneui Jeong Date: Tue, 5 Mar 2024 14:49:08 +0900 Subject: [PATCH 2/5] Delete unused status keys in xds-translator runner Signed-off-by: Yuneui Jeong --- internal/xds/translator/runner/runner.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/internal/xds/translator/runner/runner.go b/internal/xds/translator/runner/runner.go index bae2a9fe939..b8f6cdc9591 100644 --- a/internal/xds/translator/runner/runner.go +++ b/internal/xds/translator/runner/runner.go @@ -8,6 +8,7 @@ package runner import ( "context" + "k8s.io/apimachinery/pkg/types" ktypes "k8s.io/apimachinery/pkg/types" "github.com/envoyproxy/gateway/api/v1alpha1" @@ -90,6 +91,12 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { return } + // Get current status keys and mark them as deletable temporarily + deletableStatus := make(map[types.NamespacedName]bool) + for key := range r.ProviderResources.EnvoyPatchPolicyStatuses.LoadAll() { + deletableStatus[key] = true + } + // Publish EnvoyPatchPolicyStatus for _, e := range result.EnvoyPatchPolicyStatuses { key := ktypes.NamespacedName{ @@ -97,12 +104,18 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { Namespace: e.Namespace, } r.ProviderResources.EnvoyPatchPolicyStatuses.Store(key, e.Status) + delete(deletableStatus, key) } // Discard the EnvoyPatchPolicyStatuses to reduce memory footprint result.EnvoyPatchPolicyStatuses = nil // Publish r.Xds.Store(key, result) + + // Delete all the deletable status keys + for key := range deletableStatus { + r.ProviderResources.EnvoyPatchPolicyStatuses.Delete(key) + } } }, ) From d6f12420b8f5a910e0750b8a774c61831b203576 Mon Sep 17 00:00:00 2001 From: Yuneui Jeong Date: Tue, 5 Mar 2024 17:19:16 +0900 Subject: [PATCH 3/5] Add tests and fix code to pass all tests Signed-off-by: Yuneui Jeong --- internal/gatewayapi/runner/runner.go | 10 +++ internal/gatewayapi/runner/runner_test.go | 88 +++++++++++++++++++ internal/xds/translator/runner/runner.go | 3 +- internal/xds/translator/runner/runner_test.go | 19 ++-- 4 files changed, 111 insertions(+), 9 deletions(-) diff --git a/internal/gatewayapi/runner/runner.go b/internal/gatewayapi/runner/runner.go index 553a7cd17fe..554c002f0aa 100644 --- a/internal/gatewayapi/runner/runner.go +++ b/internal/gatewayapi/runner/runner.go @@ -272,34 +272,44 @@ func (r *Runner) getDeletableStatus() *DeletableStatus { func (r *Runner) deleteStatusKeys(ds *DeletableStatus) { for key := range ds.GatewayStatusKeys { r.ProviderResources.GatewayStatuses.Delete(key) + delete(ds.GatewayStatusKeys, key) } for key := range ds.HTTPRouteStatusKeys { r.ProviderResources.HTTPRouteStatuses.Delete(key) + delete(ds.HTTPRouteStatusKeys, key) } for key := range ds.GRPCRouteStatusKeys { r.ProviderResources.GRPCRouteStatuses.Delete(key) + delete(ds.GRPCRouteStatusKeys, key) } for key := range ds.TLSRouteStatusKeys { r.ProviderResources.TLSRouteStatuses.Delete(key) + delete(ds.TLSRouteStatusKeys, key) } for key := range ds.TCPRouteStatusKeys { r.ProviderResources.TCPRouteStatuses.Delete(key) + delete(ds.TCPRouteStatusKeys, key) } for key := range ds.UDPRouteStatusKeys { r.ProviderResources.UDPRouteStatuses.Delete(key) + delete(ds.UDPRouteStatusKeys, key) } for key := range ds.ClientTrafficPolicyStatusKeys { r.ProviderResources.ClientTrafficPolicyStatuses.Delete(key) + delete(ds.ClientTrafficPolicyStatusKeys, key) } for key := range ds.BackendTrafficPolicyStatusKeys { r.ProviderResources.BackendTrafficPolicyStatuses.Delete(key) + delete(ds.BackendTrafficPolicyStatusKeys, key) } for key := range ds.SecurityPolicyStatusKeys { r.ProviderResources.SecurityPolicyStatuses.Delete(key) + delete(ds.SecurityPolicyStatusKeys, key) } for key := range ds.BackendTLSPolicyStatusKeys { r.ProviderResources.BackendTLSPolicyStatuses.Delete(key) + delete(ds.BackendTLSPolicyStatusKeys, key) } } diff --git a/internal/gatewayapi/runner/runner_test.go b/internal/gatewayapi/runner/runner_test.go index b159933b508..121b7bacf95 100644 --- a/internal/gatewayapi/runner/runner_test.go +++ b/internal/gatewayapi/runner/runner_test.go @@ -13,6 +13,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/types" + v1 "sigs.k8s.io/gateway-api/apis/v1" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/envoygateway/config" @@ -104,3 +106,89 @@ func TestGetIRKeysToDelete(t *testing.T) { }) } } + +func TestDeletableStatus(t *testing.T) { + // Setup + pResources := new(message.ProviderResources) + xdsIR := new(message.XdsIR) + infraIR := new(message.InfraIR) + cfg, err := config.New() + require.NoError(t, err) + r := New(&Config{ + Server: *cfg, + ProviderResources: pResources, + XdsIR: xdsIR, + InfraIR: infraIR, + ExtensionManager: testutils.NewManager(egv1a1.ExtensionManager{}), + }) + ctx := context.Background() + + // Start + err = r.Start(ctx) + require.NoError(t, err) + + // No status is set + ds1 := r.getDeletableStatus() + require.Empty(t, len(ds1.GatewayStatusKeys)) + + // A new status gets stored + keys := []types.NamespacedName{ + { + Name: "test1", + Namespace: "test-namespace", + }, + { + Name: "test2", + Namespace: "test-namespace", + }, + { + Name: "test3", + Namespace: "test-namespace", + }, + } + statuses := []v1.GatewayStatus{ + { + Addresses: []v1.GatewayStatusAddress{ + { + Value: "192.168.0.1", + }, + }, + }, + { + Addresses: []v1.GatewayStatusAddress{ + { + Value: "192.168.0.2", + }, + }, + }, + { + Addresses: []v1.GatewayStatusAddress{ + { + Value: "192.168.0.3", + }, + }, + }, + } + r.ProviderResources.GatewayStatuses.Store(keys[0], &statuses[0]) + r.ProviderResources.GatewayStatuses.Store(keys[1], &statuses[1]) + r.ProviderResources.GatewayStatuses.Store(keys[2], &statuses[2]) + + // getDeletableStatus: Checks that the keys are successfully stored + ds2 := r.getDeletableStatus() + require.True(t, ds2.GatewayStatusKeys[keys[0]]) + require.True(t, ds2.GatewayStatusKeys[keys[1]]) + require.True(t, ds2.GatewayStatusKeys[keys[2]]) + require.Equal(t, 3, r.ProviderResources.GatewayStatuses.Len()) + + // deleteStatusKeys: Delete the third key by removing the first + // and second key from deletables + delete(ds2.GatewayStatusKeys, keys[0]) + delete(ds2.GatewayStatusKeys, keys[1]) + r.deleteStatusKeys(ds2) + require.Empty(t, len(ds2.GatewayStatusKeys)) + require.Equal(t, 2, r.ProviderResources.GatewayStatuses.Len()) + + // deleteAllStatusKeys: Delete remaining keys + r.deleteAllStatusKeys() + require.Equal(t, 0, r.ProviderResources.GatewayStatuses.Len()) +} diff --git a/internal/xds/translator/runner/runner.go b/internal/xds/translator/runner/runner.go index b8f6cdc9591..0d7a0e32752 100644 --- a/internal/xds/translator/runner/runner.go +++ b/internal/xds/translator/runner/runner.go @@ -8,7 +8,6 @@ package runner import ( "context" - "k8s.io/apimachinery/pkg/types" ktypes "k8s.io/apimachinery/pkg/types" "github.com/envoyproxy/gateway/api/v1alpha1" @@ -92,7 +91,7 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { } // Get current status keys and mark them as deletable temporarily - deletableStatus := make(map[types.NamespacedName]bool) + deletableStatus := make(map[ktypes.NamespacedName]bool) for key := range r.ProviderResources.EnvoyPatchPolicyStatuses.LoadAll() { deletableStatus[key] = true } diff --git a/internal/xds/translator/runner/runner_test.go b/internal/xds/translator/runner/runner_test.go index ab8a2c65e78..9f3d7035bd6 100644 --- a/internal/xds/translator/runner/runner_test.go +++ b/internal/xds/translator/runner/runner_test.go @@ -27,12 +27,14 @@ func TestRunner(t *testing.T) { // Setup xdsIR := new(message.XdsIR) xds := new(message.Xds) + pResource := new(message.ProviderResources) cfg, err := config.New() require.NoError(t, err) r := New(&Config{ - Server: *cfg, - XdsIR: xdsIR, - Xds: xds, + Server: *cfg, + ProviderResources: pResource, + XdsIR: xdsIR, + Xds: xds, }) ctx := context.Background() @@ -103,13 +105,16 @@ func TestRunner_withExtensionManager(t *testing.T) { // Setup xdsIR := new(message.XdsIR) xds := new(message.Xds) + pResource := new(message.ProviderResources) + cfg, err := config.New() require.NoError(t, err) r := New(&Config{ - Server: *cfg, - XdsIR: xdsIR, - Xds: xds, - ExtensionManager: &extManagerMock{}, + Server: *cfg, + ProviderResources: pResource, + XdsIR: xdsIR, + Xds: xds, + ExtensionManager: &extManagerMock{}, }) ctx := context.Background() From 0f2834682b9b7c1e24ee999b5776a4ac95b3b6db Mon Sep 17 00:00:00 2001 From: Yuneui Date: Wed, 6 Mar 2024 00:57:18 +0900 Subject: [PATCH 4/5] Cover more Signed-off-by: Yuneui --- internal/gatewayapi/runner/runner_test.go | 171 ++++++++++++++++------ 1 file changed, 129 insertions(+), 42 deletions(-) diff --git a/internal/gatewayapi/runner/runner_test.go b/internal/gatewayapi/runner/runner_test.go index 121b7bacf95..8803f3c273d 100644 --- a/internal/gatewayapi/runner/runner_test.go +++ b/internal/gatewayapi/runner/runner_test.go @@ -14,7 +14,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/types" - v1 "sigs.k8s.io/gateway-api/apis/v1" + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" + gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/envoygateway/config" @@ -107,7 +108,7 @@ func TestGetIRKeysToDelete(t *testing.T) { } } -func TestDeletableStatus(t *testing.T) { +func TestDeleteStatusKeys(t *testing.T) { // Setup pResources := new(message.ProviderResources) xdsIR := new(message.XdsIR) @@ -127,10 +128,6 @@ func TestDeletableStatus(t *testing.T) { err = r.Start(ctx) require.NoError(t, err) - // No status is set - ds1 := r.getDeletableStatus() - require.Empty(t, len(ds1.GatewayStatusKeys)) - // A new status gets stored keys := []types.NamespacedName{ { @@ -145,50 +142,140 @@ func TestDeletableStatus(t *testing.T) { Name: "test3", Namespace: "test-namespace", }, + { + Name: "test4", + Namespace: "test-namespace", + }, + { + Name: "test5", + Namespace: "test-namespace", + }, + { + Name: "test6", + Namespace: "test-namespace", + }, + { + Name: "test7", + Namespace: "test-namespace", + }, } - statuses := []v1.GatewayStatus{ + + r.ProviderResources.GatewayStatuses.Store(keys[0], &gwapiv1.GatewayStatus{}) + r.ProviderResources.HTTPRouteStatuses.Store(keys[1], &gwapiv1.HTTPRouteStatus{}) + r.ProviderResources.GRPCRouteStatuses.Store(keys[2], &gwapiv1a2.GRPCRouteStatus{}) + r.ProviderResources.TLSRouteStatuses.Store(keys[3], &gwapiv1a2.TLSRouteStatus{}) + r.ProviderResources.TCPRouteStatuses.Store(keys[4], &gwapiv1a2.TCPRouteStatus{}) + r.ProviderResources.UDPRouteStatuses.Store(keys[5], &gwapiv1a2.UDPRouteStatus{}) + r.ProviderResources.UDPRouteStatuses.Store(keys[6], &gwapiv1a2.UDPRouteStatus{}) + + // Checks that the keys are successfully stored to DeletableStatus and watchable maps + ds := r.getDeletableStatus() + + require.True(t, ds.GatewayStatusKeys[keys[0]]) + require.True(t, ds.HTTPRouteStatusKeys[keys[1]]) + require.True(t, ds.GRPCRouteStatusKeys[keys[2]]) + require.True(t, ds.TLSRouteStatusKeys[keys[3]]) + require.True(t, ds.TCPRouteStatusKeys[keys[4]]) + require.True(t, ds.UDPRouteStatusKeys[keys[5]]) + require.True(t, ds.UDPRouteStatusKeys[keys[6]]) + + require.Equal(t, 1, r.ProviderResources.GatewayStatuses.Len()) + require.Equal(t, 1, r.ProviderResources.HTTPRouteStatuses.Len()) + require.Equal(t, 1, r.ProviderResources.GRPCRouteStatuses.Len()) + require.Equal(t, 1, r.ProviderResources.TLSRouteStatuses.Len()) + require.Equal(t, 1, r.ProviderResources.TCPRouteStatuses.Len()) + require.Equal(t, 2, r.ProviderResources.UDPRouteStatuses.Len()) + + // Delete all keys except the last UDPRouteStatus key + delete(ds.UDPRouteStatusKeys, keys[6]) + r.deleteStatusKeys(ds) + + require.Equal(t, 0, r.ProviderResources.GatewayStatuses.Len()) + require.Equal(t, 0, r.ProviderResources.HTTPRouteStatuses.Len()) + require.Equal(t, 0, r.ProviderResources.GRPCRouteStatuses.Len()) + require.Equal(t, 0, r.ProviderResources.TLSRouteStatuses.Len()) + require.Equal(t, 0, r.ProviderResources.TCPRouteStatuses.Len()) + require.Equal(t, 1, r.ProviderResources.UDPRouteStatuses.Len()) +} + +func TestDeleteAllStatusKeys(t *testing.T) { + // Setup + pResources := new(message.ProviderResources) + xdsIR := new(message.XdsIR) + infraIR := new(message.InfraIR) + cfg, err := config.New() + require.NoError(t, err) + r := New(&Config{ + Server: *cfg, + ProviderResources: pResources, + XdsIR: xdsIR, + InfraIR: infraIR, + ExtensionManager: testutils.NewManager(egv1a1.ExtensionManager{}), + }) + ctx := context.Background() + + // Start + err = r.Start(ctx) + require.NoError(t, err) + + // A new status gets stored + keys := []types.NamespacedName{ { - Addresses: []v1.GatewayStatusAddress{ - { - Value: "192.168.0.1", - }, - }, + Name: "test1", + Namespace: "test-namespace", }, { - Addresses: []v1.GatewayStatusAddress{ - { - Value: "192.168.0.2", - }, - }, + Name: "test2", + Namespace: "test-namespace", }, { - Addresses: []v1.GatewayStatusAddress{ - { - Value: "192.168.0.3", - }, - }, + Name: "test3", + Namespace: "test-namespace", + }, + { + Name: "test4", + Namespace: "test-namespace", + }, + { + Name: "test5", + Namespace: "test-namespace", + }, + { + Name: "test6", + Namespace: "test-namespace", }, } - r.ProviderResources.GatewayStatuses.Store(keys[0], &statuses[0]) - r.ProviderResources.GatewayStatuses.Store(keys[1], &statuses[1]) - r.ProviderResources.GatewayStatuses.Store(keys[2], &statuses[2]) - - // getDeletableStatus: Checks that the keys are successfully stored - ds2 := r.getDeletableStatus() - require.True(t, ds2.GatewayStatusKeys[keys[0]]) - require.True(t, ds2.GatewayStatusKeys[keys[1]]) - require.True(t, ds2.GatewayStatusKeys[keys[2]]) - require.Equal(t, 3, r.ProviderResources.GatewayStatuses.Len()) - - // deleteStatusKeys: Delete the third key by removing the first - // and second key from deletables - delete(ds2.GatewayStatusKeys, keys[0]) - delete(ds2.GatewayStatusKeys, keys[1]) - r.deleteStatusKeys(ds2) - require.Empty(t, len(ds2.GatewayStatusKeys)) - require.Equal(t, 2, r.ProviderResources.GatewayStatuses.Len()) - - // deleteAllStatusKeys: Delete remaining keys + + r.ProviderResources.GatewayStatuses.Store(keys[0], &gwapiv1.GatewayStatus{}) + r.ProviderResources.HTTPRouteStatuses.Store(keys[1], &gwapiv1.HTTPRouteStatus{}) + r.ProviderResources.GRPCRouteStatuses.Store(keys[2], &gwapiv1a2.GRPCRouteStatus{}) + r.ProviderResources.TLSRouteStatuses.Store(keys[3], &gwapiv1a2.TLSRouteStatus{}) + r.ProviderResources.TCPRouteStatuses.Store(keys[4], &gwapiv1a2.TCPRouteStatus{}) + r.ProviderResources.UDPRouteStatuses.Store(keys[5], &gwapiv1a2.UDPRouteStatus{}) + + // Checks that the keys are successfully stored to DeletableStatus and watchable maps + ds := r.getDeletableStatus() + + require.True(t, ds.GatewayStatusKeys[keys[0]]) + require.True(t, ds.HTTPRouteStatusKeys[keys[1]]) + require.True(t, ds.GRPCRouteStatusKeys[keys[2]]) + require.True(t, ds.TLSRouteStatusKeys[keys[3]]) + require.True(t, ds.TCPRouteStatusKeys[keys[4]]) + require.True(t, ds.UDPRouteStatusKeys[keys[5]]) + + require.Equal(t, 1, r.ProviderResources.GatewayStatuses.Len()) + require.Equal(t, 1, r.ProviderResources.HTTPRouteStatuses.Len()) + require.Equal(t, 1, r.ProviderResources.GRPCRouteStatuses.Len()) + require.Equal(t, 1, r.ProviderResources.TLSRouteStatuses.Len()) + require.Equal(t, 1, r.ProviderResources.TCPRouteStatuses.Len()) + require.Equal(t, 1, r.ProviderResources.UDPRouteStatuses.Len()) + + // Delete all keys r.deleteAllStatusKeys() require.Equal(t, 0, r.ProviderResources.GatewayStatuses.Len()) + require.Equal(t, 0, r.ProviderResources.HTTPRouteStatuses.Len()) + require.Equal(t, 0, r.ProviderResources.GRPCRouteStatuses.Len()) + require.Equal(t, 0, r.ProviderResources.TLSRouteStatuses.Len()) + require.Equal(t, 0, r.ProviderResources.TCPRouteStatuses.Len()) + require.Equal(t, 0, r.ProviderResources.UDPRouteStatuses.Len()) } From 3ccace3ed838c1f36289de3f373f59e3c7b062b0 Mon Sep 17 00:00:00 2001 From: Yuneui Jeong Date: Wed, 6 Mar 2024 11:55:02 +0900 Subject: [PATCH 5/5] Change struct's name and other minor fixes Signed-off-by: Yuneui Jeong --- internal/gatewayapi/runner/runner.go | 74 ++++++++++++----------- internal/gatewayapi/runner/runner_test.go | 4 +- internal/xds/translator/runner/runner.go | 12 ++-- 3 files changed, 47 insertions(+), 43 deletions(-) diff --git a/internal/gatewayapi/runner/runner.go b/internal/gatewayapi/runner/runner.go index 554c002f0aa..2b34b8ad33f 100644 --- a/internal/gatewayapi/runner/runner.go +++ b/internal/gatewayapi/runner/runner.go @@ -69,8 +69,10 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { curIRKeys = append(curIRKeys, key) } - // Get the current DeletableStatus which manages status keys to be deleted - deletableStatus := r.getDeletableStatus() + // Get all status keys from watchable and save them in this StatusesToDelete structure. + // Iterating through the controller resources, any valid keys will be removed from statusesToDelete. + // Remaining keys will be deleted from watchable before we exit this function. + statusesToDelete := r.getAllStatuses() for _, resources := range *val { // Translate and publish IRs. @@ -121,62 +123,62 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { gateway := gateway key := utils.NamespacedName(gateway) r.ProviderResources.GatewayStatuses.Store(key, &gateway.Status) - delete(deletableStatus.GatewayStatusKeys, key) + delete(statusesToDelete.GatewayStatusKeys, key) } for _, httpRoute := range result.HTTPRoutes { httpRoute := httpRoute key := utils.NamespacedName(httpRoute) r.ProviderResources.HTTPRouteStatuses.Store(key, &httpRoute.Status) - delete(deletableStatus.HTTPRouteStatusKeys, key) + delete(statusesToDelete.HTTPRouteStatusKeys, key) } for _, grpcRoute := range result.GRPCRoutes { grpcRoute := grpcRoute key := utils.NamespacedName(grpcRoute) r.ProviderResources.GRPCRouteStatuses.Store(key, &grpcRoute.Status) - delete(deletableStatus.GRPCRouteStatusKeys, key) + delete(statusesToDelete.GRPCRouteStatusKeys, key) } - for _, tlsRoute := range result.TLSRoutes { tlsRoute := tlsRoute key := utils.NamespacedName(tlsRoute) r.ProviderResources.TLSRouteStatuses.Store(key, &tlsRoute.Status) - delete(deletableStatus.TLSRouteStatusKeys, key) + delete(statusesToDelete.TLSRouteStatusKeys, key) } for _, tcpRoute := range result.TCPRoutes { tcpRoute := tcpRoute key := utils.NamespacedName(tcpRoute) r.ProviderResources.TCPRouteStatuses.Store(key, &tcpRoute.Status) - delete(deletableStatus.TCPRouteStatusKeys, key) + delete(statusesToDelete.TCPRouteStatusKeys, key) } for _, udpRoute := range result.UDPRoutes { udpRoute := udpRoute key := utils.NamespacedName(udpRoute) r.ProviderResources.UDPRouteStatuses.Store(key, &udpRoute.Status) - delete(deletableStatus.UDPRouteStatusKeys, key) + delete(statusesToDelete.UDPRouteStatusKeys, key) + } + for _, backendTLSPolicy := range result.BackendTLSPolicies { + backendTLSPolicy := backendTLSPolicy + key := utils.NamespacedName(backendTLSPolicy) + r.ProviderResources.BackendTLSPolicyStatuses.Store(key, &backendTLSPolicy.Status) + delete(statusesToDelete.BackendTLSPolicyStatusKeys, key) } + for _, clientTrafficPolicy := range result.ClientTrafficPolicies { clientTrafficPolicy := clientTrafficPolicy key := utils.NamespacedName(clientTrafficPolicy) r.ProviderResources.ClientTrafficPolicyStatuses.Store(key, &clientTrafficPolicy.Status) - delete(deletableStatus.ClientTrafficPolicyStatusKeys, key) + delete(statusesToDelete.ClientTrafficPolicyStatusKeys, key) } for _, backendTrafficPolicy := range result.BackendTrafficPolicies { backendTrafficPolicy := backendTrafficPolicy key := utils.NamespacedName(backendTrafficPolicy) r.ProviderResources.BackendTrafficPolicyStatuses.Store(key, &backendTrafficPolicy.Status) - delete(deletableStatus.BackendTrafficPolicyStatusKeys, key) + delete(statusesToDelete.BackendTrafficPolicyStatusKeys, key) } for _, securityPolicy := range result.SecurityPolicies { securityPolicy := securityPolicy key := utils.NamespacedName(securityPolicy) r.ProviderResources.SecurityPolicyStatuses.Store(key, &securityPolicy.Status) - delete(deletableStatus.SecurityPolicyStatusKeys, key) - } - for _, backendTLSPolicy := range result.BackendTLSPolicies { - backendTLSPolicy := backendTLSPolicy - key := utils.NamespacedName(backendTLSPolicy) - r.ProviderResources.BackendTLSPolicyStatuses.Store(key, &backendTLSPolicy.Status) - delete(deletableStatus.BackendTLSPolicyStatusKeys, key) + delete(statusesToDelete.SecurityPolicyStatusKeys, key) } } @@ -189,7 +191,7 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { } // Delete status keys - r.deleteStatusKeys(deletableStatus) + r.deleteStatusKeys(statusesToDelete) }, ) r.Logger.Info("shutting down") @@ -203,23 +205,23 @@ func (r *Runner) deleteAllIRKeys() { } } -type DeletableStatus struct { - GatewayStatusKeys map[types.NamespacedName]bool - HTTPRouteStatusKeys map[types.NamespacedName]bool - GRPCRouteStatusKeys map[types.NamespacedName]bool - TLSRouteStatusKeys map[types.NamespacedName]bool - TCPRouteStatusKeys map[types.NamespacedName]bool - UDPRouteStatusKeys map[types.NamespacedName]bool +type StatusesToDelete struct { + GatewayStatusKeys map[types.NamespacedName]bool + HTTPRouteStatusKeys map[types.NamespacedName]bool + GRPCRouteStatusKeys map[types.NamespacedName]bool + TLSRouteStatusKeys map[types.NamespacedName]bool + TCPRouteStatusKeys map[types.NamespacedName]bool + UDPRouteStatusKeys map[types.NamespacedName]bool + BackendTLSPolicyStatusKeys map[types.NamespacedName]bool ClientTrafficPolicyStatusKeys map[types.NamespacedName]bool BackendTrafficPolicyStatusKeys map[types.NamespacedName]bool SecurityPolicyStatusKeys map[types.NamespacedName]bool - BackendTLSPolicyStatusKeys map[types.NamespacedName]bool } -func (r *Runner) getDeletableStatus() *DeletableStatus { +func (r *Runner) getAllStatuses() *StatusesToDelete { // Maps storing status keys to be deleted - ds := &DeletableStatus{ + ds := &StatusesToDelete{ GatewayStatusKeys: make(map[types.NamespacedName]bool), HTTPRouteStatusKeys: make(map[types.NamespacedName]bool), GRPCRouteStatusKeys: make(map[types.NamespacedName]bool), @@ -252,6 +254,9 @@ func (r *Runner) getDeletableStatus() *DeletableStatus { for key := range r.ProviderResources.UDPRouteStatuses.LoadAll() { ds.UDPRouteStatusKeys[key] = true } + for key := range r.ProviderResources.BackendTLSPolicyStatuses.LoadAll() { + ds.BackendTLSPolicyStatusKeys[key] = true + } for key := range r.ProviderResources.ClientTrafficPolicyStatuses.LoadAll() { ds.ClientTrafficPolicyStatusKeys[key] = true @@ -262,14 +267,11 @@ func (r *Runner) getDeletableStatus() *DeletableStatus { for key := range r.ProviderResources.SecurityPolicyStatuses.LoadAll() { ds.SecurityPolicyStatusKeys[key] = true } - for key := range r.ProviderResources.BackendTLSPolicyStatuses.LoadAll() { - ds.BackendTLSPolicyStatusKeys[key] = true - } return ds } -func (r *Runner) deleteStatusKeys(ds *DeletableStatus) { +func (r *Runner) deleteStatusKeys(ds *StatusesToDelete) { for key := range ds.GatewayStatusKeys { r.ProviderResources.GatewayStatuses.Delete(key) delete(ds.GatewayStatusKeys, key) @@ -334,6 +336,9 @@ func (r *Runner) deleteAllStatusKeys() { for key := range r.ProviderResources.UDPRouteStatuses.LoadAll() { r.ProviderResources.UDPRouteStatuses.Delete(key) } + for key := range r.ProviderResources.BackendTLSPolicyStatuses.LoadAll() { + r.ProviderResources.BackendTLSPolicyStatuses.Delete(key) + } // Fields of PolicyStatuses for key := range r.ProviderResources.ClientTrafficPolicyStatuses.LoadAll() { @@ -345,9 +350,6 @@ func (r *Runner) deleteAllStatusKeys() { for key := range r.ProviderResources.SecurityPolicyStatuses.LoadAll() { r.ProviderResources.SecurityPolicyStatuses.Delete(key) } - for key := range r.ProviderResources.BackendTLSPolicyStatuses.LoadAll() { - r.ProviderResources.BackendTLSPolicyStatuses.Delete(key) - } } // getIRKeysToDelete returns the list of IR keys to delete diff --git a/internal/gatewayapi/runner/runner_test.go b/internal/gatewayapi/runner/runner_test.go index 8803f3c273d..772f0372a3c 100644 --- a/internal/gatewayapi/runner/runner_test.go +++ b/internal/gatewayapi/runner/runner_test.go @@ -169,7 +169,7 @@ func TestDeleteStatusKeys(t *testing.T) { r.ProviderResources.UDPRouteStatuses.Store(keys[6], &gwapiv1a2.UDPRouteStatus{}) // Checks that the keys are successfully stored to DeletableStatus and watchable maps - ds := r.getDeletableStatus() + ds := r.getAllStatuses() require.True(t, ds.GatewayStatusKeys[keys[0]]) require.True(t, ds.HTTPRouteStatusKeys[keys[1]]) @@ -254,7 +254,7 @@ func TestDeleteAllStatusKeys(t *testing.T) { r.ProviderResources.UDPRouteStatuses.Store(keys[5], &gwapiv1a2.UDPRouteStatus{}) // Checks that the keys are successfully stored to DeletableStatus and watchable maps - ds := r.getDeletableStatus() + ds := r.getAllStatuses() require.True(t, ds.GatewayStatusKeys[keys[0]]) require.True(t, ds.HTTPRouteStatusKeys[keys[1]]) diff --git a/internal/xds/translator/runner/runner.go b/internal/xds/translator/runner/runner.go index 0d7a0e32752..114b00b4550 100644 --- a/internal/xds/translator/runner/runner.go +++ b/internal/xds/translator/runner/runner.go @@ -90,10 +90,12 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { return } - // Get current status keys and mark them as deletable temporarily - deletableStatus := make(map[ktypes.NamespacedName]bool) + // Get all status keys from watchable and save them in the map statusesToDelete. + // Iterating through result.EnvoyPatchPolicyStatuses, any valid keys will be removed from statusesToDelete. + // Remaining keys will be deleted from watchable before we exit this function. + statusesToDelete := make(map[ktypes.NamespacedName]bool) for key := range r.ProviderResources.EnvoyPatchPolicyStatuses.LoadAll() { - deletableStatus[key] = true + statusesToDelete[key] = true } // Publish EnvoyPatchPolicyStatus @@ -103,7 +105,7 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { Namespace: e.Namespace, } r.ProviderResources.EnvoyPatchPolicyStatuses.Store(key, e.Status) - delete(deletableStatus, key) + delete(statusesToDelete, key) } // Discard the EnvoyPatchPolicyStatuses to reduce memory footprint result.EnvoyPatchPolicyStatuses = nil @@ -112,7 +114,7 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { r.Xds.Store(key, result) // Delete all the deletable status keys - for key := range deletableStatus { + for key := range statusesToDelete { r.ProviderResources.EnvoyPatchPolicyStatuses.Delete(key) } }