Skip to content

Commit 79916aa

Browse files
committed
OTA-1010: Add a new version of GetImplicitlyEnabledCapabilities
1 parent 198fdab commit 79916aa

File tree

4 files changed

+100
-118
lines changed

4 files changed

+100
-118
lines changed

lib/capability/capability.go

-14
Original file line numberDiff line numberDiff line change
@@ -85,20 +85,6 @@ func GetCapabilitiesStatus(capabilities ClusterCapabilities) configv1.ClusterVer
8585
return status
8686
}
8787

88-
// GetImplicitlyEnabledCapabilities, given an enabled resource's current capabilities, compares them against
89-
// the resource's capabilities from an update release. Any of the updated resource's capabilities that do not
90-
// exist in the current resource, are not enabled, and do not already exist in the implicitly enabled capabilities
91-
// are returned. The returned capabilities must be implicitly enabled.
92-
func GetImplicitlyEnabledCapabilities(enabledManifestCaps sets.Set[configv1.ClusterVersionCapability],
93-
updatedManifestCaps sets.Set[configv1.ClusterVersionCapability],
94-
capabilities ClusterCapabilities) sets.Set[configv1.ClusterVersionCapability] {
95-
caps := updatedManifestCaps.Difference(enabledManifestCaps).Difference(capabilities.Enabled).Difference(capabilities.ImplicitlyEnabled)
96-
if caps.Len() == 0 {
97-
return nil
98-
}
99-
return caps
100-
}
101-
10288
// categorizeEnabledCapabilities categorizes enabled capabilities by implicitness from cluster version's
10389
// capabilities specification and a collection of capabilities that are enabled including implicitly enabled.
10490
func categorizeEnabledCapabilities(capabilitiesSpec *configv1.ClusterVersionCapabilitiesSpec,

lib/capability/capability_test.go

-69
Original file line numberDiff line numberDiff line change
@@ -285,72 +285,3 @@ func TestSetFromImplicitlyEnabledCapabilities(t *testing.T) {
285285
})
286286
}
287287
}
288-
289-
func TestGetImplicitlyEnabledCapabilities(t *testing.T) {
290-
tests := []struct {
291-
name string
292-
enabledManCaps sets.Set[configv1.ClusterVersionCapability]
293-
updatedManCaps sets.Set[configv1.ClusterVersionCapability]
294-
capabilities ClusterCapabilities
295-
wantImplicit sets.Set[configv1.ClusterVersionCapability]
296-
}{
297-
{name: "implicitly enable capability",
298-
enabledManCaps: sets.New[configv1.ClusterVersionCapability]("cap1", "cap3"),
299-
updatedManCaps: sets.New[configv1.ClusterVersionCapability]("cap2"),
300-
capabilities: ClusterCapabilities{
301-
Enabled: sets.New[configv1.ClusterVersionCapability]("cap1"),
302-
},
303-
wantImplicit: sets.New[configv1.ClusterVersionCapability]("cap2"),
304-
},
305-
{name: "no prior caps, implicitly enabled capability",
306-
updatedManCaps: sets.New[configv1.ClusterVersionCapability]("cap2"),
307-
wantImplicit: sets.New[configv1.ClusterVersionCapability]("cap2"),
308-
},
309-
{name: "multiple implicitly enable capability",
310-
enabledManCaps: sets.New[configv1.ClusterVersionCapability]("cap1", "cap2", "cap3"),
311-
updatedManCaps: sets.New[configv1.ClusterVersionCapability]("cap4", "cap5", "cap6"),
312-
wantImplicit: sets.New[configv1.ClusterVersionCapability]("cap4", "cap5", "cap6"),
313-
},
314-
{name: "no implicitly enable capability",
315-
enabledManCaps: sets.New[configv1.ClusterVersionCapability]("cap1", "cap3"),
316-
updatedManCaps: sets.New[configv1.ClusterVersionCapability]("cap1"),
317-
capabilities: ClusterCapabilities{
318-
Enabled: sets.New[configv1.ClusterVersionCapability]("cap1"),
319-
},
320-
},
321-
{name: "prior cap, no updated caps, no implicitly enabled capability",
322-
enabledManCaps: sets.New[configv1.ClusterVersionCapability]("cap1"),
323-
},
324-
{name: "no implicitly enable capability, already enabled",
325-
enabledManCaps: sets.New[configv1.ClusterVersionCapability]("cap1", "cap2"),
326-
updatedManCaps: sets.New[configv1.ClusterVersionCapability]("cap2"),
327-
capabilities: ClusterCapabilities{
328-
Enabled: sets.New[configv1.ClusterVersionCapability]("cap1", "cap2"),
329-
},
330-
},
331-
{name: "no implicitly enable capability, new cap but already enabled",
332-
enabledManCaps: sets.New[configv1.ClusterVersionCapability]("cap1"),
333-
updatedManCaps: sets.New[configv1.ClusterVersionCapability]("cap2"),
334-
capabilities: ClusterCapabilities{
335-
Enabled: sets.New[configv1.ClusterVersionCapability]("cap2"),
336-
},
337-
},
338-
{name: "no implicitly enable capability, already implicitly enabled",
339-
enabledManCaps: sets.New[configv1.ClusterVersionCapability]("cap1"),
340-
updatedManCaps: sets.New[configv1.ClusterVersionCapability]("cap2"),
341-
capabilities: ClusterCapabilities{
342-
Enabled: sets.New[configv1.ClusterVersionCapability]("cap2"),
343-
ImplicitlyEnabled: sets.New[configv1.ClusterVersionCapability]("cap2"),
344-
},
345-
},
346-
}
347-
for _, test := range tests {
348-
t.Run(test.name, func(t *testing.T) {
349-
caps := GetImplicitlyEnabledCapabilities(test.enabledManCaps, test.updatedManCaps, test.capabilities)
350-
if diff := cmp.Diff(test.wantImplicit, caps); diff != "" {
351-
t.Errorf("%s: wantImplicit differs from expected:\n%s", test.name, diff)
352-
}
353-
354-
})
355-
}
356-
}

