diff --git a/lib/capability/capability.go b/lib/capability/capability.go index 6f4bb042a..22398cda9 100644 --- a/lib/capability/capability.go +++ b/lib/capability/capability.go @@ -85,20 +85,6 @@ func GetCapabilitiesStatus(capabilities ClusterCapabilities) configv1.ClusterVer return status } -// GetImplicitlyEnabledCapabilities, given an enabled resource's current capabilities, compares them against -// the resource's capabilities from an update release. Any of the updated resource's capabilities that do not -// exist in the current resource, are not enabled, and do not already exist in the implicitly enabled capabilities -// are returned. The returned capabilities must be implicitly enabled. -func GetImplicitlyEnabledCapabilities(enabledManifestCaps sets.Set[configv1.ClusterVersionCapability], - updatedManifestCaps sets.Set[configv1.ClusterVersionCapability], - capabilities ClusterCapabilities) sets.Set[configv1.ClusterVersionCapability] { - caps := updatedManifestCaps.Difference(enabledManifestCaps).Difference(capabilities.Enabled).Difference(capabilities.ImplicitlyEnabled) - if caps.Len() == 0 { - return nil - } - return caps -} - // categorizeEnabledCapabilities categorizes enabled capabilities by implicitness from cluster version's // capabilities specification and a collection of capabilities that are enabled including implicitly enabled. func categorizeEnabledCapabilities(capabilitiesSpec *configv1.ClusterVersionCapabilitiesSpec, diff --git a/lib/capability/capability_test.go b/lib/capability/capability_test.go index 57687fdfd..7aaaadf8a 100644 --- a/lib/capability/capability_test.go +++ b/lib/capability/capability_test.go @@ -285,72 +285,3 @@ func TestSetFromImplicitlyEnabledCapabilities(t *testing.T) { }) } } - -func TestGetImplicitlyEnabledCapabilities(t *testing.T) { - tests := []struct { - name string - enabledManCaps sets.Set[configv1.ClusterVersionCapability] - updatedManCaps sets.Set[configv1.ClusterVersionCapability] - capabilities ClusterCapabilities - wantImplicit sets.Set[configv1.ClusterVersionCapability] - }{ - {name: "implicitly enable capability", - enabledManCaps: sets.New[configv1.ClusterVersionCapability]("cap1", "cap3"), - updatedManCaps: sets.New[configv1.ClusterVersionCapability]("cap2"), - capabilities: ClusterCapabilities{ - Enabled: sets.New[configv1.ClusterVersionCapability]("cap1"), - }, - wantImplicit: sets.New[configv1.ClusterVersionCapability]("cap2"), - }, - {name: "no prior caps, implicitly enabled capability", - updatedManCaps: sets.New[configv1.ClusterVersionCapability]("cap2"), - wantImplicit: sets.New[configv1.ClusterVersionCapability]("cap2"), - }, - {name: "multiple implicitly enable capability", - enabledManCaps: sets.New[configv1.ClusterVersionCapability]("cap1", "cap2", "cap3"), - updatedManCaps: sets.New[configv1.ClusterVersionCapability]("cap4", "cap5", "cap6"), - wantImplicit: sets.New[configv1.ClusterVersionCapability]("cap4", "cap5", "cap6"), - }, - {name: "no implicitly enable capability", - enabledManCaps: sets.New[configv1.ClusterVersionCapability]("cap1", "cap3"), - updatedManCaps: sets.New[configv1.ClusterVersionCapability]("cap1"), - capabilities: ClusterCapabilities{ - Enabled: sets.New[configv1.ClusterVersionCapability]("cap1"), - }, - }, - {name: "prior cap, no updated caps, no implicitly enabled capability", - enabledManCaps: sets.New[configv1.ClusterVersionCapability]("cap1"), - }, - {name: "no implicitly enable capability, already enabled", - enabledManCaps: sets.New[configv1.ClusterVersionCapability]("cap1", "cap2"), - updatedManCaps: sets.New[configv1.ClusterVersionCapability]("cap2"), - capabilities: ClusterCapabilities{ - Enabled: sets.New[configv1.ClusterVersionCapability]("cap1", "cap2"), - }, - }, - {name: "no implicitly enable capability, new cap but already enabled", - enabledManCaps: sets.New[configv1.ClusterVersionCapability]("cap1"), - updatedManCaps: sets.New[configv1.ClusterVersionCapability]("cap2"), - capabilities: ClusterCapabilities{ - Enabled: sets.New[configv1.ClusterVersionCapability]("cap2"), - }, - }, - {name: "no implicitly enable capability, already implicitly enabled", - enabledManCaps: sets.New[configv1.ClusterVersionCapability]("cap1"), - updatedManCaps: sets.New[configv1.ClusterVersionCapability]("cap2"), - capabilities: ClusterCapabilities{ - Enabled: sets.New[configv1.ClusterVersionCapability]("cap2"), - ImplicitlyEnabled: sets.New[configv1.ClusterVersionCapability]("cap2"), - }, - }, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - caps := GetImplicitlyEnabledCapabilities(test.enabledManCaps, test.updatedManCaps, test.capabilities) - if diff := cmp.Diff(test.wantImplicit, caps); diff != "" { - t.Errorf("%s: wantImplicit differs from expected:\n%s", test.name, diff) - } - - }) - } -} diff --git a/lib/manifest/manifest.go b/lib/manifest/manifest.go new file mode 100644 index 000000000..d5ad68233 --- /dev/null +++ b/lib/manifest/manifest.go @@ -0,0 +1,93 @@ +// Package manifest collects functions about manifests that are going to be lifted to library-go +// https://github.com/openshift/library-go +// Thus it should not depend on any packages from CVO: github.com/openshift/cluster-version-operator/ +package manifest + +import ( + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/library-go/pkg/manifest" + + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/klog/v2" +) + +// InclusionConfiguration configures manifest inclusion, so +// callers can opt in to new filtering options instead of having to +// update existing call-sites, even if they do not need a new +// filtering option. +type InclusionConfiguration struct { + // ExcludeIdentifier, if non-nil, excludes manifests that match the exclusion identifier. + ExcludeIdentifier *string + + // RequiredFeatureSet, if non-nil, excludes manifests unless they match the desired feature set. + RequiredFeatureSet *string + + // Profile, if non-nil, excludes manifests unless they match the cluster profile. + Profile *string + + // Capabilities, if non-nil, excludes manifests unless they match the enabled cluster capabilities. + Capabilities *configv1.ClusterVersionCapabilitiesStatus + + // Overrides excludes manifests for overridden resources. + Overrides []configv1.ComponentOverride + + // Platform, if non-nil, excludes CredentialsRequests manifests unless they match the infrastructure platform. + Platform *string +} + +// GetImplicitlyEnabledCapabilities returns a set of capabilities that are implicitly enabled after a cluster update. +// The arguments are two sets of manifests, manifest inclusion configuration, and +// a set of capabilities that are implicitly enabled on the cluster, i.e., the capabilities +// that are NOT specified in the cluster version but has to considered enabled on the cluster. +// The manifest inclusion configuration is used to determine if a manifest should be included. +// In other words, whether, or not the cluster version operator reconcile that manifest on the cluster. +// The two sets of manifests are respectively from the release that is currently running on the cluster and +// from the release that the cluster is updated to. +func GetImplicitlyEnabledCapabilities( + updatePayloadManifests []manifest.Manifest, + currentPayloadManifests []manifest.Manifest, + manifestInclusionConfiguration InclusionConfiguration, + currentImplicitlyEnabled sets.Set[configv1.ClusterVersionCapability], +) sets.Set[configv1.ClusterVersionCapability] { + ret := currentImplicitlyEnabled.Clone() + for _, updateManifest := range updatePayloadManifests { + updateManErr := updateManifest.IncludeAllowUnknownCapabilities( + manifestInclusionConfiguration.ExcludeIdentifier, + manifestInclusionConfiguration.RequiredFeatureSet, + manifestInclusionConfiguration.Profile, + manifestInclusionConfiguration.Capabilities, + manifestInclusionConfiguration.Overrides, + true, + ) + // update manifest is enabled, no need to check + if updateManErr == nil { + continue + } + for _, currentManifest := range currentPayloadManifests { + if !updateManifest.SameResourceID(currentManifest) { + continue + } + // current manifest is disabled, no need to check + if err := currentManifest.IncludeAllowUnknownCapabilities( + manifestInclusionConfiguration.ExcludeIdentifier, + manifestInclusionConfiguration.RequiredFeatureSet, + manifestInclusionConfiguration.Profile, + manifestInclusionConfiguration.Capabilities, + manifestInclusionConfiguration.Overrides, + true, + ); err != nil { + continue + } + newImplicitlyEnabled := sets.New[configv1.ClusterVersionCapability](updateManifest.GetManifestCapabilities()...). + Difference(sets.New[configv1.ClusterVersionCapability](currentManifest.GetManifestCapabilities()...)). + Difference(currentImplicitlyEnabled). + Difference(sets.New[configv1.ClusterVersionCapability](manifestInclusionConfiguration.Capabilities.EnabledCapabilities...)) + ret = ret.Union(newImplicitlyEnabled) + if newImplicitlyEnabled.Len() > 0 { + 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", + updateManifest.String(), sets.List(newImplicitlyEnabled)) + } + } + } + return ret +} diff --git a/pkg/payload/payload.go b/pkg/payload/payload.go index a0730dc04..e7fad6366 100644 --- a/pkg/payload/payload.go +++ b/pkg/payload/payload.go @@ -22,10 +22,11 @@ import ( "github.com/blang/semver/v4" configv1 "github.com/openshift/api/config/v1" imagev1 "github.com/openshift/api/image/v1" + "github.com/openshift/library-go/pkg/manifest" "github.com/openshift/cluster-version-operator/lib/capability" + localmanifest "github.com/openshift/cluster-version-operator/lib/manifest" "github.com/openshift/cluster-version-operator/lib/resourceread" - "github.com/openshift/library-go/pkg/manifest" ) // State describes the state of the payload and alters @@ -237,41 +238,13 @@ func LoadUpdate(dir, releaseImage, excludeIdentifier string, requiredFeatureSet // All capabilities requiring implicit enablement are returned. func GetImplicitlyEnabledCapabilities(updatePayloadManifests []manifest.Manifest, currentPayloadManifests []manifest.Manifest, capabilities capability.ClusterCapabilities) sets.Set[configv1.ClusterVersionCapability] { - clusterCaps := capability.GetCapabilitiesStatus(capabilities) - - // Initialize so it contains existing implicitly enabled capabilities - implicitlyEnabledCaps := capabilities.ImplicitlyEnabled.Clone() - - for _, updateManifest := range updatePayloadManifests { - updateManErr := updateManifest.IncludeAllowUnknownCapabilities(nil, nil, nil, &clusterCaps, nil, true) - - // update manifest is enabled, no need to check - if updateManErr == nil { - continue - } - for _, currentManifest := range currentPayloadManifests { - if !updateManifest.SameResourceID(currentManifest) { - continue - } - - // current manifest is disabled, no need to check - if err := currentManifest.IncludeAllowUnknownCapabilities(nil, nil, nil, &clusterCaps, nil, true); err != nil { - continue - } - caps := capability.GetImplicitlyEnabledCapabilities(sets.New[configv1.ClusterVersionCapability](currentManifest.GetManifestCapabilities()...), - sets.New[configv1.ClusterVersionCapability](updateManifest.GetManifestCapabilities()...), capabilities) - implicitlyEnabledCaps = implicitlyEnabledCaps.Union(caps) - if caps.Len() > 0 { - 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", - getManifestResourceId(updateManifest), capability.SortedList(caps)) - } - } - } - if implicitlyEnabledCaps.Len() == 0 { - return nil - } - return implicitlyEnabledCaps + return localmanifest.GetImplicitlyEnabledCapabilities( + updatePayloadManifests, + currentPayloadManifests, + localmanifest.InclusionConfiguration{Capabilities: &clusterCaps}, + capabilities.ImplicitlyEnabled, + ) } // ValidateDirectory checks if a directory can be a candidate update by