-
Notifications
You must be signed in to change notification settings - Fork 363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Delete unused status keys from watchable #2782
Changes from 5 commits
ac0f611
a1bcbd8
d6f1242
f02709d
0f28346
3ccace3
6e0c21e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
"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 @@ | |
// 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 @@ | |
errChan <- err | ||
} else { | ||
r.InfraIR.Store(key, val) | ||
newKeys = append(newKeys, key) | ||
newIRKeys = append(newIRKeys, key) | ||
} | ||
} | ||
|
||
|
@@ -114,61 +121,75 @@ | |
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,153 @@ | |
} | ||
} | ||
|
||
type DeletableStatus struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. prefer |
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: can we move |
||
} | ||
|
||
func (r *Runner) getDeletableStatus() *DeletableStatus { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. prefer if this API was called it took me 2 readings to figure out your approach of collecting all statuses and then deleting the current ones from this truck and then deleting the remaining ones from watchable :) |
||
// 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) | ||
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) | ||
} | ||
} | ||
|
||
// 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() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets move this to L334 |
||
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. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets add a comment here explaining the logic