Skip to content

Commit fd5df0e

Browse files
hongkailiuhongkailiu
committed
OTA-1010: Add a new version of GetImplicitlyEnabledCapabilities
1 parent e90705b commit fd5df0e

File tree

4 files changed

+99
-149
lines changed

4 files changed

+99
-149
lines changed

‎lib/capability/capability.go

-34
Original file line numberDiff line numberDiff line change
@@ -97,40 +97,6 @@ func GetCapabilitiesStatus(capabilities ClusterCapabilities) configv1.ClusterVer
9797
return status
9898
}
9999

100-
// GetImplicitlyEnabledCapabilities, given an enabled resource's current capabilities, compares them against
101-
// the resource's capabilities from an update release. Any of the updated resource's capabilities that do not
102-
// exist in the current resource, are not enabled, and do not already exist in the implicitly enabled list of
103-
// capabilities are returned. The returned list are capabilities which must be implicitly enabled.
104-
func GetImplicitlyEnabledCapabilities(enabledManifestCaps []configv1.ClusterVersionCapability,
105-
updatedManifestCaps []configv1.ClusterVersionCapability,
106-
capabilities ClusterCapabilities) []configv1.ClusterVersionCapability {
107-
108-
var caps []configv1.ClusterVersionCapability
109-
for _, c := range updatedManifestCaps {
110-
if Contains(enabledManifestCaps, c) {
111-
continue
112-
}
113-
if _, ok := capabilities.EnabledCapabilities[c]; !ok {
114-
if !Contains(capabilities.ImplicitlyEnabledCapabilities, c) {
115-
caps = append(caps, c)
116-
}
117-
}
118-
}
119-
sort.Sort(capabilitiesSort(caps))
120-
return caps
121-
}
122-
123-
func Contains(caps []configv1.ClusterVersionCapability, capability configv1.ClusterVersionCapability) bool {
124-
found := false
125-
for _, c := range caps {
126-
if capability == c {
127-
found = true
128-
break
129-
}
130-
}
131-
return found
132-
}
133-
134100
// setKnownCapabilities populates a map keyed by capability from all known capabilities as defined in ClusterVersion.
135101
func setKnownCapabilities() map[configv1.ClusterVersionCapability]struct{} {
136102
known := make(map[configv1.ClusterVersionCapability]struct{})

‎lib/capability/capability_test.go

-80
Original file line numberDiff line numberDiff line change
@@ -328,83 +328,3 @@ func TestSetFromImplicitlyEnabledCapabilities(t *testing.T) {
328328
})
329329
}
330330
}
331-
332-
func TestGetImplicitlyEnabledCapabilities(t *testing.T) {
333-
tests := []struct {
334-
name string
335-
enabledManCaps []configv1.ClusterVersionCapability
336-
updatedManCaps []configv1.ClusterVersionCapability
337-
capabilities ClusterCapabilities
338-
wantImplicit []string
339-
}{
340-
{name: "implicitly enable capability",
341-
enabledManCaps: []configv1.ClusterVersionCapability{"cap1", "cap3"},
342-
updatedManCaps: []configv1.ClusterVersionCapability{"cap2"},
343-
capabilities: ClusterCapabilities{
344-
EnabledCapabilities: map[configv1.ClusterVersionCapability]struct{}{"cap1": {}},
345-
},
346-
wantImplicit: []string{"cap2"},
347-
},
348-
{name: "no prior caps, implicitly enabled capability",
349-
updatedManCaps: []configv1.ClusterVersionCapability{"cap2"},
350-
wantImplicit: []string{"cap2"},
351-
},
352-
{name: "multiple implicitly enable capability",
353-
enabledManCaps: []configv1.ClusterVersionCapability{"cap1", "cap2", "cap3"},
354-
updatedManCaps: []configv1.ClusterVersionCapability{"cap4", "cap5", "cap6"},
355-
wantImplicit: []string{"cap4", "cap5", "cap6"},
356-
},
357-
{name: "no implicitly enable capability",
358-
enabledManCaps: []configv1.ClusterVersionCapability{"cap1", "cap3"},
359-
updatedManCaps: []configv1.ClusterVersionCapability{"cap1"},
360-
capabilities: ClusterCapabilities{
361-
EnabledCapabilities: map[configv1.ClusterVersionCapability]struct{}{"cap1": {}},
362-
},
363-
},
364-
{name: "prior cap, no updated caps, no implicitly enabled capability",
365-
enabledManCaps: []configv1.ClusterVersionCapability{"cap1"},
366-
},
367-
{name: "no implicitly enable capability, already enabled",
368-
enabledManCaps: []configv1.ClusterVersionCapability{"cap1", "cap2"},
369-
updatedManCaps: []configv1.ClusterVersionCapability{"cap2"},
370-
capabilities: ClusterCapabilities{
371-
EnabledCapabilities: map[configv1.ClusterVersionCapability]struct{}{"cap1": {}, "cap2": {}},
372-
},
373-
},
374-
{name: "no implicitly enable capability, new cap but already enabled",
375-
enabledManCaps: []configv1.ClusterVersionCapability{"cap1"},
376-
updatedManCaps: []configv1.ClusterVersionCapability{"cap2"},
377-
capabilities: ClusterCapabilities{
378-
EnabledCapabilities: map[configv1.ClusterVersionCapability]struct{}{"cap2": {}},
379-
},
380-
},
381-
{name: "no implicitly enable capability, already implcitly enabled",
382-
enabledManCaps: []configv1.ClusterVersionCapability{"cap1"},
383-
updatedManCaps: []configv1.ClusterVersionCapability{"cap2"},
384-
capabilities: ClusterCapabilities{
385-
EnabledCapabilities: map[configv1.ClusterVersionCapability]struct{}{"cap2": {}},
386-
ImplicitlyEnabledCapabilities: []configv1.ClusterVersionCapability{"cap2"},
387-
},
388-
},
389-
}
390-
for _, test := range tests {
391-
t.Run(test.name, func(t *testing.T) {
392-
caps := GetImplicitlyEnabledCapabilities(test.enabledManCaps, test.updatedManCaps, test.capabilities)
393-
if len(caps) != len(test.wantImplicit) {
394-
t.Errorf("Incorrect number of implicitly enabled keys, wanted: %d. Implicitly enabled capabilities returned: %v", len(test.wantImplicit), caps)
395-
}
396-
for _, wanted := range test.wantImplicit {
397-
found := false
398-
for _, have := range caps {
399-
if wanted == string(have) {
400-
found = true
401-
break
402-
}
403-
}
404-
if !found {
405-
t.Errorf("Missing implicitly enabled capability %q. Implicitly enabled capabilities returned : %v", wanted, caps)
406-
}
407-
}
408-
})
409-
}
410-
}

