Skip to content

Commit a18a0a3

Browse files
aa1exaa1ex
authored
memory optimization (#203)
1 parent 88b0b6c commit a18a0a3

File tree

4 files changed

+174
-80
lines changed

4 files changed

+174
-80
lines changed

‎internal/store/optimized.go‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ type OptimizedStore struct {
5555

5656
// Additional indices
5757
specClusters map[string]*v1alpha1.Cluster
58-
domainSecrets map[string]corev1.Secret
58+
domainSecrets map[string]*corev1.Secret
5959
nodeDomains map[string]map[string]struct{}
6060
}
6161

@@ -100,7 +100,7 @@ func NewOptimizedStore() Store {
100100

101101
// Additional indices
102102
specClusters: make(map[string]*v1alpha1.Cluster, 500),
103-
domainSecrets: make(map[string]corev1.Secret, 200),
103+
domainSecrets: make(map[string]*corev1.Secret, 200),
104104
nodeDomains: make(map[string]map[string]struct{}, 100),
105105
}
106106
}
@@ -257,7 +257,7 @@ func (s *OptimizedStore) Copy() Store {
257257

258258
// Additional indices
259259
specClusters: make(map[string]*v1alpha1.Cluster, len(s.specClusters)),
260-
domainSecrets: make(map[string]corev1.Secret, len(s.domainSecrets)),
260+
domainSecrets: make(map[string]*corev1.Secret, len(s.domainSecrets)),
261261
nodeDomains: make(map[string]map[string]struct{}, len(s.nodeDomains)),
262262
}
263263

@@ -1247,8 +1247,8 @@ func (s *OptimizedStore) GetAccessLogByUID(uid string) *v1alpha1.AccessLogConfig
12471247
}
12481248

12491249
// GetDomainSecret returns the secret for a given domain
1250-
// Returns zero value if not found (check with secret.Name != "")
1251-
func (s *OptimizedStore) GetDomainSecret(domain string) corev1.Secret {
1250+
// Returns nil if not found
1251+
func (s *OptimizedStore) GetDomainSecret(domain string) *corev1.Secret {
12521252
s.mu.RLock()
12531253
defer s.mu.RUnlock()
12541254
return s.domainSecrets[domain]
@@ -1293,8 +1293,7 @@ func (s *OptimizedStore) MapDomainSecrets() map[string]*corev1.Secret {
12931293

12941294
result := make(map[string]*corev1.Secret, len(s.domainSecrets))
12951295
for k, v := range s.domainSecrets {
1296-
secret := v // Create explicit copy for pointer safety
1297-
result[k] = &secret
1296+
result[k] = v // v is already a pointer now
12981297
}
12991298
return result
13001299
}
@@ -1374,7 +1373,7 @@ func (s *OptimizedStore) addDomainSecretsForSecret(secret *corev1.Secret) {
13741373
// TODO: domain already exists in another secret! Need to create error case
13751374
continue
13761375
}
1377-
s.domainSecrets[domain] = *secret
1376+
s.domainSecrets[domain] = secret
13781377
}
13791378
}
13801379

