Skip to content

Commit 3090a6e

Browse files
rashmigottipatirashmigottipati
committed
improvements
Signed-off-by: Rashmi Gottipati <[email protected]>
1 parent 60696f5 commit 3090a6e

File tree

4 files changed

+142
-42
lines changed

4 files changed

+142
-42
lines changed

‎internal/cmd/internal/olmv1/catalog_update.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ func NewCatalogUpdateCmd(cfg *action.Configuration) *cobra.Command {
3333
}
3434

3535
func bindCatalogUpdateFlags(fs *pflag.FlagSet, i *v1action.CatalogUpdate) {
36-
fs.Int32Var(&i.Priority, "priority", 1, "priority determines the likelihood of a catalog being selected in conflict scenarios")
37-
fs.IntVar(&i.PollIntervalMinutes, "source-poll-interval-minutes", 5, "catalog source polling interval [in minutes]. Set to 0 or -1 to remove the polling interval.")
36+
fs.Int32Var(i.Priority, "priority", 1, "priority determines the likelihood of a catalog being selected in conflict scenarios")
37+
fs.IntVar(i.PollIntervalMinutes, "source-poll-interval-minutes", 5, "catalog source polling interval [in minutes]. Set to 0 or -1 to remove the polling interval.")
3838
fs.StringToStringVar(&i.Labels, "labels", map[string]string{}, "labels that will be added to the catalog")
3939
fs.StringVar(&i.AvailabilityMode, "availability-mode", "", "available means that the catalog should be active and serving data")
4040
fs.StringVar(&i.ImageRef, "image", "", "Image reference for the catalog source. Leave unset to retain the current image.")

‎internal/pkg/v1/action/action_suite_test.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -149,19 +149,18 @@ func withCatalogSourceType(sourceType olmv1.SourceType) catalogOpt {
149149
}
150150
}
151151