‎lib/capability_v2/capability.go

+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// Package capability_v2 collects functions about capabilities 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 capability_v2
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+
// ManifestInclusionConfiguration 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 ManifestInclusionConfiguration 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 ManifestInclusionConfiguration,
50+
currentImplicitlyEnabled sets.Set[configv1.ClusterVersionCapability],
51+
) sets.Set[configv1.ClusterVersionCapability] {
52+
53+
ret := currentImplicitlyEnabled.Clone()
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

+7-35
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
imagev1 "github.com/openshift/api/image/v1"
2525

2626
"github.com/openshift/cluster-version-operator/lib/capability"
27+
"github.com/openshift/cluster-version-operator/lib/capability_v2"
2728
"github.com/openshift/cluster-version-operator/lib/resourceread"
2829
"github.com/openshift/library-go/pkg/manifest"
2930
)
@@ -239,41 +240,12 @@ func GetImplicitlyEnabledCapabilities(updatePayloadManifests []manifest.Manifest
239240
capabilities capability.ClusterCapabilities) []configv1.ClusterVersionCapability {
240241

241242
clusterCaps := capability.GetCapabilitiesStatus(capabilities)
242-
243-
// Initialize so it contains existing implicitly enabled capabilities
244-
implicitlyEnabledCaps := capabilities.ImplicitlyEnabledCapabilities
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(currentManifest.GetManifestCapabilities(),
263-
updateManifest.GetManifestCapabilities(), capabilities)
264-
265-
capStrings := make([]string, len(caps))
266-
for i, c := range caps {
267-
capStrings[i] = string(c)
268-
if !capability.Contains(implicitlyEnabledCaps, c) {
269-
implicitlyEnabledCaps = append(implicitlyEnabledCaps, c)
270-
}
271-
}
272-
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",
273-
getManifestResourceId(updateManifest), strings.Join(capStrings, ", "))
274-
}
275-
}
276-
return implicitlyEnabledCaps
243+
return capability_v2.GetImplicitlyEnabledCapabilities(
244+
updatePayloadManifests,
245+
currentPayloadManifests,
246+
capability_v2.ManifestInclusionConfiguration{Capabilities: &clusterCaps},
247+
sets.New[configv1.ClusterVersionCapability](capabilities.ImplicitlyEnabledCapabilities...),
248+
).UnsortedList()
277249
}
278250

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

0 commit comments

Comments
 (0)