lib/manifest/manifest.go

+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// Package manifest collects functions about manifests that are going to be lifted to library-go
2+
// https://github.com/openshift/library-go
3+
// Thus it should not depend on any packages from CVO: github.com/openshift/cluster-version-operator/
4+
package manifest
5+
6+
import (
7+
configv1 "github.com/openshift/api/config/v1"
8+
"github.com/openshift/library-go/pkg/manifest"
9+
10+
"k8s.io/apimachinery/pkg/util/sets"
11+
"k8s.io/klog/v2"
12+
)
13+
14+
// InclusionConfiguration configures manifest inclusion, so
15+
// callers can opt in to new filtering options instead of having to
16+
// update existing call-sites, even if they do not need a new
17+
// filtering option.
18+
type InclusionConfiguration struct {
19+
// ExcludeIdentifier, if non-nil, excludes manifests that match the exclusion identifier.
20+
ExcludeIdentifier *string
21+
22+
// RequiredFeatureSet, if non-nil, excludes manifests unless they match the desired feature set.
23+
RequiredFeatureSet *string
24+
25+
// Profile, if non-nil, excludes manifests unless they match the cluster profile.
26+
Profile *string
27+
28+
// Capabilities, if non-nil, excludes manifests unless they match the enabled cluster capabilities.
29+
Capabilities *configv1.ClusterVersionCapabilitiesStatus
30+
31+
// Overrides excludes manifests for overridden resources.
32+
Overrides []configv1.ComponentOverride
33+
34+
// Platform, if non-nil, excludes CredentialsRequests manifests unless they match the infrastructure platform.
35+
Platform *string
36+
}
37+
38+
// GetImplicitlyEnabledCapabilities returns a set of capabilities that are implicitly enabled after a cluster update.
39+
// The arguments are two sets of manifests, manifest inclusion configuration, and
40+
// a set of capabilities that are implicitly enabled on the cluster, i.e., the capabilities
41+
// that are NOT specified in the cluster version but has to considered enabled on the cluster.
42+
// The manifest inclusion configuration is used to determine if a manifest should be included.
43+
// In other words, whether, or not the cluster version operator reconcile that manifest on the cluster.
44+
// The two sets of manifests are respectively from the release that is currently running on the cluster and
45+
// from the release that the cluster is updated to.
46+
func GetImplicitlyEnabledCapabilities(
47+
updatePayloadManifests []manifest.Manifest,
48+
currentPayloadManifests []manifest.Manifest,
49+
manifestInclusionConfiguration InclusionConfiguration,
50+
currentImplicitlyEnabled sets.Set[configv1.ClusterVersionCapability],
51+
) sets.Set[configv1.ClusterVersionCapability] {
52+
53+
ret := sets.New[configv1.ClusterVersionCapability]().Union(currentImplicitlyEnabled)
54+
for _, updateManifest := range updatePayloadManifests {
55+
updateManErr := updateManifest.IncludeAllowUnknownCapabilities(
56+
manifestInclusionConfiguration.ExcludeIdentifier,
57+
manifestInclusionConfiguration.RequiredFeatureSet,
58+
manifestInclusionConfiguration.Profile,
59+
manifestInclusionConfiguration.Capabilities,
60+
manifestInclusionConfiguration.Overrides,
61+
true,
62+
)
63+
// update manifest is enabled, no need to check
64+
if updateManErr == nil {
65+
continue
66+
}
67+
for _, currentManifest := range currentPayloadManifests {
68+
if !updateManifest.SameResourceID(currentManifest) {
69+
continue
70+
}
71+
// current manifest is disabled, no need to check
72+
if err := currentManifest.IncludeAllowUnknownCapabilities(
73+
manifestInclusionConfiguration.ExcludeIdentifier,
74+
manifestInclusionConfiguration.RequiredFeatureSet,
75+
manifestInclusionConfiguration.Profile,
76+
manifestInclusionConfiguration.Capabilities,
77+
manifestInclusionConfiguration.Overrides,
78+
true,
79+
); err != nil {
80+
continue
81+
}
82+
newImplicitlyEnabled := sets.New[configv1.ClusterVersionCapability](updateManifest.GetManifestCapabilities()...).
83+
Difference(sets.New[configv1.ClusterVersionCapability](currentManifest.GetManifestCapabilities()...)).
84+
Difference(currentImplicitlyEnabled).
85+
Difference(sets.New[configv1.ClusterVersionCapability](manifestInclusionConfiguration.Capabilities.EnabledCapabilities...))
86+
ret = ret.Union(newImplicitlyEnabled)
87+
klog.V(2).Infof("%s has changed and is now part of one or more disabled capabilities. The following capabilities will be implicitly enabled: %s",
88+
updateManifest.String(), newImplicitlyEnabled.UnsortedList())
89+
}
90+
}
91+
return ret
92+
}