152-
func withCatalogSourcePriority(priority int32) catalogOpt {
152+
func withCatalogSourcePriority(priority *int32) catalogOpt {
153153
return func(catalog *olmv1.ClusterCatalog) {
154-
catalog.Spec.Priority = priority
154+
catalog.Spec.Priority = *priority
155155
}
156156
}
157157

158-
func withCatalogPollInterval(pollInterval int, ref string) catalogOpt {
158+
func withCatalogPollInterval(pollInterval *int) catalogOpt {
159159
return func(catalog *olmv1.ClusterCatalog) {
160160
if catalog.Spec.Source.Image == nil {
161161
catalog.Spec.Source.Image = &olmv1.ImageSource{}
162162
}
163-
catalog.Spec.Source.Image.Ref = ref
164-
catalog.Spec.Source.Image.PollIntervalMinutes = &pollInterval
163+
catalog.Spec.Source.Image.PollIntervalMinutes = pollInterval
165164
}
166165
}
167166

‎internal/pkg/v1/action/catalog_update.go‎

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ type CatalogUpdate struct {
1616
config *action.Configuration
1717
CatalogName string
1818

19-
Priority int32
20-
PollIntervalMinutes int
19+
Priority *int32
20+
PollIntervalMinutes *int
2121
Labels map[string]string
2222
AvailabilityMode string
2323
ImageRef string
@@ -63,38 +63,67 @@ func (cu *CatalogUpdate) Run(ctx context.Context) (*olmv1.ClusterCatalog, error)
6363
}
6464

6565
func (cu *CatalogUpdate) setUpdatedCatalog(catalog *olmv1.ClusterCatalog) {
66-
catalog.SetLabels(cu.Labels)
67-
catalog.Spec.Priority = cu.Priority
66+
existingLabels := catalog.GetLabels()
67+
if existingLabels == nil {
68+
existingLabels = make(map[string]string)
69+
}
70+
71+
if cu.Labels != nil {
72+
for k, v := range cu.Labels {
73+
if v == "" {
74+
delete(existingLabels, k) // remove keys with empty values
75+
} else {
76+
existingLabels[k] = v
77+
}
78+
}
79+
catalog.SetLabels(existingLabels)
80+
}
81+
82+
if cu.Priority != nil {
83+
catalog.Spec.Priority = *cu.Priority
84+
}
85+
6886
if catalog.Spec.Source.Image != nil {
69-
switch {
70-
case cu.PollIntervalMinutes <= 0:
87+
if cu.PollIntervalMinutes != nil {
88+
// Set PollIntervalMinutes to the value if it's not nil
89+
catalog.Spec.Source.Image.PollIntervalMinutes = cu.PollIntervalMinutes
90+
} else {
91+
// If it's nil, explicitly unset it
7192
catalog.Spec.Source.Image.PollIntervalMinutes = nil
72-
default:
73-
catalog.Spec.Source.Image.PollIntervalMinutes = &cu.PollIntervalMinutes
7493
}
7594

7695
if cu.ImageRef != "" {
7796
catalog.Spec.Source.Image.Ref = cu.ImageRef
7897
}
7998
}
80-
catalog.Spec.AvailabilityMode = olmv1.AvailabilityMode(cu.AvailabilityMode)
99+
100+
if cu.AvailabilityMode != "" {
101+
catalog.Spec.AvailabilityMode = olmv1.AvailabilityMode(cu.AvailabilityMode)
102+
}
103+
81104
}
82105

83106
func (cu *CatalogUpdate) setDefaults(catalog olmv1.ClusterCatalog) {
84107
catalogSrc := catalog.Spec.Source
108+
109+
if cu.PollIntervalMinutes != nil && (*cu.PollIntervalMinutes == 0 || *cu.PollIntervalMinutes == -1) {
110+
cu.PollIntervalMinutes = nil
111+
} else if cu.PollIntervalMinutes == nil && catalogSrc.Image != nil && catalogSrc.Image.PollIntervalMinutes != nil {
112+
// Only default if user didn’t explicitly set anything
113+
cu.PollIntervalMinutes = catalogSrc.Image.PollIntervalMinutes
114+
}
115+
85116
if cu.ImageRef == "" && catalogSrc.Image != nil {
86-
{
87-
cu.ImageRef = catalogSrc.Image.Ref
88-
}
89-
if cu.AvailabilityMode == "" {
90-
cu.AvailabilityMode = string(catalog.Spec.AvailabilityMode)
91-
}
92-
if cu.Priority == 1 {
93-
cu.Priority = catalog.Spec.Priority
94-
}
95-
if len(cu.Labels) == 0 {
96-
cu.Labels = catalog.Labels
97-
}
117+
cu.ImageRef = catalogSrc.Image.Ref
118+
}
119+
if cu.AvailabilityMode == "" {
120+
cu.AvailabilityMode = string(catalog.Spec.AvailabilityMode)
121+
}
122+
if cu.Priority == nil {
123+
cu.Priority = &catalog.Spec.Priority
124+
}
125+
if len(cu.Labels) == 0 {
126+
cu.Labels = catalog.Labels
98127
}
99128
}
100129

‎internal/pkg/v1/action/catalog_update_test.go‎

Lines changed: 86 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package action_test
22

33
import (
44
"context"
5-
"maps"
65

76
. "github.com/onsi/ginkgo"
87
. "github.com/onsi/gomega"
@@ -60,8 +59,8 @@ var _ = Describe("CatalogUpdate", func() {
6059
testCatalog := buildCatalog(
6160
"testCatalog",
6261
withCatalogSourceType(olmv1.SourceTypeImage),
63-
withCatalogPollInterval(5, "testCatalog"),
64-
withCatalogSourcePriority(1),
62+
withCatalogPollInterval(pointerToInt(5)),
63+
withCatalogSourcePriority(pointerToInt32(1)),
6564
withCatalogImageRef("quay.io/myrepo/myimage"),
6665
withCatalogAvailabilityMode(olmv1.AvailabilityModeAvailable),
6766
withCatalogLabels(map[string]string{"foo": "bar"}),
@@ -70,35 +69,53 @@ var _ = Describe("CatalogUpdate", func() {
7069

7170
updater := internalaction.NewCatalogUpdate(&cfg)
7271
updater.CatalogName = "testCatalog"
73-
updater.Priority = int32(1)
72+
updater.Priority = pointerToInt32(1)
7473
updater.Labels = map[string]string{"abc": "xyz"}
7574
updater.AvailabilityMode = string(olmv1.AvailabilityModeAvailable)
76-
updater.PollIntervalMinutes = int(5)
75+
updater.PollIntervalMinutes = pointerToInt(5)
7776
catalog, err := updater.Run(context.TODO())
7877

7978
Expect(err).To(BeNil())
8079
Expect(testCatalog).NotTo(BeNil())
81-
Expect(maps.Equal(catalog.Labels, updater.Labels)).To(BeTrue())
82-
Expect(catalog.Spec.Priority).To(Equal(updater.Priority))
80+
Expect(catalog.Labels).To(HaveKeyWithValue("foo", "bar")) //existing
81+
Expect(catalog.Labels).To(HaveKeyWithValue("abc", "xyz")) //newly added
82+
Expect(catalog.Spec.Priority).To(Equal(*updater.Priority))
8383
Expect(catalog.Spec.Source.Image.PollIntervalMinutes).ToNot(BeNil())
84-
Expect(*catalog.Spec.Source.Image.PollIntervalMinutes).To(Equal(int(5)))
84+
Expect(*catalog.Spec.Source.Image.PollIntervalMinutes).To(Equal(*updater.PollIntervalMinutes))
8585
Expect(catalog.Spec.AvailabilityMode).To(Equal(olmv1.AvailabilityMode(updater.AvailabilityMode)))
8686
})
8787

8888
It("unsets the poll interval field when set to 0", func() {
8989
testCatalog := buildCatalog(
9090
"test",
9191
withCatalogSourceType(olmv1.SourceTypeImage),
92-
withCatalogPollInterval(7, "test"),
92+
withCatalogPollInterval(pointerToInt(7)),
9393
withCatalogImageRef("quay.io/myrepo/myimage"),
94-
withCatalogAvailabilityMode(olmv1.AvailabilityModeAvailable),
95-
withCatalogLabels(map[string]string{"foo": "bar"}),
9694
)
9795
cfg := setupEnv(testCatalog)
9896

9997
updater := internalaction.NewCatalogUpdate(&cfg)
10098
updater.CatalogName = "test"
101-
updater.PollIntervalMinutes = 0
99+
updater.PollIntervalMinutes = pointerToInt(-1)
100+
catalog, err := updater.Run(context.TODO())
101+
102+
Expect(err).NotTo(HaveOccurred())
103+
Expect(catalog.Spec.Source.Image.PollIntervalMinutes).To(BeNil())
104+
})
105+
106+
It("unsets the poll interval field when set to 0", func() {
107+
testCatalog := buildCatalog(
108+
"test",
109+
withCatalogSourceType(olmv1.SourceTypeImage),
110+
withCatalogPollInterval(pointerToInt(10)),
111+
withCatalogImageRef("quay.io/myrepo/myimage"),
112+
)
113+
cfg := setupEnv(testCatalog)
114+
115+
updater := internalaction.NewCatalogUpdate(&cfg)
116+
updater.CatalogName = "test"
117+
updater.PollIntervalMinutes = pointerToInt(0)
118+
102119
catalog, err := updater.Run(context.TODO())
103120

104121
Expect(err).NotTo(HaveOccurred())
@@ -110,8 +127,8 @@ var _ = Describe("CatalogUpdate", func() {
110127
"test",
111128
withCatalogSourceType(olmv1.SourceTypeImage),
112129
withCatalogImageRef("quay.io/myrepo/myimage"),
113-
withCatalogPollInterval(10, "my-catalog"),
114-
withCatalogSourcePriority(5),
130+
withCatalogPollInterval(pointerToInt(10)),
131+
withCatalogSourcePriority(pointerToInt32(5)),
115132
withCatalogAvailabilityMode(olmv1.AvailabilityModeAvailable),
116133
withCatalogLabels(map[string]string{"foo": "bar"}),
117134
)
@@ -143,4 +160,59 @@ var _ = Describe("CatalogUpdate", func() {
143160
Expect(err.Error()).To(ContainSubstring("invalid image reference"))
144161
})
145162

163+
It("preserves existing catalog values if Priority and PollIntervalMinutes are nil", func() {
164+
testCatalog := buildCatalog(
165+
"test",
166+
withCatalogSourceType(olmv1.SourceTypeImage),
167+
withCatalogPollInterval(pointerToInt(15)),
168+
withCatalogSourcePriority(pointerToInt32(10)),
169+
withCatalogImageRef("quay.io/myrepo/image"),
170+
)
171+
172+
cfg := setupEnv(testCatalog)
173+
174+
updater := internalaction.NewCatalogUpdate(&cfg)
175+
updater.CatalogName = "test"
176+
updater.Priority = nil
177+
updater.PollIntervalMinutes = nil
178+
179+
catalog, err := updater.Run(context.TODO())
180+
181+
Expect(err).NotTo(HaveOccurred())
182+
Expect(catalog.Spec.Priority).To(Equal(int32(10)))
183+
Expect(*catalog.Spec.Source.Image.PollIntervalMinutes).To(Equal(15))
184+
})
185+
186+
It("removes labels with empty values and merges the rest", func() {
187+
initial := map[string]string{"foo": "bar", "remove": "yes"}
188+
testCatalog := buildCatalog(
189+
"test",
190+
withCatalogSourceType(olmv1.SourceTypeImage),
191+
withCatalogLabels(initial),
192+
)
193+
cfg := setupEnv(testCatalog)
194+
195+
updater := internalaction.NewCatalogUpdate(&cfg)
196+
updater.CatalogName = "test"
197+
updater.Labels = map[string]string{
198+
"remove": "",
199+
"new": "label",
200+
}
201+
catalog, err := updater.Run(context.TODO())
202+
Expect(err).NotTo(HaveOccurred())
203+
204+
Expect(catalog.Labels).To(Equal(map[string]string{
205+
"foo": "bar",
206+
"new": "label",
207+
}))
208+
})
209+
146210
})
211+
212+
func pointerToInt32(i int32) *int32 {
213+
return &i
214+
}
215+
216+
func pointerToInt(i int) *int {
217+
return &i
218+
}

0 commit comments

Comments
 (0)