Skip to content

Commit f0852ed

Browse files
authored
Merge pull request #1435 from prembhaskal/reduce-apply-timeout
updating default value for apply-timeout option for kapp to 5mins
2 parents d4125e1 + 13b0b8e commit f0852ed

File tree

2 files changed

+21
-6
lines changed

2 files changed

+21
-6
lines changed

pkg/config/config.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,11 +164,17 @@ func (gc *Config) KappDeployRawOptions() []string {
164164
gc.dataLock.RLock()
165165
defer gc.dataLock.RUnlock()
166166

167+
kappOptions := make([]string, 0)
168+
167169
// Configure kapp to keep only 5 app changes as it seems that
168170
// larger number of ConfigMaps negative affects other controllers on the cluster.
169171
// Eventually kapp can be smart enough to keep minimal number of app changes.
170172
// Set default first so that it can be overridden by user provided options.
171-
return append([]string{"--app-changes-max-to-keep=5"}, gc.data.kappDeployRawOptions...)
173+
// return append([]string{"--app-changes-max-to-keep=5"}, gc.data.kappDeployRawOptions...)
174+
kappOptions = append(kappOptions, "--app-changes-max-to-keep=5")
175+
kappOptions = append(kappOptions, "--apply-timeout=5m")
176+
kappOptions = append(kappOptions, gc.data.kappDeployRawOptions...)
177+
return kappOptions
172178
}
173179

174180
// AppDefaultSyncPeriod returns duration that is used by Apps

pkg/config/config_test.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,9 @@ func Test_NewConfig_AppMinimumSyncPeriod(t *testing.T) {
166166
}
167167

168168
func Test_NewConfig_KappDeployRawOptions(t *testing.T) {
169+
defaultRawOptions := []string{
170+
"--app-changes-max-to-keep=5", "--apply-timeout=5m",
171+
}
169172
t.Run("with empty config value, returns just default", func(t *testing.T) {
170173
secret := &v1.Secret{
171174
ObjectMeta: metav1.ObjectMeta{
@@ -176,7 +179,7 @@ func Test_NewConfig_KappDeployRawOptions(t *testing.T) {
176179
}
177180
config, err := kcconfig.NewConfig(k8sfake.NewSimpleClientset(secret))
178181
assert.NoError(t, err)
179-
assert.Equal(t, []string{"--app-changes-max-to-keep=5"}, config.KappDeployRawOptions())
182+
assert.Equal(t, defaultRawOptions, config.KappDeployRawOptions())
180183
})
181184

182185
t.Run("with empty config value, returns just default", func(t *testing.T) {
@@ -191,7 +194,7 @@ func Test_NewConfig_KappDeployRawOptions(t *testing.T) {
191194
}
192195
config, err := kcconfig.NewConfig(k8sfake.NewSimpleClientset(secret))
193196
assert.NoError(t, err)
194-
assert.Equal(t, []string{"--app-changes-max-to-keep=5"}, config.KappDeployRawOptions())
197+
assert.Equal(t, defaultRawOptions, config.KappDeployRawOptions())
195198
})
196199

197200
t.Run("with populated config value, returns default and user set", func(t *testing.T) {
@@ -206,7 +209,7 @@ func Test_NewConfig_KappDeployRawOptions(t *testing.T) {
206209
}
207210
config, err := kcconfig.NewConfig(k8sfake.NewSimpleClientset(secret))
208211
assert.NoError(t, err)
209-
assert.Equal(t, []string{"--app-changes-max-to-keep=5", "--key=val"}, config.KappDeployRawOptions())
212+
assert.Equal(t, appendNewSlice(defaultRawOptions, "--key=val"), config.KappDeployRawOptions())
210213
})
211214

212215
t.Run("clears previously set value when secret is gone", func(t *testing.T) {
@@ -223,17 +226,23 @@ func Test_NewConfig_KappDeployRawOptions(t *testing.T) {
223226

224227
config, err := kcconfig.NewConfig(client)
225228
assert.NoError(t, err)
226-
assert.Equal(t, []string{"--app-changes-max-to-keep=5", "--key=val"}, config.KappDeployRawOptions())
229+
assert.Equal(t, appendNewSlice(defaultRawOptions, "--key=val"), config.KappDeployRawOptions())
227230

228231
err = client.CoreV1().Secrets("default").Delete(
229232
context.Background(), "kapp-controller-config", metav1.DeleteOptions{})
230233
assert.NoError(t, err)
231234

232235
assert.NoError(t, config.Reload())
233-
assert.Equal(t, []string{"--app-changes-max-to-keep=5"}, config.KappDeployRawOptions())
236+
assert.Equal(t, defaultRawOptions, config.KappDeployRawOptions())
234237
})
235238
}
236239

240+
func appendNewSlice(act []string, items ...string) []string {
241+
newslice := make([]string, 0)
242+
newslice = append(newslice, act...)
243+
return append(newslice, items...)
244+
}
245+
237246
func Test_NewConfig_ReturnsConfigMap_WhenOnlyConfigMapExists(t *testing.T) {
238247
configMap := &v1.ConfigMap{
239248
ObjectMeta: metav1.ObjectMeta{

0 commit comments

Comments
 (0)