Skip to content

Commit 6163d52

Browse files
weilerNweilerN
committed
Compare ingress.resourceVersion to avoid redundant updates
1 parent 4642fbc commit 6163d52

File tree

2 files changed

+71
-13
lines changed

2 files changed

+71
-13
lines changed

‎pkg/kube/ingress.go‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ type ingressValue struct {
3939
name string
4040
host string
4141
path string
42+
version string
4243
targets []string
4344
}
4445

@@ -148,6 +149,29 @@ func (iw *IngressWatcher) AddHandler(obj interface{}) {
148149
}
149150

150151
func (iw *IngressWatcher) UpdateHandler(oldObj, newObj interface{}) {
152+
oldIngressResource, ok := oldObj.(*networkingv1.Ingress)
153+
if !ok {
154+
iw.logger.DebugWith("Failed to cast old object to Ingress",
155+
"object", oldObj)
156+
return
157+
}
158+
159+
newIngressResource, ok := newObj.(*networkingv1.Ingress)
160+
if !ok {
161+
iw.logger.DebugWith("Failed to cast new object to Ingress",
162+
"object", newObj)
163+
return
164+
}
165+
166+
// ResourceVersion is managed by Kubernetes and indicates whether the resource has changed.
167+
// Comparing resourceVersion helps avoid unnecessary updates triggered by periodic informer resync
168+
if oldIngressResource.ResourceVersion == newIngressResource.ResourceVersion {
169+
iw.logger.DebugWith("No changes in resource, skipping",
170+
"resourceVersion", oldIngressResource.ResourceVersion,
171+
"ingressName", oldIngressResource.Name)
172+
return
173+
}
174+
151175
oldIngress, err := iw.extractValuesFromIngressResource(oldObj)
152176
if err != nil {
153177
iw.logger.DebugWith("Update ingress handler - failed to extract values from old object",
@@ -230,7 +254,7 @@ func (iw *IngressWatcher) extractValuesFromIngressResource(obj interface{}) (*in
230254

231255
targets, err := iw.resolveTargetsCallback(ingress)
232256
if err != nil {
233-
return nil, errors.Wrap(err, "Failed to extract targets from ingress labels")
257+
return nil, errors.Wrapf(err, "Failed to extract targets from ingress labels: %s", err.Error())
234258
}
235259

236260
if len(targets) == 0 {

‎pkg/kube/ingress_test.go‎

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"testing"
2727

2828
"github.com/v3io/scaler/pkg/ingresscache"
29+
"github.com/v3io/scaler/pkg/scalertypes"
2930

3031
"github.com/nuclio/logger"
3132
nucliozap "github.com/nuclio/zap"
@@ -116,7 +117,7 @@ func (suite *IngressWatcherTestSuite) TestAddHandler() {
116117
testIngressWatcher, err := suite.createTestIngressWatcher()
117118
suite.Require().NoError(err)
118119

119-
testObj = suite.createDummyIngress(testCase.testArgs.host, testCase.testArgs.path, testCase.testArgs.targets)
120+
testObj = suite.createDummyIngress(testCase.testArgs.host, testCase.testArgs.path, testCase.testArgs.version, testCase.testArgs.targets)
120121

121122
if testCase.expectError {
122123
testObj = &networkingv1.IngressSpec{}
@@ -162,11 +163,39 @@ func (suite *IngressWatcherTestSuite) TestUpdateHandler() {
162163
testOldObj: ingressValue{
163164
host: "www.example.com",
164165
path: "/test/path",
166+
version: "1",
165167
targets: []string{"test-targets-name-1", "test-targets-name-2"},
166168
},
167169
testNewObj: ingressValue{
168170
host: "www.example.com",
169171
path: "/test/path",
172+
version: "2",
173+
targets: []string{"test-targets-name-1", "test-targets-name-3"},
174+
},
175+
initialCachedData: &ingressValue{
176+
host: "www.example.com",
177+
path: "/test/path",
178+
targets: []string{"test-targets-name-1", "test-targets-name-2"},
179+
},
180+
}, {
181+
name: "Update PairTarget - same ResourceVersion, no change in targets",
182+
expectedResults: []expectedResult{
183+
{
184+
host: "www.example.com",
185+
path: "/test/path",
186+
targets: []string{"test-targets-name-1", "test-targets-name-2"},
187+
},
188+
},
189+
testOldObj: ingressValue{
190+
host: "www.example.com",
191+
path: "/test/path",
192+
version: "1",
193+
targets: []string{"test-targets-name-1", "test-targets-name-2"},
194+
},
195+
testNewObj: ingressValue{
196+
host: "www.example.com",
197+
path: "/test/path",
198+
version: "1",
170199
targets: []string{"test-targets-name-1", "test-targets-name-3"},
171200
},
172201
initialCachedData: &ingressValue{
@@ -184,11 +213,13 @@ func (suite *IngressWatcherTestSuite) TestUpdateHandler() {
184213
testOldObj: ingressValue{
185214
host: "www.example.com",
186215
path: "/test/path",
216+
version: "1",
187217
targets: []string{"test-targets-name-1", "test-targets-name-2"},
188218
},
189219
testNewObj: ingressValue{
190220
host: "www.example.com",
191221
path: "/another/path",
222+
version: "2",
192223
targets: []string{"test-targets-name-1", "test-targets-name-2"},
193224
},
194225
expectedResults: []expectedResult{
@@ -213,11 +244,13 @@ func (suite *IngressWatcherTestSuite) TestUpdateHandler() {
213244
testOldObj: ingressValue{
214245
host: "www.example.com",
215246
path: "/test/path",
247+
version: "1",
216248
targets: []string{"test-targets-name-1", "test-targets-name-2"},
217249
},
218250
testNewObj: ingressValue{
219251
host: "www.google.com",
220252
path: "/test/path",
253+
version: "2",
221254
targets: []string{"test-targets-name-1", "test-targets-name-2"},
222255
},
223256
expectedResults: []expectedResult{
@@ -238,8 +271,8 @@ func (suite *IngressWatcherTestSuite) TestUpdateHandler() {
238271
testIngressWatcher, err := suite.createTestIngressWatcher()
239272
suite.Require().NoError(err)
240273

241-
testOldObj := suite.createDummyIngress(testCase.testOldObj.host, testCase.testOldObj.path, testCase.testOldObj.targets)
242-
testNewObj := suite.createDummyIngress(testCase.testNewObj.host, testCase.testNewObj.path, testCase.testNewObj.targets)
274+
testOldObj := suite.createDummyIngress(testCase.testOldObj.host, testCase.testOldObj.path, testCase.testOldObj.version, testCase.testOldObj.targets)
275+
testNewObj := suite.createDummyIngress(testCase.testNewObj.host, testCase.testNewObj.path, testCase.testNewObj.version, testCase.testNewObj.targets)
243276

244277
if testCase.initialCachedData != nil {
245278
err = testIngressWatcher.cache.Set(testCase.initialCachedData.host, testCase.initialCachedData.path, testCase.initialCachedData.targets)
@@ -337,7 +370,7 @@ func (suite *IngressWatcherTestSuite) TestDeleteHandler() {
337370
testIngressWatcher, err := suite.createTestIngressWatcher()
338371
suite.Require().NoError(err)
339372

340-
testObj = suite.createDummyIngress(testCase.testArgs.host, testCase.testArgs.path, testCase.testArgs.targets)
373+
testObj = suite.createDummyIngress(testCase.testArgs.host, testCase.testArgs.path, testCase.testArgs.version, testCase.testArgs.targets)
341374

342375
if testCase.expectError {
343376
testObj = &networkingv1.IngressSpec{}
@@ -374,7 +407,7 @@ func (suite *IngressWatcherTestSuite) TestGetPathFromIngress() {
374407
tests := []testCase{
375408
{
376409
name: "Valid ingress with path",
377-
ingress: suite.createDummyIngress("host", "/test", []string{"target"}),
410+
ingress: suite.createDummyIngress("host", "/test", "1", []string{"target"}),
378411
expected: "/test",
379412
},
380413
{
@@ -463,7 +496,7 @@ func (suite *IngressWatcherTestSuite) TestGetHostFromIngress() {
463496
}{
464497
{
465498
name: "Valid ingress with host",
466-
ingress: suite.createDummyIngress("test-host", "/test", []string{"target"}),
499+
ingress: suite.createDummyIngress("test-host", "/test", "1", []string{"target"}),
467500
expected: "test-host",
468501
},
469502
{
@@ -527,15 +560,15 @@ func (suite *IngressWatcherTestSuite) createTestIngressWatcher() (*IngressWatche
527560
return NewIngressWatcher(ctx,
528561
suite.logger,
529562
suite.kubeClientSet,
530-
*ingresscache.NewIngressCache(suite.logger),
563+
ingresscache.NewIngressCache(suite.logger),
531564
suite.createMockResolveFunc(),
532-
nil,
565+
scalertypes.Duration{},
533566
"test-namespace",
534567
"test-labels-filter",
535568
)
536569
}
537570

538-
func (suite *IngressWatcherTestSuite) createMockResolveFunc() ResolveTargetsFromIngressCallback {
571+
func (suite *IngressWatcherTestSuite) createMockResolveFunc() scalertypes.ResolveTargetsFromIngressCallback {
539572
return func(ingress *networkingv1.Ingress) ([]string, error) {
540573
// Extract targets from ingress - matches createDummyIngress structure (1 rule, 1 path)
541574
if len(ingress.Spec.Rules) != 1 {
@@ -563,12 +596,13 @@ func (suite *IngressWatcherTestSuite) createMockResolveFunc() ResolveTargetsFrom
563596
}
564597

565598
// createDummyIngress Creates a dummy Ingress object for testing
566-
func (suite *IngressWatcherTestSuite) createDummyIngress(host, path string, targets []string) *networkingv1.Ingress {
599+
func (suite *IngressWatcherTestSuite) createDummyIngress(host, path, version string, targets []string) *networkingv1.Ingress {
567600
target := strings.Join(targets, ",")
568601
return &networkingv1.Ingress{
569602
ObjectMeta: metav1.ObjectMeta{
570-
Name: "test-ingress",
571-
Namespace: "test-namespace",
603+
Name: "test-ingress",
604+
Namespace: "test-namespace",
605+
ResourceVersion: version,
572606
},
573607
Spec: networkingv1.IngressSpec{
574608
Rules: []networkingv1.IngressRule{

0 commit comments

Comments
 (0)