Skip to content

Commit fd85a98

Browse files
authored
feat(diff): change HeadsOnly mode to be an explicit setting (#950)
Currently the headsOnly mode is implilitly set if newRef is not provided. This behavior is unclear and leads to confusion. This commit will change the current headsOnly to be explicit setting that will be converted to package setting that will be executed accordingly. Signed-off-by: Vu Dinh <[email protected]>
1 parent aa8db09 commit fd85a98

File tree

6 files changed

+192
-14
lines changed

6 files changed

+192
-14
lines changed

alpha/action/diff.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ type Diff struct {
2828
IncludeConfig DiffIncludeConfig
2929
// IncludeAdditively catalog objects specified in IncludeConfig.
3030
IncludeAdditively bool
31+
// HeadsOnly is the mode that selects the head of the channels only.
32+
HeadsOnly bool
3133

3234
Logger *logrus.Entry
3335
}
@@ -75,6 +77,7 @@ func (diff Diff) Run(ctx context.Context) (*declcfg.DeclarativeConfig, error) {
7577
SkipDependencies: diff.SkipDependencies,
7678
Includer: convertIncludeConfigToIncluder(diff.IncludeConfig),
7779
IncludeAdditively: diff.IncludeAdditively,
80+
HeadsOnly: diff.HeadsOnly,
7881
}
7982
diffModel, err := g.Run(oldModel, newModel)
8083
if err != nil {

alpha/action/diff_test.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ func TestDiff(t *testing.T) {
4646
{
4747
name: "Success/HeadsOnly",
4848
diff: Diff{
49-
Registry: registry,
50-
NewRefs: []string{filepath.Join("testdata", "index-declcfgs", "latest")},
49+
Registry: registry,
50+
NewRefs: []string{filepath.Join("testdata", "index-declcfgs", "latest")},
51+
HeadsOnly: true,
5152
},
5253
expectedCfg: loadDirFS(t, indicesDir, filepath.Join("testdata", "index-declcfgs", "exp-headsonly")),
5354
assertion: require.NoError,
@@ -61,6 +62,7 @@ func TestDiff(t *testing.T) {
6162
Packages: []DiffIncludePackage{{Name: "baz"}},
6263
},
6364
IncludeAdditively: true,
65+
HeadsOnly: true,
6466
},
6567
expectedCfg: loadDirFS(t, indicesDir, filepath.Join("testdata", "index-declcfgs", "exp-include-pkg")),
6668
assertion: require.NoError,
@@ -79,6 +81,7 @@ func TestDiff(t *testing.T) {
7981
},
8082
},
8183
IncludeAdditively: true,
84+
HeadsOnly: true,
8285
},
8386
expectedCfg: loadDirFS(t, indicesDir, filepath.Join("testdata", "index-declcfgs", "exp-include-channel")),
8487
assertion: require.NoError,
@@ -97,6 +100,7 @@ func TestDiff(t *testing.T) {
97100
},
98101
},
99102
IncludeAdditively: true,
103+
HeadsOnly: true,
100104
},
101105
expectedCfg: loadDirFS(t, indicesDir, filepath.Join("testdata", "index-declcfgs", "exp-include-channel")),
102106
assertion: require.NoError,
@@ -115,6 +119,7 @@ func TestDiff(t *testing.T) {
115119
},
116120
},
117121
IncludeAdditively: true,
122+
HeadsOnly: true,
118123
},
119124
expectedCfg: loadDirFS(t, indicesDir, filepath.Join("testdata", "index-declcfgs", "exp-include-channel")),
120125
assertion: require.NoError,
@@ -134,15 +139,17 @@ func TestDiff(t *testing.T) {
134139
},
135140
},
136141
IncludeAdditively: true,
142+
HeadsOnly: true,
137143
},
138144
expectedCfg: loadDirFS(t, indicesDir, filepath.Join("testdata", "index-declcfgs", "exp-include-channel")),
139145
assertion: require.NoError,
140146
},
141147
{
142148
name: "Fail/NewBundleImage",
143149
diff: Diff{
144-
Registry: registry,
145-
NewRefs: []string{"test.registry/foo-operator/foo-bundle:v0.1.0"},
150+
Registry: registry,
151+
NewRefs: []string{"test.registry/foo-operator/foo-bundle:v0.1.0"},
152+
HeadsOnly: true,
146153
},
147154
assertion: func(t require.TestingT, err error, _ ...interface{}) {
148155
if !assert.Error(t, err) {

alpha/declcfg/diff.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ type DiffGenerator struct {
2525
Includer DiffIncluder
2626
// IncludeAdditively catalog objects specified in Includer in headsOnly mode.
2727
IncludeAdditively bool
28+
// HeadsOnly is the mode that selects the head of the channels only.
29+
HeadsOnly bool
2830

2931
initOnce sync.Once
3032
}
@@ -37,6 +39,8 @@ func (g *DiffGenerator) init() {
3739
if g.Includer.Logger == nil {
3840
g.Includer.Logger = g.Logger
3941
}
42+
// Inject headsOnly setting into DiffIncluder from command line setting
43+
g.Includer.HeadsOnly = g.HeadsOnly
4044
})
4145
}
4246

@@ -79,7 +83,7 @@ func (g *DiffGenerator) Run(oldModel, newModel model.Model) (model.Model, error)
7983
return nil
8084
}
8185

