Skip to content

Commit 1887292

Browse files
msafwankarimmsafwankarim
authored
Merge pull request stakater#952 from videlov/upgrade-optimization
[optimization] Do not re-fetch resource on first attempt on upgrade
2 parents 69361c9 + 68118e1 commit 1887292

2 files changed

Lines changed: 123 additions & 11 deletions

File tree

‎internal/pkg/handler/upgrade.go‎

Lines changed: 32 additions & 8 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
)
@@ -203,7 +205,6 @@ func doRollingUpgrade(config util.Config, collectors metrics.Collectors, recorde
203205
}
204206

205207
func rollingUpgrade(clients kube.Clients, config util.Config, upgradeFuncs callbacks.RollingUpgradeFuncs, collectors metrics.Collectors, recorder record.EventRecorder, strategy invokeStrategy) error {
206-
207208
err := PerformAction(clients, config, upgradeFuncs, collectors, recorder, strategy)
208209
if err != nil {
209210
logrus.Errorf("Rolling upgrade for '%s' failed with error = %v", config.ResourceName, err)
@@ -216,10 +217,9 @@ func PerformAction(clients kube.Clients, config util.Config, upgradeFuncs callba
216217
items := upgradeFuncs.ItemsFunc(clients, config.Namespace)
217218

218219
for _, item := range items {
219-
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
220-
return upgradeResource(clients, config, upgradeFuncs, collectors, recorder, strategy, item)
220+
err := retryOnConflict(retry.DefaultRetry, func(fetchResource bool) error {
221+
return upgradeResource(clients, config, upgradeFuncs, collectors, recorder, strategy, item, fetchResource)
221222
})
222-
223223
if err != nil {
224224
return err
225225
}
@@ -228,16 +228,40 @@ func PerformAction(clients kube.Clients, config util.Config, upgradeFuncs callba
228228
return nil
229229
}
230230

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

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

243267
// find correct annotation and update the resource

‎internal/pkg/handler/upgrade_test.go‎

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

1444+
itemCalled := 0
1445+
itemsCalled := 0
1446+
1447+
deploymentFuncs.ItemFunc = func(client kube.Clients, namespace string, name string) (runtime.Object, error) {
1448+
itemCalled++
1449+
return callbacks.GetDeploymentItem(client, namespace, name)
1450+
}
1451+
deploymentFuncs.ItemsFunc = func(client kube.Clients, namespace string) []runtime.Object {
1452+
itemsCalled++
1453+
return callbacks.GetDeploymentItems(client, namespace)
1454+
}
1455+
14441456
err := PerformAction(clients, config, deploymentFuncs, collectors, nil, invokeReloadStrategy)
14451457
time.Sleep(5 * time.Second)
14461458
if err != nil {
@@ -1460,6 +1472,10 @@ func TestRollingUpgradeForDeploymentWithConfigmapUsingArs(t *testing.T) {
14601472
if promtestutil.ToFloat64(collectors.ReloadedByNamespace.With(prometheus.Labels{"success": "true", "namespace": arsNamespace})) != 1 {
14611473
t.Errorf("Counter by namespace was not increased")
14621474
}
1475+
1476+
assert.Equal(t, 0, itemCalled, "ItemFunc should not be called")
1477+
assert.Equal(t, 2, itemsCalled, "ItemsFunc should be called twice")
1478+
14631479
testRollingUpgradeInvokeDeleteStrategyArs(t, clients, config, deploymentFuncs, collectors, envVarPostfix)
14641480
}
14651481

@@ -1474,6 +1490,18 @@ func TestRollingUpgradeForDeploymentWithPatchAndRetryUsingArs(t *testing.T) {
14741490
assert.True(t, deploymentFuncs.SupportsPatch)
14751491
assert.NotEmpty(t, deploymentFuncs.PatchTemplatesFunc().AnnotationTemplate)
14761492

1493+
itemCalled := 0
1494+
itemsCalled := 0
1495+
1496+
deploymentFuncs.ItemFunc = func(client kube.Clients, namespace string, name string) (runtime.Object, error) {
1497+
itemCalled++
1498+
return callbacks.GetDeploymentItem(client, namespace, name)
1499+
}
1500+
deploymentFuncs.ItemsFunc = func(client kube.Clients, namespace string) []runtime.Object {
1501+
itemsCalled++
1502+
return callbacks.GetDeploymentItems(client, namespace)
1503+
}
1504+
14771505
patchCalled := 0
14781506
deploymentFuncs.PatchFunc = func(client kube.Clients, namespace string, resource runtime.Object, patchType patchtypes.PatchType, bytes []byte) error {
14791507
patchCalled++
@@ -1498,7 +1526,9 @@ func TestRollingUpgradeForDeploymentWithPatchAndRetryUsingArs(t *testing.T) {
14981526
t.Errorf("Rolling upgrade failed for Deployment with Configmap")
14991527
}
15001528

1501-
assert.Equal(t, 2, patchCalled)
1529+
assert.Equal(t, 1, itemCalled, "ItemFunc should be called once")
1530+
assert.Equal(t, 1, itemsCalled, "ItemsFunc should be called once")
1531+
assert.Equal(t, 2, patchCalled, "PatchFunc should be called twice")
15021532

15031533
deploymentFuncs = GetDeploymentRollingUpgradeFuncs()
15041534
testRollingUpgradeWithPatchAndInvokeDeleteStrategyArs(t, clients, config, deploymentFuncs, collectors, envVarPostfix)
@@ -2204,6 +2234,18 @@ func TestRollingUpgradeForDaemonSetWithConfigmapUsingArs(t *testing.T) {
22042234
daemonSetFuncs := GetDaemonSetRollingUpgradeFuncs()
22052235
collectors := getCollectors()
22062236

2237+
itemCalled := 0
2238+
itemsCalled := 0
2239+
2240+
daemonSetFuncs.ItemFunc = func(client kube.Clients, namespace string, name string) (runtime.Object, error) {
2241+
itemCalled++
2242+
return callbacks.GetDaemonSetItem(client, namespace, name)
2243+
}
2244+
daemonSetFuncs.ItemsFunc = func(client kube.Clients, namespace string) []runtime.Object {
2245+
itemsCalled++
2246+
return callbacks.GetDaemonSetItems(client, namespace)
2247+
}
2248+
22072249
err := PerformAction(clients, config, daemonSetFuncs, collectors, nil, invokeReloadStrategy)
22082250
time.Sleep(5 * time.Second)
22092251
if err != nil {
@@ -2224,6 +2266,9 @@ func TestRollingUpgradeForDaemonSetWithConfigmapUsingArs(t *testing.T) {
22242266
t.Errorf("Counter by namespace was not increased")
22252267
}
22262268

2269+
assert.Equal(t, 0, itemCalled, "ItemFunc should not be called")
2270+
assert.Equal(t, 2, itemsCalled, "ItemsFunc should be called twice")
2271+
22272272
testRollingUpgradeInvokeDeleteStrategyArs(t, clients, config, daemonSetFuncs, collectors, envVarPostfix)
22282273
}
22292274

@@ -2235,6 +2280,18 @@ func TestRollingUpgradeForDaemonSetWithPatchAndRetryUsingArs(t *testing.T) {
22352280
config := getConfigWithAnnotations(envVarPostfix, arsConfigmapName, shaData, options.ConfigmapUpdateOnChangeAnnotation, options.ConfigmapReloaderAutoAnnotation)
22362281
daemonSetFuncs := GetDaemonSetRollingUpgradeFuncs()
22372282

2283+
itemCalled := 0
2284+
itemsCalled := 0
2285+
2286+
daemonSetFuncs.ItemFunc = func(client kube.Clients, namespace string, name string) (runtime.Object, error) {
2287+
itemCalled++
2288+
return callbacks.GetDaemonSetItem(client, namespace, name)
2289+
}
2290+
daemonSetFuncs.ItemsFunc = func(client kube.Clients, namespace string) []runtime.Object {
2291+
itemsCalled++
2292+
return callbacks.GetDaemonSetItems(client, namespace)
2293+
}
2294+
22382295
assert.True(t, daemonSetFuncs.SupportsPatch)
22392296
assert.NotEmpty(t, daemonSetFuncs.PatchTemplatesFunc().AnnotationTemplate)
22402297

@@ -2263,7 +2320,9 @@ func TestRollingUpgradeForDaemonSetWithPatchAndRetryUsingArs(t *testing.T) {
22632320
t.Errorf("Rolling upgrade failed for DaemonSet with configmap")
22642321
}
22652322

2266-
assert.Equal(t, 2, patchCalled)
2323+
assert.Equal(t, 1, itemCalled, "ItemFunc should be called once")
2324+
assert.Equal(t, 1, itemsCalled, "ItemsFunc should be called once")
2325+
assert.Equal(t, 2, patchCalled, "PatchFunc should be called twice")
22672326

22682327
daemonSetFuncs = GetDeploymentRollingUpgradeFuncs()
22692328
testRollingUpgradeWithPatchAndInvokeDeleteStrategyArs(t, clients, config, daemonSetFuncs, collectors, envVarPostfix)
@@ -2406,6 +2465,18 @@ func TestRollingUpgradeForStatefulSetWithConfigmapUsingArs(t *testing.T) {
24062465
statefulSetFuncs := GetStatefulSetRollingUpgradeFuncs()
24072466
collectors := getCollectors()
24082467

2468+
itemCalled := 0
2469+
itemsCalled := 0
2470+
2471+
statefulSetFuncs.ItemFunc = func(client kube.Clients, namespace string, name string) (runtime.Object, error) {
2472+
itemCalled++
2473+
return callbacks.GetStatefulSetItem(client, namespace, name)
2474+
}
2475+
statefulSetFuncs.ItemsFunc = func(client kube.Clients, namespace string) []runtime.Object {
2476+
itemsCalled++
2477+
return callbacks.GetStatefulSetItems(client, namespace)
2478+
}
2479+
24092480
err := PerformAction(clients, config, statefulSetFuncs, collectors, nil, invokeReloadStrategy)
24102481
time.Sleep(5 * time.Second)
24112482
if err != nil {
@@ -2426,6 +2497,9 @@ func TestRollingUpgradeForStatefulSetWithConfigmapUsingArs(t *testing.T) {
24262497
t.Errorf("Counter by namespace was not increased")
24272498
}
24282499

2500+
assert.Equal(t, 0, itemCalled, "ItemFunc should not be called")
2501+
assert.Equal(t, 2, itemsCalled, "ItemsFunc should be called twice")
2502+
24292503
testRollingUpgradeInvokeDeleteStrategyArs(t, clients, config, statefulSetFuncs, collectors, envVarPostfix)
24302504
}
24312505

@@ -2437,6 +2511,18 @@ func TestRollingUpgradeForStatefulSetWithPatchAndRetryUsingArs(t *testing.T) {
24372511
config := getConfigWithAnnotations(envVarPostfix, arsConfigmapName, shaData, options.ConfigmapUpdateOnChangeAnnotation, options.ConfigmapReloaderAutoAnnotation)
24382512
statefulSetFuncs := GetStatefulSetRollingUpgradeFuncs()
24392513

2514+
itemCalled := 0
2515+
itemsCalled := 0
2516+
2517+
statefulSetFuncs.ItemFunc = func(client kube.Clients, namespace string, name string) (runtime.Object, error) {
2518+
itemCalled++
2519+
return callbacks.GetStatefulSetItem(client, namespace, name)
2520+
}
2521+
statefulSetFuncs.ItemsFunc = func(client kube.Clients, namespace string) []runtime.Object {
2522+
itemsCalled++
2523+
return callbacks.GetStatefulSetItems(client, namespace)
2524+
}
2525+
24402526
assert.True(t, statefulSetFuncs.SupportsPatch)
24412527
assert.NotEmpty(t, statefulSetFuncs.PatchTemplatesFunc().AnnotationTemplate)
24422528

@@ -2465,7 +2551,9 @@ func TestRollingUpgradeForStatefulSetWithPatchAndRetryUsingArs(t *testing.T) {
24652551
t.Errorf("Rolling upgrade failed for StatefulSet with configmap")
24662552
}
24672553

2468-
assert.Equal(t, 2, patchCalled)
2554+
assert.Equal(t, 1, itemCalled, "ItemFunc should be called once")
2555+
assert.Equal(t, 1, itemsCalled, "ItemsFunc should be called once")
2556+
assert.Equal(t, 2, patchCalled, "PatchFunc should be called twice")
24692557

24702558
statefulSetFuncs = GetDeploymentRollingUpgradeFuncs()
24712559
testRollingUpgradeWithPatchAndInvokeDeleteStrategyArs(t, clients, config, statefulSetFuncs, collectors, envVarPostfix)

0 commit comments

Comments
 (0)