@@ -1400,7 +1399,7 @@ func (s *OptimizedStore) removeDomainSecretsForSecret(secret *corev1.Secret) {
14001399
// updateDomainSecretsMap rebuilds the entire domain to secret mapping from all secrets
14011400
// This is used only during FillFromKubernetes for initial population
14021401
func (s *OptimizedStore) updateDomainSecretsMap() {
1403-
m := make(map[string]corev1.Secret)
1402+
m := make(map[string]*corev1.Secret)
14041403

14051404
for _, secret := range s.secrets {
14061405
if secret.Annotations == nil {
@@ -1420,7 +1419,7 @@ func (s *OptimizedStore) updateDomainSecretsMap() {
14201419
// TODO: domain already exists in another secret! Need to create error case
14211420
continue
14221421
}
1423-
m[domain] = *secret
1422+
m[domain] = secret
14241423
}
14251424
}
14261425
s.domainSecrets = m

‎internal/store/secret_test.go‎

Lines changed: 23 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -78,44 +78,32 @@ func TestMapDomainSecrets_ReturnsPointers(t *testing.T) {
7878
}
7979
}
8080

81-
// TestMapDomainSecrets_ImmutableCopy verifies that returned map is a copy
82-
func TestMapDomainSecrets_ImmutableCopy(t *testing.T) {
83-
tests := []struct {
84-
name string
85-
store Store
86-
}{
87-
{"OptimizedStore", NewOptimizedStore()},
88-
{"LegacyStore", &StoreAdapter{LegacyStore: New()}},
81+
// TestMapDomainSecrets_OptimizedStore_SamePointers verifies OptimizedStore returns same pointers for performance
82+
func TestMapDomainSecrets_OptimizedStore_SamePointers(t *testing.T) {
83+
store := NewOptimizedStore()
84+
85+
secret := &corev1.Secret{
86+
ObjectMeta: metav1.ObjectMeta{
87+
Name: "secret",
88+
Namespace: "default",
89+
Annotations: map[string]string{
90+
"envoy.kaasops.io/domains": "test.com",
91+
},
92+
},
93+
Data: map[string][]byte{"key": []byte("value")},
8994
}
9095

91-
for _, tt := range tests {
92-
t.Run(tt.name, func(t *testing.T) {
93-
secret := &corev1.Secret{
94-
ObjectMeta: metav1.ObjectMeta{
95-
Name: "secret",
96-
Namespace: "default",
97-
Annotations: map[string]string{
98-
"envoy.kaasops.io/domains": "test.com",
99-
},
100-
},
101-
Data: map[string][]byte{"key": []byte("value")},
102-
}
103-
104-
tt.store.SetSecret(secret)
105-
106-
// Get first copy
107-
map1 := tt.store.MapDomainSecrets()
96+
store.SetSecret(secret)
10897

109-
// Get second copy
110-
map2 := tt.store.MapDomainSecrets()
98+
// Get two maps
99+
map1 := store.MapDomainSecrets()
100+
map2 := store.MapDomainSecrets()
111101

112-
// Verify both have the data
113-
assert.Len(t, map1, 1)
114-
assert.Len(t, map2, 1)
102+
// Verify both have the data
103+
assert.Len(t, map1, 1)
104+
assert.Len(t, map2, 1)
115105

116-
// Pointers should be different (new allocation each time)
117-
assert.NotSame(t, map1["test.com"], map2["test.com"],
118-
"each call should return new pointers to prevent accidental mutation")
119-
})
120-
}
106+
// OptimizedStore returns same pointer for performance (secrets are immutable in store)
107+
assert.Same(t, map1["test.com"], map2["test.com"],
108+
"OptimizedStore should return same secret pointer for performance")
121109
}

‎internal/webhook/v1alpha1/virtualservicetemplate_webhook.go‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"encoding/json"
2222
"errors"
2323
"fmt"
24+
"math/rand"
2425
"regexp"
2526
"strings"
2627
"time"
@@ -57,6 +58,15 @@ func SetupVirtualServiceTemplateWebhookWithManager(mgr ctrl.Manager, cacheUpdate
5758
// Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
5859
// +kubebuilder:webhook:path=/validate-envoy-kaasops-io-v1alpha1-virtualservicetemplate,mutating=false,failurePolicy=fail,sideEffects=None,groups=envoy.kaasops.io,resources=virtualservicetemplates,verbs=create;update;delete,versions=v1alpha1,name=vvirtualservicetemplate-v1alpha1.envoy.kaasops.io,admissionReviewVersions=v1
5960

61+
type contextKey string
62+
63+
const validationIDKey contextKey = "validationID"
64+
65+
// generateValidationID creates a short unique ID for correlating logs
66+
func generateValidationID() string {
67+
return fmt.Sprintf("%08x", rand.Uint32())
68+
}
69+
6070
// VirtualServiceTemplateCustomValidator struct is responsible for validating the VirtualServiceTemplate resource
6171
// when it is created, updated, or deleted.
6272
//
@@ -175,11 +185,16 @@ func (v *VirtualServiceTemplateCustomValidator) ValidateUpdate(ctx context.Conte
175185
if !ok {
176186
return nil, fmt.Errorf("expected a VirtualServiceTemplate object but got %T", newObj)
177187
}
178-
virtualservicetemplatelog.Info("Validation for VirtualServiceTemplate upon update", "name", virtualservicetemplate.GetName())
188+
189+
// Generate correlation ID for linking logs
190+
validationID := generateValidationID()
191+
ctx = context.WithValue(ctx, validationIDKey, validationID)
192+
193+
virtualservicetemplatelog.Info("Validation for VirtualServiceTemplate upon update", "name", virtualservicetemplate.GetName(), "validationID", validationID)
179194

180195
// Validate that all extraFields are used in the template
181196
if err := validateExtraFieldsUsage(virtualservicetemplate); err != nil {
182-
virtualservicetemplatelog.Error(err, "ExtraFields validation failed", "name", virtualservicetemplate.GetName())
197+
virtualservicetemplatelog.Error(err, "ExtraFields validation failed", "name", virtualservicetemplate.GetName(), "validationID", validationID)
183198
return nil, err
184199
}
185200

@@ -188,27 +203,27 @@ func (v *VirtualServiceTemplateCustomValidator) ValidateUpdate(ctx context.Conte
188203
defer cancelTracing()
189204
if err := validateTemplateTracing(ctxTracing, v.Client, virtualservicetemplate); err != nil {
190205
if errors.Is(ctxTracing.Err(), context.DeadlineExceeded) {
191-
virtualservicetemplatelog.Error(err, "Tracing validation timed out", "name", virtualservicetemplate.GetName(), "timeout", v.getDryRunTimeout())
206+
virtualservicetemplatelog.Error(err, "Tracing validation timed out", "name", virtualservicetemplate.GetName(), "timeout", v.getDryRunTimeout(), "validationID", validationID)
192207
return nil, fmt.Errorf("tracing validation timed out after %s; please check Kubernetes API availability", v.getDryRunTimeout())
193208
}
194-
virtualservicetemplatelog.Error(err, "Tracing validation failed", "name", virtualservicetemplate.GetName())
209+
virtualservicetemplatelog.Error(err, "Tracing validation failed", "name", virtualservicetemplate.GetName(), "validationID", validationID)
195210
return nil, err
196211
}
197212

198213
// Apply timeout for heavy dry-run path when updating template
199-
virtualservicetemplatelog.Info("Starting dry-run validation", "name", virtualservicetemplate.GetName(), "timeout", v.getDryRunTimeout())
214+
virtualservicetemplatelog.Info("Starting dry-run validation", "name", virtualservicetemplate.GetName(), "timeout", v.getDryRunTimeout(), "validationID", validationID)
200215
ctxTO, cancel := context.WithTimeout(ctx, v.getDryRunTimeout())
201216
defer cancel()
202217
if err := v.cacheUpdater.DryBuildSnapshotsWithVirtualServiceTemplate(ctxTO, virtualservicetemplate); err != nil {
203218
if errors.Is(ctxTO.Err(), context.DeadlineExceeded) {
204-
virtualservicetemplatelog.Error(err, "Dry-run validation timed out", "name", virtualservicetemplate.GetName(), "timeout", v.getDryRunTimeout())
219+
virtualservicetemplatelog.Error(err, "Dry-run validation timed out", "name", virtualservicetemplate.GetName(), "timeout", v.getDryRunTimeout(), "validationID", validationID)
205220
return nil, fmt.Errorf("validation timed out after %s; please retry or increase WEBHOOK_DRYRUN_TIMEOUT_MS", v.getDryRunTimeout())
206221
}
207-
virtualservicetemplatelog.Error(err, "Dry-run validation failed", "name", virtualservicetemplate.GetName())
222+
virtualservicetemplatelog.Error(err, "Dry-run validation failed", "name", virtualservicetemplate.GetName(), "validationID", validationID)
208223
return nil, fmt.Errorf("failed to apply VirtualServiceTemplate: %w", err)
209224
}
210225

211-
virtualservicetemplatelog.Info("VirtualServiceTemplate validation completed", "name", virtualservicetemplate.GetName())
226+
virtualservicetemplatelog.Info("VirtualServiceTemplate validation completed", "name", virtualservicetemplate.GetName(), "validationID", validationID)
212227

213228
return nil, nil
214229
}

0 commit comments

Comments
 (0)