Skip to content

Commit 7579a77

Browse files
joelanfordjoelanford
committed
main.go,pkg/annotation: fixup defaults, improve annotations
In pkg/annotation, added default annotations. In pkg/reconciler, updated annotation options to take variadic list of annotations (and fixed unit tests). In main.go, updated reconciler constructor call to use default annotations. Also fixed bugs with setting the default max concurrent reconciles and reconcile period.
1 parent b96a83a commit 7579a77

File tree

4 files changed

+71
-63
lines changed

4 files changed

+71
-63
lines changed

‎main.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,9 @@ func main() {
5252
enableLeaderElection bool
5353
leaderElectionID string
5454

55-
watchesFile string
56-
maxConcurrentReconciles int
57-
reconcilePeriod time.Duration
55+
watchesFile string
56+
defaultMaxConcurrentReconciles int
57+
defaultReconcilePeriod time.Duration
5858
)
5959

6060
flag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.")
@@ -64,8 +64,8 @@ func main() {
6464
"Name of the configmap that is used for holding the leader lock.")
6565

6666
flag.StringVar(&watchesFile, "watches-file", "./watches.yaml", "Path to watches.yaml file.")
67-
flag.IntVar(&maxConcurrentReconciles, "max-concurrent-reconciles", 1, "Default maximum number of concurrent reconciles for controllers.")
68-
flag.DurationVar(&reconcilePeriod, "reconcile-period", time.Minute, "Default reconcile period for controllers.")
67+
flag.IntVar(&defaultMaxConcurrentReconciles, "max-concurrent-reconciles", 1, "Default maximum number of concurrent reconciles for controllers.")
68+
flag.DurationVar(&defaultReconcilePeriod, "reconcile-period", 0, "Default reconcile period for controllers (use 0 to disable periodic reconciliation)")
6969

7070
klog.InitFlags(flag.CommandLine)
7171
flag.Parse()
@@ -100,9 +100,12 @@ func main() {
100100
}
101101