82-
headsOnlyMode := len(oldModel) == 0
86+
headsOnlyMode := g.HeadsOnly
8387
latestMode := !headsOnlyMode
8488
isInclude := len(g.Includer.Packages) != 0
8589

@@ -111,6 +115,9 @@ func (g *DiffGenerator) Run(oldModel, newModel model.Model) (model.Model, error)
111115

112116
case isInclude: // Add included objects to outputModel.
113117

118+
// Assume heads-only is false for include additively since we already have the channel heads
119+
// in the output model.
120+
g.Includer.HeadsOnly = false
114121
// Add included packages/channels/bundles from newModel to outputModel.
115122
if err := g.Includer.Run(newModel, outputModel); err != nil {
116123
return nil, err

alpha/declcfg/diff_include.go

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ type DiffIncluder struct {
1717
// Packages to add.
1818
Packages []DiffIncludePackage
1919
Logger *logrus.Entry
20+
// HeadsOnly is the mode that selects the head of the channels only.
21+
// This setting will be overridden by any versions or bundles in the channels.
22+
HeadsOnly bool
2023
}
2124

2225
// DiffIncludePackage specifies a package, and optionally channels
@@ -35,6 +38,9 @@ type DiffIncludePackage struct {
3538
// Package range setting is mutually exclusive with channel range/bundles/version
3639
// settings.
3740
Range semver.Range
41+
// HeadsOnly is the mode that selects the head of the channels only.
42+
// This setting will be overridden by any versions or bundles in the channels.
43+
HeadsOnly bool
3844
}
3945

4046
// DiffIncludeChannel specifies a channel, and optionally bundles and bundle versions
@@ -126,6 +132,7 @@ func (i DiffIncluder) Run(newModel, outputModel model.Model) error {
126132

127133
for _, ipkg := range i.Packages {
128134
pkgLog := i.Logger.WithField("package", ipkg.Name)
135+
ipkg.HeadsOnly = i.HeadsOnly
129136
includeErrs = append(includeErrs, ipkg.includeNewInOutputModel(newModel, outputModel, pkgLog)...)
130137
}
131138
if len(includeErrs) != 0 {
@@ -146,10 +153,20 @@ func (ipkg DiffIncludePackage) includeNewInOutputModel(newModel, outputModel mod
146153
}
147154
pkgLog := logger.WithField("package", newPkg.Name)
148155

149-
// No channels or versions were specified, meaning "include the full package".
156+
// No range, channels or versions were specified
150157
if len(ipkg.Channels) == 0 && len(ipkg.AllChannels.Versions) == 0 && len(ipkg.AllChannels.Bundles) == 0 && ipkg.Range == nil {
151-
outputModel[ipkg.Name] = newPkg
152-
return nil
158+
// heads-only false, meaning "include the full package".
159+
if !ipkg.HeadsOnly {
160+
outputModel[ipkg.Name] = newPkg
161+
return nil
162+
}
163+
// heads-only true, get the head of every channel in the package
164+
for _, c := range newPkg.Channels {
165+
newCh := DiffIncludeChannel{
166+
Name: c.Name,
167+
}
168+
ipkg.Channels = append(ipkg.Channels, newCh)
169+
}
153170
}
154171

155172
outputPkg := copyPackageNoChannels(newPkg)
@@ -197,16 +214,24 @@ func (ipkg DiffIncludePackage) includeNewInOutputModel(newModel, outputModel mod
197214
chLog := pkgLog.WithField("channel", newCh.Name)
198215

199216
var bundles []*model.Bundle
217+
var head *model.Bundle
200218
var err error
201-
if ich.Range != nil {
219+
// No versions have been specified, but heads-only set to true, get the channel head only.
220+
switch {
221+
case ipkg.HeadsOnly && len(ich.Versions) == 0 && len(ich.Bundles) == 0 && ich.Range == nil:
222+
head, err = newCh.Head()
223+
bundles = append(bundles, head)
224+
case ich.Range != nil:
202225
bundles, err = getBundlesForRange(newCh, ich.Range, chLog)
203-
} else {
226+
default:
204227
bundles, err = getBundlesForVersions(newCh, ich.Versions, ich.Bundles, chLog, skipMissingBundleForChannels[newCh.Name])
205228
}
229+
206230
if err != nil {
207231
ierrs = append(ierrs, fmt.Errorf("[package=%q channel=%q] %v", newPkg.Name, newCh.Name, err))
208232
continue
209233
}
234+
210235
outputCh := copyChannelNoBundles(newCh, outputPkg)
211236
outputPkg.Channels[outputCh.Name] = outputCh
212237
for _, b := range bundles {

0 commit comments

Comments
 (0)