Skip to content

Commit 3e555b7

Browse files
videlovvidelov
committed
Do not fetch resource on first attempt (already up-to-date from Items func)
1 parent d28f646 commit 3e555b7

2 files changed

Lines changed: 135 additions & 10 deletions

File tree

‎internal/pkg/handler/upgrade.go‎

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@ import (
2222
"github.com/stakater/Reloader/internal/pkg/util"
2323
"github.com/stakater/Reloader/pkg/kube"
2424
v1 "k8s.io/api/core/v1"
25+
apierrors "k8s.io/apimachinery/pkg/api/errors"
2526
"k8s.io/apimachinery/pkg/api/meta"
2627
"k8s.io/apimachinery/pkg/runtime"
2728
patchtypes "k8s.io/apimachinery/pkg/types"
29+
"k8s.io/apimachinery/pkg/util/wait"
2830
"k8s.io/client-go/tools/record"
2931
"k8s.io/client-go/util/retry"
3032
)
@@ -216,10 +218,9 @@ func PerformAction(clients kube.Clients, config util.Config, upgradeFuncs callba
216218
items := upgradeFuncs.ItemsFunc(clients, config.Namespace)
217219

218220
for _, item := range items {
219-
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
220-
return upgradeResource(clients, config, upgradeFuncs, collectors, recorder, strategy, item)
221+
err := retryOnConflict(retry.DefaultRetry, func(shouldRefresh bool) error {
222+
return upgradeResource(clients, config, upgradeFuncs, collectors, recorder, strategy, item, shouldRefresh)
221223
})
222-
223224
if err != nil {
224225
return err
225226
}
@@ -228,16 +229,40 @@ func PerformAction(clients kube.Clients, config util.Config, upgradeFuncs callba
228229
return nil
229230
}
230231

231-
func upgradeResource(clients kube.Clients, config util.Config, upgradeFuncs callbacks.RollingUpgradeFuncs, collectors metrics.Collectors, recorder record.EventRecorder, strategy invokeStrategy, resource runtime.Object) error {
232+
func retryOnConflict(backoff wait.Backoff, fn func(_ bool) error) error {
233+
var lastError error
234+
fetchResource := false // do not fetch resource on first attempt, already done by ItemsFunc
235+
err := wait.ExponentialBackoff(backoff, func() (bool, error) {
236+
err := fn(fetchResource)
237+
fetchResource = true
238+
switch {
239+
case err == nil:
240+
return true, nil
241+
case apierrors.IsConflict(err):
242+
lastError = err
243+
return false, nil
244+
default:
245+
return false, err
246+
}
247+
})
248+
if wait.Interrupted(err) {
249+
err = lastError
250+
}
251+
return err
252+
}
253+
254+
func upgradeResource(clients kube.Clients, config util.Config, upgradeFuncs callbacks.RollingUpgradeFuncs, collectors metrics.Collectors, recorder record.EventRecorder, strategy invokeStrategy, resource runtime.Object, fetchResource bool) error {
232255
accessor, err := meta.Accessor(resource)
233256
if err != nil {
234257
return err
235258
}
236259

237260
resourceName := accessor.GetName()
238-
resource, err = upgradeFuncs.ItemFunc(clients, resourceName, config.Namespace)
239-
if err != nil {
240-
return err
261+
if fetchResource {
262+
resource, err = upgradeFuncs.ItemFunc(clients, resourceName, config.Namespace)
263+
if err != nil {
264+
return err
265+
}
241266
}
242267

243268
// find correct annotation and update the resource

‎internal/pkg/handler/upgrade_test.go‎

Lines changed: 103 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1441,6 +1441,20 @@ func TestRollingUpgradeForDeploymentWithConfigmapUsingArs(t *testing.T) {
14411441
deploymentFuncs := GetDeploymentRollingUpgradeFuncs()
14421442
collectors := getCollectors()
14431443

1444+
orgItemFunc := deploymentFuncs.ItemFunc
1445+
orgItemsFunc := deploymentFuncs.ItemsFunc
1446+
itemCalled := 0
1447+
itemsCalled := 0
1448+
1449+
deploymentFuncs.ItemFunc = func(client kube.Clients, namespace string, name string) (runtime.Object, error) {
1450+
itemCalled++
1451+
return orgItemFunc(client, namespace, name)
1452+
}
1453+
deploymentFuncs.ItemsFunc = func(client kube.Clients, namespace string) []runtime.Object {
1454+
itemsCalled++
1455+
return orgItemsFunc(client, namespace)
1456+
}
1457+
14441458
err := PerformAction(clients, config, deploymentFuncs, collectors, nil, invokeReloadStrategy)
14451459
time.Sleep(5 * time.Second)
14461460
if err != nil {
@@ -1460,6 +1474,10 @@ func TestRollingUpgradeForDeploymentWithConfigmapUsingArs(t *testing.T) {
14601474
if promtestutil.ToFloat64(collectors.ReloadedByNamespace.With(prometheus.Labels{"success": "true", "namespace": arsNamespace})) != 1 {
14611475
t.Errorf("Counter by namespace was not increased")
14621476
}
1477+
1478+
assert.Equal(t, 0, itemCalled, "ItemFunc should not be called")
1479+
assert.Equal(t, 2, itemsCalled, "ItemsFunc should be called twice")
1480+
14631481
testRollingUpgradeInvokeDeleteStrategyArs(t, clients, config, deploymentFuncs, collectors, envVarPostfix)
14641482
}
14651483

@@ -1474,6 +1492,20 @@ func TestRollingUpgradeForDeploymentWithPatchAndRetryUsingArs(t *testing.T) {
14741492
assert.True(t, deploymentFuncs.SupportsPatch)
14751493
assert.NotEmpty(t, deploymentFuncs.PatchTemplatesFunc().AnnotationTemplate)
14761494

1495+
orgItemFunc := deploymentFuncs.ItemFunc
1496+
orgItemsFunc := deploymentFuncs.ItemsFunc
1497+
itemCalled := 0
1498+
itemsCalled := 0
1499+
1500+
deploymentFuncs.ItemFunc = func(client kube.Clients, namespace string, name string) (runtime.Object, error) {
1501+
itemCalled++
1502+
return orgItemFunc(client, namespace, name)
1503+
}
1504+
deploymentFuncs.ItemsFunc = func(client kube.Clients, namespace string) []runtime.Object {
1505+
itemsCalled++
1506+
return orgItemsFunc(client, namespace)
1507+
}
1508+
14771509
patchCalled := 0
14781510
deploymentFuncs.PatchFunc = func(client kube.Clients, namespace string, resource runtime.Object, patchType patchtypes.PatchType, bytes []byte) error {
14791511
patchCalled++
@@ -1498,7 +1530,9 @@ func TestRollingUpgradeForDeploymentWithPatchAndRetryUsingArs(t *testing.T) {
14981530
t.Errorf("Rolling upgrade failed for Deployment with Configmap")
14991531
}
15001532

1501-
assert.Equal(t, 2, patchCalled)
1533+
assert.Equal(t, 1, itemCalled, "ItemFunc should be called once")
1534+
assert.Equal(t, 1, itemsCalled, "ItemsFunc should be called once")
1535+
assert.Equal(t, 2, patchCalled, "PatchFunc should be called twice")
15021536

15031537
deploymentFuncs = GetDeploymentRollingUpgradeFuncs()
15041538
testRollingUpgradeWithPatchAndInvokeDeleteStrategyArs(t, clients, config, deploymentFuncs, collectors, envVarPostfix)
@@ -2204,6 +2238,20 @@ func TestRollingUpgradeForDaemonSetWithConfigmapUsingArs(t *testing.T) {
22042238
daemonSetFuncs := GetDaemonSetRollingUpgradeFuncs()
22052239
collectors := getCollectors()
22062240

2241+
orgItemFunc := daemonSetFuncs.ItemFunc
2242+
orgItemsFunc := daemonSetFuncs.ItemsFunc
2243+
itemCalled := 0
2244+
itemsCalled := 0
2245+
2246+
daemonSetFuncs.ItemFunc = func(client kube.Clients, namespace string, name string) (runtime.Object, error) {
2247+
itemCalled++
2248+
return orgItemFunc(client, namespace, name)
2249+
}
2250+
daemonSetFuncs.ItemsFunc = func(client kube.Clients, namespace string) []runtime.Object {
2251+
itemsCalled++
2252+
return orgItemsFunc(client, namespace)
2253+
}
2254+
22072255
err := PerformAction(clients, config, daemonSetFuncs, collectors, nil, invokeReloadStrategy)
22082256
time.Sleep(5 * time.Second)
22092257
if err != nil {
@@ -2224,6 +2272,9 @@ func TestRollingUpgradeForDaemonSetWithConfigmapUsingArs(t *testing.T) {
22242272
t.Errorf("Counter by namespace was not increased")
22252273
}
22262274

2275+
assert.Equal(t, 0, itemCalled, "ItemFunc should not be called")
2276+
assert.Equal(t, 2, itemsCalled, "ItemsFunc should be called twice")
2277+
22272278
testRollingUpgradeInvokeDeleteStrategyArs(t, clients, config, daemonSetFuncs, collectors, envVarPostfix)
22282279
}
22292280

@@ -2235,6 +2286,20 @@ func TestRollingUpgradeForDaemonSetWithPatchAndRetryUsingArs(t *testing.T) {
22352286
config := getConfigWithAnnotations(envVarPostfix, arsConfigmapName, shaData, options.ConfigmapUpdateOnChangeAnnotation, options.ConfigmapReloaderAutoAnnotation)
22362287
daemonSetFuncs := GetDaemonSetRollingUpgradeFuncs()
22372288

2289+
orgItemFunc := daemonSetFuncs.ItemFunc
2290+
orgItemsFunc := daemonSetFuncs.ItemsFunc
2291+
itemCalled := 0
2292+
itemsCalled := 0
2293+
2294+
daemonSetFuncs.ItemFunc = func(client kube.Clients, namespace string, name string) (runtime.Object, error) {
2295+
itemCalled++
2296+
return orgItemFunc(client, namespace, name)
2297+
}
2298+
daemonSetFuncs.ItemsFunc = func(client kube.Clients, namespace string) []runtime.Object {
2299+
itemsCalled++
2300+
return orgItemsFunc(client, namespace)
2301+
}
2302+
22382303
assert.True(t, daemonSetFuncs.SupportsPatch)
22392304
assert.NotEmpty(t, daemonSetFuncs.PatchTemplatesFunc().AnnotationTemplate)
22402305

@@ -2263,7 +2328,9 @@ func TestRollingUpgradeForDaemonSetWithPatchAndRetryUsingArs(t *testing.T) {
22632328
t.Errorf("Rolling upgrade failed for DaemonSet with configmap")
22642329
}
22652330

2266-
assert.Equal(t, 2, patchCalled)
2331+
assert.Equal(t, 1, itemCalled, "ItemFunc should be called once")
2332+
assert.Equal(t, 1, itemsCalled, "ItemsFunc should be called once")
2333+
assert.Equal(t, 2, patchCalled, "PatchFunc should be called twice")
22672334

22682335
daemonSetFuncs = GetDeploymentRollingUpgradeFuncs()
22692336
testRollingUpgradeWithPatchAndInvokeDeleteStrategyArs(t, clients, config, daemonSetFuncs, collectors, envVarPostfix)
@@ -2406,6 +2473,20 @@ func TestRollingUpgradeForStatefulSetWithConfigmapUsingArs(t *testing.T) {
24062473
statefulSetFuncs := GetStatefulSetRollingUpgradeFuncs()
24072474
collectors := getCollectors()
24082475

2476+
orgItemFunc := statefulSetFuncs.ItemFunc
2477+
orgItemsFunc := statefulSetFuncs.ItemsFunc
2478+
itemCalled := 0
2479+
itemsCalled := 0
2480+
2481+
statefulSetFuncs.ItemFunc = func(client kube.Clients, namespace string, name string) (runtime.Object, error) {
2482+
itemCalled++
2483+
return orgItemFunc(client, namespace, name)
2484+
}
2485+
statefulSetFuncs.ItemsFunc = func(client kube.Clients, namespace string) []runtime.Object {
2486+
itemsCalled++
2487+
return orgItemsFunc(client, namespace)
2488+
}
2489+
24092490
err := PerformAction(clients, config, statefulSetFuncs, collectors, nil, invokeReloadStrategy)
24102491
time.Sleep(5 * time.Second)
24112492
if err != nil {
@@ -2426,6 +2507,9 @@ func TestRollingUpgradeForStatefulSetWithConfigmapUsingArs(t *testing.T) {
24262507
t.Errorf("Counter by namespace was not increased")
24272508
}
24282509

2510+
assert.Equal(t, 0, itemCalled, "ItemFunc should not be called")
2511+
assert.Equal(t, 2, itemsCalled, "ItemsFunc should be called twice")
2512+
24292513
testRollingUpgradeInvokeDeleteStrategyArs(t, clients, config, statefulSetFuncs, collectors, envVarPostfix)
24302514
}
24312515

@@ -2437,6 +2521,20 @@ func TestRollingUpgradeForStatefulSetWithPatchAndRetryUsingArs(t *testing.T) {
24372521
config := getConfigWithAnnotations(envVarPostfix, arsConfigmapName, shaData, options.ConfigmapUpdateOnChangeAnnotation, options.ConfigmapReloaderAutoAnnotation)
24382522
statefulSetFuncs := GetStatefulSetRollingUpgradeFuncs()
24392523

2524+
orgItemFunc := statefulSetFuncs.ItemFunc
2525+
orgItemsFunc := statefulSetFuncs.ItemsFunc
2526+
itemCalled := 0
2527+
itemsCalled := 0
2528+
2529+
statefulSetFuncs.ItemFunc = func(client kube.Clients, namespace string, name string) (runtime.Object, error) {
2530+
itemCalled++
2531+
return orgItemFunc(client, namespace, name)
2532+
}
2533+
statefulSetFuncs.ItemsFunc = func(client kube.Clients, namespace string) []runtime.Object {
2534+
itemsCalled++
2535+
return orgItemsFunc(client, namespace)
2536+
}
2537+
24402538
assert.True(t, statefulSetFuncs.SupportsPatch)
24412539
assert.NotEmpty(t, statefulSetFuncs.PatchTemplatesFunc().AnnotationTemplate)
24422540

@@ -2465,7 +2563,9 @@ func TestRollingUpgradeForStatefulSetWithPatchAndRetryUsingArs(t *testing.T) {
24652563
t.Errorf("Rolling upgrade failed for StatefulSet with configmap")
24662564
}
24672565

2468-
assert.Equal(t, 2, patchCalled)
2566+
assert.Equal(t, 1, itemCalled, "ItemFunc should be called once")
2567+
assert.Equal(t, 1, itemsCalled, "ItemsFunc should be called once")
2568+
assert.Equal(t, 2, patchCalled, "PatchFunc should be called twice")
24692569

24702570
statefulSetFuncs = GetDeploymentRollingUpgradeFuncs()
24712571
testRollingUpgradeWithPatchAndInvokeDeleteStrategyArs(t, clients, config, statefulSetFuncs, collectors, envVarPostfix)

0 commit comments

Comments
 (0)