102102
for _, w := range ws {
103+
reconcilePeriod := defaultReconcilePeriod
103104
if w.ReconcilePeriod != nil {
104105
reconcilePeriod = *w.ReconcilePeriod
105106
}
107+
108+
maxConcurrentReconciles := defaultMaxConcurrentReconciles
106109
if w.MaxConcurrentReconciles != nil {
107110
maxConcurrentReconciles = *w.MaxConcurrentReconciles
108111
}
@@ -114,10 +117,9 @@ func main() {
114117
reconciler.SkipDependentWatches(w.WatchDependentResources != nil && !*w.WatchDependentResources),
115118
reconciler.WithMaxConcurrentReconciles(maxConcurrentReconciles),
116119
reconciler.WithReconcilePeriod(reconcilePeriod),
117-
reconciler.WithInstallAnnotation(&annotation.InstallDisableHooks{}),
118-
reconciler.WithUpgradeAnnotation(&annotation.UpgradeDisableHooks{}),
119-
reconciler.WithUpgradeAnnotation(&annotation.UpgradeForce{}),
120-
reconciler.WithUninstallAnnotation(&annotation.UninstallDisableHooks{}),
120+
reconciler.WithInstallAnnotations(annotation.DefaultInstallAnnotations...),
121+
reconciler.WithUpgradeAnnotations(annotation.DefaultUpgradeAnnotations...),
122+
reconciler.WithUninstallAnnotations(annotation.DefaultUninstallAnnotations...),
121123
)
122124
if err != nil {
123125
setupLog.Error(err, "unable to create helm reconciler", "controller", "Helm")

‎pkg/annotation/annotation.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ import (
88
helmclient "github.com/joelanford/helm-operator/pkg/client"
99
)
1010

11+
var (
12+
DefaultInstallAnnotations = []Install{InstallDescription{}, InstallDisableHooks{}}
13+
DefaultUpgradeAnnotations = []Upgrade{UpgradeDescription{}, UpgradeDisableHooks{}, UpgradeForce{}}
14+
DefaultUninstallAnnotations = []Uninstall{UninstallDescription{}, UninstallDisableHooks{}}
15+
)
16+
1117
type Install interface {
1218
Name() string
1319
InstallOption(string) helmclient.InstallOption

‎pkg/reconciler/reconciler.go

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -275,59 +275,65 @@ func WithReconcilePeriod(rp time.Duration) Option {
275275
}
276276
}
277277

278-
// WithInstallAnnotation is an Option that configures an Install annotation
278+
// WithInstallAnnotations is an Option that configures Install annotations
279279
// to enable custom action.Install fields to be set based on the value of
280280
// annotations found in the custom resource watched by this reconciler.
281281
// Duplicate annotation names will result in an error.
282-
func WithInstallAnnotation(a annotation.Install) Option {
282+
func WithInstallAnnotations(as ...annotation.Install) Option {
283283
return func(r *Reconciler) error {
284284
r.annotSetupOnce.Do(r.setupAnnotationMaps)
285285

286-
name := a.Name()
287-
if _, ok := r.annotations[name]; ok {
288-
return fmt.Errorf("annotation %q already exists", name)
289-
}
286+
for _, a := range as {
287+
name := a.Name()
288+
if _, ok := r.annotations[name]; ok {
289+
return fmt.Errorf("annotation %q already exists", name)
290+
}
290291

291-
r.annotations[name] = struct{}{}
292-
r.installAnnotations[name] = a
292+
r.annotations[name] = struct{}{}
293+
r.installAnnotations[name] = a
294+
}
293295
return nil
294296
}
295297
}
296298

297-
// WithUpgradeAnnotation is an Option that configures an Upgrade annotation
299+
// WithUpgradeAnnotations is an Option that configures Upgrade annotations
298300
// to enable custom action.Upgrade fields to be set based on the value of
299301
// annotations found in the custom resource watched by this reconciler.
300302
// Duplicate annotation names will result in an error.
301-
func WithUpgradeAnnotation(a annotation.Upgrade) Option {
303+
func WithUpgradeAnnotations(as ...annotation.Upgrade) Option {
302304
return func(r *Reconciler) error {
303305
r.annotSetupOnce.Do(r.setupAnnotationMaps)
304306

305-
name := a.Name()
306-
if _, ok := r.annotations[name]; ok {
307-
return fmt.Errorf("annotation %q already exists", name)
308-
}
307+
for _, a := range as {
308+
name := a.Name()
309+
if _, ok := r.annotations[name]; ok {
310+
return fmt.Errorf("annotation %q already exists", name)
311+
}
309312

310-
r.annotations[name] = struct{}{}
311-
r.upgradeAnnotations[name] = a
313+
r.annotations[name] = struct{}{}
314+
r.upgradeAnnotations[name] = a
315+
}
312316
return nil
313317
}
314318
}
315319

316-
// WithUninstallAnnotation is an Option that configures an Uninstall annotation
320+
// WithUninstallAnnotations is an Option that configures Uninstall annotations
317321
// to enable custom action.Uninstall fields to be set based on the value of
318322
// annotations found in the custom resource watched by this reconciler.
319323
// Duplicate annotation names will result in an error.
320-
func WithUninstallAnnotation(a annotation.Uninstall) Option {
324+
func WithUninstallAnnotations(as ...annotation.Uninstall) Option {
321325
return func(r *Reconciler) error {
322326
r.annotSetupOnce.Do(r.setupAnnotationMaps)
323327

324-
name := a.Name()
325-
if _, ok := r.annotations[name]; ok {
326-
return fmt.Errorf("annotation %q already exists", name)
327-
}
328+
for _, a := range as {
329+
name := a.Name()
330+
if _, ok := r.annotations[name]; ok {
331+
return fmt.Errorf("annotation %q already exists", name)
332+
}
328333

329-
r.annotations[name] = struct{}{}
330-
r.uninstallAnnotations[name] = a
334+
r.annotations[name] = struct{}{}
335+
r.uninstallAnnotations[name] = a
336+
}
331337
return nil
332338
}
333339
}

‎pkg/reconciler/reconciler_test.go

Lines changed: 24 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,11 @@ var _ = Describe("Reconciler", func() {
157157
Expect(WithReconcilePeriod(-time.Nanosecond)(r)).NotTo(Succeed())
158158
})
159159
})
160-
var _ = Describe("WithInstallAnnotation", func() {
160+
var _ = Describe("WithInstallAnnotations", func() {
161161
It("should set multiple reconciler install annotations", func() {
162162
a1 := annotation.InstallDisableHooks{CustomName: "my.domain/custom-name1"}
163163
a2 := annotation.InstallDisableHooks{CustomName: "my.domain/custom-name2"}
164-
Expect(WithInstallAnnotation(a1)(r)).To(Succeed())
165-
Expect(WithInstallAnnotation(a2)(r)).To(Succeed())
164+
Expect(WithInstallAnnotations(a1, a2)(r)).To(Succeed())
166165
Expect(r.annotations).To(Equal(map[string]struct{}{
167166
"my.domain/custom-name1": struct{}{},
168167
"my.domain/custom-name2": struct{}{},
@@ -175,8 +174,7 @@ var _ = Describe("Reconciler", func() {
175174
It("should error with duplicate install annotation", func() {
176175
a1 := annotation.InstallDisableHooks{CustomName: "my.domain/custom-name1"}
177176
a2 := annotation.InstallDisableHooks{CustomName: "my.domain/custom-name1"}
178-
Expect(WithInstallAnnotation(a1)(r)).To(Succeed())
179-
Expect(WithInstallAnnotation(a2)(r)).To(HaveOccurred())
177+
Expect(WithInstallAnnotations(a1, a2)(r)).NotTo(Succeed())
180178
Expect(r.annotations).To(Equal(map[string]struct{}{
181179
"my.domain/custom-name1": struct{}{},
182180
}))
@@ -187,8 +185,8 @@ var _ = Describe("Reconciler", func() {
187185
It("should error with duplicate upgrade annotation", func() {
188186
a1 := annotation.InstallDisableHooks{CustomName: "my.domain/custom-name1"}
189187
a2 := annotation.UpgradeDisableHooks{CustomName: "my.domain/custom-name1"}
190-
Expect(WithInstallAnnotation(a1)(r)).To(Succeed())
191-
Expect(WithUpgradeAnnotation(a2)(r)).To(HaveOccurred())
188+
Expect(WithInstallAnnotations(a1)(r)).To(Succeed())
189+
Expect(WithUpgradeAnnotations(a2)(r)).To(HaveOccurred())
192190
Expect(r.annotations).To(Equal(map[string]struct{}{
193191
"my.domain/custom-name1": struct{}{},
194192
}))
@@ -199,8 +197,8 @@ var _ = Describe("Reconciler", func() {
199197
It("should error with duplicate uninstall annotation", func() {
200198
a1 := annotation.InstallDisableHooks{CustomName: "my.domain/custom-name1"}
201199
a2 := annotation.UninstallDisableHooks{CustomName: "my.domain/custom-name1"}
202-
Expect(WithInstallAnnotation(a1)(r)).To(Succeed())
203-
Expect(WithUninstallAnnotation(a2)(r)).To(HaveOccurred())
200+
Expect(WithInstallAnnotations(a1)(r)).To(Succeed())
201+
Expect(WithUninstallAnnotations(a2)(r)).To(HaveOccurred())
204202
Expect(r.annotations).To(Equal(map[string]struct{}{
205203
"my.domain/custom-name1": struct{}{},
206204
}))
@@ -209,12 +207,11 @@ var _ = Describe("Reconciler", func() {
209207
}))
210208
})
211209
})
212-
var _ = Describe("WithUpgradeAnnotation", func() {
210+
var _ = Describe("WithUpgradeAnnotations", func() {
213211
It("should set multiple reconciler upgrade annotations", func() {
214212
a1 := annotation.UpgradeDisableHooks{CustomName: "my.domain/custom-name1"}
215213
a2 := annotation.UpgradeDisableHooks{CustomName: "my.domain/custom-name2"}
216-
Expect(WithUpgradeAnnotation(a1)(r)).To(Succeed())
217-
Expect(WithUpgradeAnnotation(a2)(r)).To(Succeed())
214+
Expect(WithUpgradeAnnotations(a1, a2)(r)).To(Succeed())
218215
Expect(r.annotations).To(Equal(map[string]struct{}{
219216
"my.domain/custom-name1": struct{}{},
220217
"my.domain/custom-name2": struct{}{},
@@ -227,8 +224,8 @@ var _ = Describe("Reconciler", func() {
227224
It("should error with duplicate install annotation", func() {
228225
a1 := annotation.UpgradeDisableHooks{CustomName: "my.domain/custom-name1"}
229226
a2 := annotation.InstallDisableHooks{CustomName: "my.domain/custom-name1"}
230-
Expect(WithUpgradeAnnotation(a1)(r)).To(Succeed())
231-
Expect(WithInstallAnnotation(a2)(r)).To(HaveOccurred())
227+
Expect(WithUpgradeAnnotations(a1)(r)).To(Succeed())
228+
Expect(WithInstallAnnotations(a2)(r)).To(HaveOccurred())
232229
Expect(r.annotations).To(Equal(map[string]struct{}{
233230
"my.domain/custom-name1": struct{}{},
234231
}))
@@ -239,8 +236,7 @@ var _ = Describe("Reconciler", func() {
239236
It("should error with duplicate upgrade annotation", func() {
240237
a1 := annotation.UpgradeDisableHooks{CustomName: "my.domain/custom-name1"}
241238
a2 := annotation.UpgradeDisableHooks{CustomName: "my.domain/custom-name1"}
242-
Expect(WithUpgradeAnnotation(a1)(r)).To(Succeed())
243-
Expect(WithUpgradeAnnotation(a2)(r)).To(HaveOccurred())
239+
Expect(WithUpgradeAnnotations(a1, a2)(r)).NotTo(Succeed())
244240
Expect(r.annotations).To(Equal(map[string]struct{}{
245241
"my.domain/custom-name1": struct{}{},
246242
}))
@@ -251,8 +247,8 @@ var _ = Describe("Reconciler", func() {
251247
It("should error with duplicate uninstall annotation", func() {
252248
a1 := annotation.UpgradeDisableHooks{CustomName: "my.domain/custom-name1"}
253249
a2 := annotation.UninstallDisableHooks{CustomName: "my.domain/custom-name1"}
254-
Expect(WithUpgradeAnnotation(a1)(r)).To(Succeed())
255-
Expect(WithUninstallAnnotation(a2)(r)).To(HaveOccurred())
250+
Expect(WithUpgradeAnnotations(a1)(r)).To(Succeed())
251+
Expect(WithUninstallAnnotations(a2)(r)).To(HaveOccurred())
256252
Expect(r.annotations).To(Equal(map[string]struct{}{
257253
"my.domain/custom-name1": struct{}{},
258254
}))
@@ -261,12 +257,11 @@ var _ = Describe("Reconciler", func() {
261257
}))
262258
})
263259
})
264-
var _ = Describe("WithUninstallAnnotation", func() {
260+
var _ = Describe("WithUninstallAnnotations", func() {
265261
It("should set multiple reconciler uninstall annotations", func() {
266262
a1 := annotation.UninstallDisableHooks{CustomName: "my.domain/custom-name1"}
267263
a2 := annotation.UninstallDisableHooks{CustomName: "my.domain/custom-name2"}
268-
Expect(WithUninstallAnnotation(a1)(r)).To(Succeed())
269-
Expect(WithUninstallAnnotation(a2)(r)).To(Succeed())
264+
Expect(WithUninstallAnnotations(a1, a2)(r)).To(Succeed())
270265
Expect(r.annotations).To(Equal(map[string]struct{}{
271266
"my.domain/custom-name1": struct{}{},
272267
"my.domain/custom-name2": struct{}{},
@@ -279,8 +274,8 @@ var _ = Describe("Reconciler", func() {
279274
It("should error with duplicate install annotation", func() {
280275
a1 := annotation.UninstallDisableHooks{CustomName: "my.domain/custom-name1"}
281276
a2 := annotation.InstallDisableHooks{CustomName: "my.domain/custom-name1"}
282-
Expect(WithUninstallAnnotation(a1)(r)).To(Succeed())
283-
Expect(WithInstallAnnotation(a2)(r)).To(HaveOccurred())
277+
Expect(WithUninstallAnnotations(a1)(r)).To(Succeed())
278+
Expect(WithInstallAnnotations(a2)(r)).To(HaveOccurred())
284279
Expect(r.annotations).To(Equal(map[string]struct{}{
285280
"my.domain/custom-name1": struct{}{},
286281
}))
@@ -291,8 +286,8 @@ var _ = Describe("Reconciler", func() {
291286
It("should error with duplicate uninstall annotation", func() {
292287
a1 := annotation.UninstallDisableHooks{CustomName: "my.domain/custom-name1"}
293288
a2 := annotation.UpgradeDisableHooks{CustomName: "my.domain/custom-name1"}
294-
Expect(WithUninstallAnnotation(a1)(r)).To(Succeed())
295-
Expect(WithUpgradeAnnotation(a2)(r)).To(HaveOccurred())
289+
Expect(WithUninstallAnnotations(a1)(r)).To(Succeed())
290+
Expect(WithUpgradeAnnotations(a2)(r)).To(HaveOccurred())
296291
Expect(r.annotations).To(Equal(map[string]struct{}{
297292
"my.domain/custom-name1": struct{}{},
298293
}))
@@ -303,8 +298,7 @@ var _ = Describe("Reconciler", func() {
303298
It("should error with duplicate uninstall annotation", func() {
304299
a1 := annotation.UninstallDisableHooks{CustomName: "my.domain/custom-name1"}
305300
a2 := annotation.UninstallDisableHooks{CustomName: "my.domain/custom-name1"}
306-
Expect(WithUninstallAnnotation(a1)(r)).To(Succeed())
307-
Expect(WithUninstallAnnotation(a2)(r)).To(HaveOccurred())
301+
Expect(WithUninstallAnnotations(a1, a2)(r)).NotTo(Succeed())
308302
Expect(r.annotations).To(Equal(map[string]struct{}{
309303
"my.domain/custom-name1": struct{}{},
310304
}))
@@ -378,9 +372,9 @@ var _ = Describe("Reconciler", func() {
378372
r, err = New(
379373
WithGroupVersionKind(gvk),
380374
WithChart(chrt),
381-
WithInstallAnnotation(annotation.InstallDescription{}),
382-
WithUpgradeAnnotation(annotation.UpgradeDescription{}),
383-
WithUninstallAnnotation(annotation.UninstallDescription{}),
375+
WithInstallAnnotations(annotation.InstallDescription{}),
376+
WithUpgradeAnnotations(annotation.UpgradeDescription{}),
377+
WithUninstallAnnotations(annotation.UninstallDescription{}),
384378
WithOverrideValues(map[string]string{
385379
"image.repository": "custom-nginx",
386380
}),

0 commit comments

Comments
 (0)