pkg/payload/payload.go

+8-35
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,11 @@ import (
2222
"github.com/blang/semver/v4"
2323
configv1 "github.com/openshift/api/config/v1"
2424
imagev1 "github.com/openshift/api/image/v1"
25+
"github.com/openshift/library-go/pkg/manifest"
2526

2627
"github.com/openshift/cluster-version-operator/lib/capability"
28+
localmanifest "github.com/openshift/cluster-version-operator/lib/manifest"
2729
"github.com/openshift/cluster-version-operator/lib/resourceread"
28-
"github.com/openshift/library-go/pkg/manifest"
2930
)
3031

3132
// State describes the state of the payload and alters
@@ -237,41 +238,13 @@ func LoadUpdate(dir, releaseImage, excludeIdentifier string, requiredFeatureSet
237238
// All capabilities requiring implicit enablement are returned.
238239
func GetImplicitlyEnabledCapabilities(updatePayloadManifests []manifest.Manifest, currentPayloadManifests []manifest.Manifest,
239240
capabilities capability.ClusterCapabilities) sets.Set[configv1.ClusterVersionCapability] {
240-
241241
clusterCaps := capability.GetCapabilitiesStatus(capabilities)
242-
243-
// Initialize so it contains existing implicitly enabled capabilities
244-
implicitlyEnabledCaps := capabilities.ImplicitlyEnabled.Clone()
245-
246-
for _, updateManifest := range updatePayloadManifests {
247-
updateManErr := updateManifest.IncludeAllowUnknownCapabilities(nil, nil, nil, &clusterCaps, nil, true)
248-
249-
// update manifest is enabled, no need to check
250-
if updateManErr == nil {
251-
continue
252-
}
253-
for _, currentManifest := range currentPayloadManifests {
254-
if !updateManifest.SameResourceID(currentManifest) {
255-
continue
256-
}
257-
258-
// current manifest is disabled, no need to check
259-
if err := currentManifest.IncludeAllowUnknownCapabilities(nil, nil, nil, &clusterCaps, nil, true); err != nil {
260-
continue
261-
}
262-
caps := capability.GetImplicitlyEnabledCapabilities(sets.New[configv1.ClusterVersionCapability](currentManifest.GetManifestCapabilities()...),
263-
sets.New[configv1.ClusterVersionCapability](updateManifest.GetManifestCapabilities()...), capabilities)
264-
implicitlyEnabledCaps = implicitlyEnabledCaps.Union(caps)
265-
if caps.Len() > 0 {
266-
klog.V(2).Infof("%s has changed and is now part of one or more disabled capabilities. The following capabilities will be implicitly enabled: %s",
267-
getManifestResourceId(updateManifest), capability.SortedList(caps))
268-
}
269-
}
270-
}
271-
if implicitlyEnabledCaps.Len() == 0 {
272-
return nil
273-
}
274-
return implicitlyEnabledCaps
242+
return localmanifest.GetImplicitlyEnabledCapabilities(
243+
updatePayloadManifests,
244+
currentPayloadManifests,
245+
localmanifest.InclusionConfiguration{Capabilities: &clusterCaps},
246+
capabilities.ImplicitlyEnabled,
247+
)
275248
}
276249

277250
// ValidateDirectory checks if a directory can be a candidate update by

0 commit comments

Comments
 (0)