Skip to content

OCPSTRAT-2876: Allow inclusion/exclusion of manifests based on the OCP major version #1282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JoelSpeed wants to merge 1 commit into
base: main
Choose a base branch
from
Open

OCPSTRAT-2876: Allow inclusion/exclusion of manifests based on the OCP major version #1282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ require (
github.com/onsi/ginkgo/v2 v2.21.0
github.com/onsi/gomega v1.35.1
github.com/openshift-eng/openshift-tests-extension v0.0.0-20250220212757-b9c4d98a0c45
github.com/openshift/api v0.0.0-20260116192047-6fb7fdae95fd
github.com/openshift/api v0.0.0-20260126183958-606bd613f9f7
github.com/openshift/client-go v0.0.0-20260108185524-48f4ccfc4e13
github.com/openshift/library-go v0.0.0-20260121132910-dc3a1c884c04
github.com/operator-framework/api v0.17.1
Expand All @@ -34,6 +34,8 @@ require (
k8s.io/utils v0.0.0-20250604170112-4c0f3b243397
)

replace github.com/openshift/library-go => github.com/JoelSpeed/library-go v0.0.0-20260130121715-6e5ccfd0da42

require (
cel.dev/expr v0.24.0 // indirect
github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
Expand Down
8 changes: 4 additions & 4 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
cel.dev/expr v0.24.0 h1:56OvJKSH3hDGL0ml5uSxZmz3/3Pq4tJ+fb1unVLAFcY=
cel.dev/expr v0.24.0/go.mod h1:hLPLo1W4QUmuYdA72RBX06QTs6MXw941piREPl3Yfiw=
github.com/JoelSpeed/library-go v0.0.0-20260130121715-6e5ccfd0da42 h1:uIrR1qWklQm8U7WgtSZnyxj8bxCRtzvQ9JDKWvihOCg=
github.com/JoelSpeed/library-go v0.0.0-20260130121715-6e5ccfd0da42/go.mod h1:DCRz1EgdayEmr9b6KXKDL+DWBN0rGHu/VYADeHzPoOk=
github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI=
github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g=
github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
Expand Down Expand Up @@ -86,12 +88,10 @@ github.com/onsi/gomega v1.35.1 h1:Cwbd75ZBPxFSuZ6T+rN/WCb/gOc6YgFBXLlZLhC7Ds4=
github.com/onsi/gomega v1.35.1/go.mod h1:PvZbdDc8J6XJEpDK4HCuRBm8a6Fzp9/DmhC9C7yFlog=
github.com/openshift-eng/openshift-tests-extension v0.0.0-20250220212757-b9c4d98a0c45 h1:hXpbYtP3iTh8oy/RKwKkcMziwchY3fIk95ciczf7cOA=
github.com/openshift-eng/openshift-tests-extension v0.0.0-20250220212757-b9c4d98a0c45/go.mod h1:6gkP5f2HL0meusT0Aim8icAspcD1cG055xxBZ9yC68M=
github.com/openshift/api v0.0.0-20260116192047-6fb7fdae95fd h1:Hpjq/55Qb0Gy65RewTSWWlyxpSovaRF86EPjYQkdlRU=
github.com/openshift/api v0.0.0-20260116192047-6fb7fdae95fd/go.mod h1:d5uzF0YN2nQQFA0jIEWzzOZ+edmo6wzlGLvx5Fhz4uY=
github.com/openshift/api v0.0.0-20260126183958-606bd613f9f7 h1:96rhgJpWlWzKEslMd6aYFMixV9vQVY32M71JcO4Gzn0=
github.com/openshift/api v0.0.0-20260126183958-606bd613f9f7/go.mod h1:d5uzF0YN2nQQFA0jIEWzzOZ+edmo6wzlGLvx5Fhz4uY=
github.com/openshift/client-go v0.0.0-20260108185524-48f4ccfc4e13 h1:6rd4zSo2UaWQcAPZfHK9yzKVqH0BnMv1hqMzqXZyTds=
github.com/openshift/client-go v0.0.0-20260108185524-48f4ccfc4e13/go.mod h1:YvOmPmV7wcJxpfhTDuFqqs2Xpb3M3ovsM6Qs/i2ptq4=
github.com/openshift/library-go v0.0.0-20260121132910-dc3a1c884c04 h1:Fm9C8pT4l6VjpdqdhI1cBX9Y3D3S+rFxptVhCPBbMAI=
github.com/openshift/library-go v0.0.0-20260121132910-dc3a1c884c04/go.mod h1:nIzWQQE49XbiKizVnVOip9CEB7HJ0hoJwNi3g3YKnKc=
github.com/openshift/onsi-ginkgo/v2 v2.6.1-0.20241205171354-8006f302fd12 h1:AKx/w1qpS8We43bsRgf8Nll3CGlDHpr/WAXvuedTNZI=
github.com/openshift/onsi-ginkgo/v2 v2.6.1-0.20241205171354-8006f302fd12/go.mod h1:7Du3c42kxCUegi0IImZ1wUQzMBVecgIHjR1C+NkhLQo=
github.com/operator-framework/api v0.17.1 h1:J/6+Xj4IEV8C7hcirqUFwOiZAU3PbnJhWvB0/bB51c4=
Expand Down
5 changes: 5 additions & 0 deletions lib/manifest/manifest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ type InclusionConfiguration struct {

// EnabledFeatureGates excludes manifests with a feature gate requirement when the condition is not met.
EnabledFeatureGates sets.Set[string]

// MajorVersion, if non-nil, excludes manifests unless they match the major version.
MajorVersion *uint64
}

// GetImplicitlyEnabledCapabilities returns a set of capabilities that are implicitly enabled after a cluster update.
Expand All @@ -61,6 +64,7 @@ func GetImplicitlyEnabledCapabilities(
manifestInclusionConfiguration.Capabilities,
manifestInclusionConfiguration.Overrides,
manifestInclusionConfiguration.EnabledFeatureGates,
manifestInclusionConfiguration.MajorVersion,
true,
)
// update manifest is enabled, no need to check
Expand All @@ -79,6 +83,7 @@ func GetImplicitlyEnabledCapabilities(
manifestInclusionConfiguration.Capabilities,
manifestInclusionConfiguration.Overrides,
manifestInclusionConfiguration.EnabledFeatureGates,
manifestInclusionConfiguration.MajorVersion,
true,
); err != nil {
continue
Expand Down
8 changes: 4 additions & 4 deletions pkg/cvo/featuregate_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func TestFeatureGateManifestFiltering(t *testing.T) {
}

// Test the manifest inclusion logic
err := manifest.Include(nil, nil, nil, nil, nil, tt.enabledGates)
err := manifest.Include(nil, nil, nil, nil, nil, tt.enabledGates, nil)

if tt.shouldInclude {
if err != nil {
Expand Down Expand Up @@ -143,7 +143,7 @@ func TestSyncWorkIntegration(t *testing.T) {
Obj: testObj,
}

err := manifest.Include(nil, nil, nil, nil, nil, work.EnabledFeatureGates)
err := manifest.Include(nil, nil, nil, nil, nil, work.EnabledFeatureGates, nil)
if err != nil {
t.Errorf("Expected manifest to be included with TestGate1 enabled, got error: %v", err)
}
Expand All @@ -153,7 +153,7 @@ func TestSyncWorkIntegration(t *testing.T) {
"release.openshift.io/feature-gate": "DisabledGate",
})

err = manifest.Include(nil, nil, nil, nil, nil, work.EnabledFeatureGates)
err = manifest.Include(nil, nil, nil, nil, nil, work.EnabledFeatureGates, nil)
if err == nil {
t.Error("Expected manifest to be excluded with DisabledGate not enabled, but no error occurred")
}
Expand Down Expand Up @@ -262,7 +262,7 @@ func TestManifestFilteringExamples(t *testing.T) {
Obj: obj,
}

err := manifest.Include(nil, nil, nil, nil, nil, example.enabledGates)
err := manifest.Include(nil, nil, nil, nil, nil, example.enabledGates, nil)

if example.shouldInclude {
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/cvo/sync_worker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ func (w *SyncWorker) syncPayload(ctx context.Context, work *SyncWork) ([]configv

if w.payload != nil {
implicitlyEnabledCaps = capability.SortedList(payload.GetImplicitlyEnabledCapabilities(payloadUpdate.Manifests, w.payload.Manifests,
work.Capabilities, work.EnabledFeatureGates))
work.Capabilities, work.EnabledFeatureGates, w.payload.MajorVersion))
}

w.payload = payloadUpdate
Expand Down Expand Up @@ -1065,7 +1065,7 @@ func (w *SyncWorker) apply(ctx context.Context, work *SyncWork, maxWorkers int,
if task.Manifest.GVK != configv1.GroupVersion.WithKind("ClusterOperator") {
continue
}
if err := task.Manifest.Include(nil, nil, nil, &capabilities, work.Overrides, work.EnabledFeatureGates); err != nil {
if err := task.Manifest.Include(nil, nil, nil, &capabilities, work.Overrides, work.EnabledFeatureGates, payloadUpdate.MajorVersion); err != nil {
klog.V(manifestVerbosity).Infof("Skipping precreation of %s: %s", task, err)
continue
}
Expand All @@ -1085,7 +1085,7 @@ func (w *SyncWorker) apply(ctx context.Context, work *SyncWork, maxWorkers int,

klog.V(manifestVerbosity).Infof("Running sync for %s", task)

if err := task.Manifest.Include(nil, nil, nil, &capabilities, work.Overrides, work.EnabledFeatureGates); err != nil {
if err := task.Manifest.Include(nil, nil, nil, &capabilities, work.Overrides, work.EnabledFeatureGates, payloadUpdate.MajorVersion); err != nil {
klog.V(manifestVerbosity).Infof("Skipping %s: %s", task, err)
continue
}
Expand Down
14 changes: 11 additions & 3 deletions pkg/payload/payload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"time"

"k8s.io/klog/v2"
"k8s.io/utils/ptr"

utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -110,6 +111,7 @@ type Update struct {
ImageRef *imagev1.ImageStream

Architecture string
MajorVersion *uint64
Copy link
Contributor

@DavidHurta DavidHurta Feb 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, the purpose of MajorVersion is starting to become a bit fuzzy in comparison to the Release.Version field. The meaning of MajorVersion in the inclusion filtering context is clear; however, here we use it in a general struct type representing the contents of a release image. Thus, their purpose becomes less clear.

We could do one of the following:

  • Document MajorVersion field to make the difference between Release.Version and MajorVersion clearer.
  • Leave only one source of truth (that is, Release.Version), and extract the major version when needed using a function.
  • Create a new field ParsedVersion semver.Version; we could use it to get access to the parsed cached version of the update easily. This could even be utilized in the future when we need easy access to the parsed version of an update, which is more handy for fine grained processing. This would bring additional value to the new field, make it more general, and make its distinction clearer. The call sites requiring the MajorVersion would need to be updated to just ptr.To(payload.ParsedVersion.Major), so it would still be readable. I am more inclined towards this approach, but I am curious what you think.


// manifestHash is a hash of the manifests included in this payload
ManifestHash string
Expand Down Expand Up @@ -211,7 +213,7 @@ func LoadUpdate(dir, releaseImage, excludeIdentifier string, requiredFeatureSet
filteredMs := []manifest.Manifest{}

for _, manifest := range ms {
if err := manifest.Include(&excludeIdentifier, &requiredFeatureSet, &profile, onlyKnownCaps, nil, enabledFeatureGates); err != nil {
if err := manifest.Include(&excludeIdentifier, &requiredFeatureSet, &profile, onlyKnownCaps, nil, enabledFeatureGates, payload.MajorVersion); err != nil {
klog.V(2).Infof("excluding %s: %v\n", manifest.String(), err)
continue
}
Expand Down Expand Up @@ -243,12 +245,12 @@ func LoadUpdate(dir, releaseImage, excludeIdentifier string, requiredFeatureSet
// the current payload the updated manifest's capabilities are checked to see if any must be implicitly enabled.
// All capabilities requiring implicit enablement are returned.
func GetImplicitlyEnabledCapabilities(updatePayloadManifests []manifest.Manifest, currentPayloadManifests []manifest.Manifest,
capabilities capability.ClusterCapabilities, enabledFeatureGates sets.Set[string]) sets.Set[configv1.ClusterVersionCapability] {
capabilities capability.ClusterCapabilities, enabledFeatureGates sets.Set[string], majorVersion *uint64) sets.Set[configv1.ClusterVersionCapability] {
clusterCaps := capability.GetCapabilitiesStatus(capabilities)
return localmanifest.GetImplicitlyEnabledCapabilities(
updatePayloadManifests,
currentPayloadManifests,
localmanifest.InclusionConfiguration{Capabilities: &clusterCaps, EnabledFeatureGates: enabledFeatureGates},
localmanifest.InclusionConfiguration{Capabilities: &clusterCaps, EnabledFeatureGates: enabledFeatureGates, MajorVersion: majorVersion},
capabilities.ImplicitlyEnabled,
)
}
Expand Down Expand Up @@ -284,6 +286,11 @@ func loadPayloadMetadata(releaseDir, releaseImage string) (*Update, error) {
klog.V(2).Infof("Architecture from %s (%s) retrieved from runtime: %q", cincinnatiJSONFile, release.Version, arch)
}

parsedVersion, err := semver.Parse(release.Version)
if err != nil {
return nil, fmt.Errorf("failed to parse release version %s: %w", release.Version, err)
}

release.Image = releaseImage

imageRef, err := loadImageReferences(releaseDir)
Expand All @@ -299,6 +306,7 @@ func loadPayloadMetadata(releaseDir, releaseImage string) (*Update, error) {
Release: release,
ImageRef: imageRef,
Architecture: arch,
MajorVersion: ptr.To(parsedVersion.Major),
}, nil
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/payload/payload_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"

configv1 "github.com/openshift/api/config/v1"
imagev1 "github.com/openshift/api/image/v1"
Expand Down Expand Up @@ -66,6 +67,7 @@ func TestLoadUpdate(t *testing.T) {
},
},
Architecture: architecture,
MajorVersion: ptr.To(uint64(1)),
ManifestHash: "DL-FFQ2Uem8=",
Manifests: []manifest.Manifest{
{
Expand Down Expand Up @@ -177,6 +179,7 @@ func TestLoadUpdateArchitecture(t *testing.T) {
},
},
Architecture: "Multi",
MajorVersion: ptr.To(uint64(1)),
ManifestHash: "fvnVhLw92pE=",
Manifests: []manifest.Manifest{
{
Expand Down Expand Up @@ -365,7 +368,7 @@ func TestGetImplicitlyEnabledCapabilities(t *testing.T) {
if tt.pathExt == "test10" {
updateMans = append(updateMans, updateMans[0])
}
caps := GetImplicitlyEnabledCapabilities(updateMans, currentMans, tt.capabilities, sets.Set[string]{})
caps := GetImplicitlyEnabledCapabilities(updateMans, currentMans, tt.capabilities, sets.Set[string]{}, nil)
if diff := cmp.Diff(tt.wantImplicit, caps); diff != "" {
t.Errorf("%s: wantImplicit differs from expected:\n%s", tt.name, diff)
}
Expand Down
39 changes: 35 additions & 4 deletions pkg/payload/render.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package payload

import (
"bytes"
"encoding/json"
"fmt"
"os"
"path/filepath"
"strings"
"text/template"

"github.com/blang/semver/v4"
"github.com/openshift/api/config"
configv1 "github.com/openshift/api/config/v1"
"github.com/openshift/library-go/pkg/manifest"
Expand Down Expand Up @@ -40,6 +42,11 @@ func Render(outputDir, releaseImage, featureGateManifestPath, clusterProfile str
return fmt.Errorf("error parsing feature gate manifest: %w", err)
}

payloadVersion, err := loadReleaseVersion(releaseManifestsDir)
if err != nil {
return fmt.Errorf("error loading release version: %w", err)
}

tasks := []struct {
idir string
odir string
Expand Down Expand Up @@ -70,7 +77,7 @@ func Render(outputDir, releaseImage, featureGateManifestPath, clusterProfile str
}}
var errs []error
for _, task := range tasks {
if err := renderDir(renderConfig, task.idir, task.odir, requiredFeatureSet, enabledFeatureGates, &clusterProfile, task.processTemplate, task.skipFiles, task.filterGroupKind); err != nil {
if err := renderDir(renderConfig, task.idir, task.odir, requiredFeatureSet, enabledFeatureGates, &clusterProfile, ptr.To(payloadVersion.Major), task.processTemplate, task.skipFiles, task.filterGroupKind); err != nil {
errs = append(errs, err)
}
}
Expand All @@ -82,8 +89,8 @@ func Render(outputDir, releaseImage, featureGateManifestPath, clusterProfile str
return nil
}

func renderDir(renderConfig manifestRenderConfig, idir, odir string, requiredFeatureSet *string, enabledFeatureGates sets.Set[string], clusterProfile *string, processTemplate bool, skipFiles sets.Set[string], filterGroupKind sets.Set[schema.GroupKind]) error {
klog.Infof("Filtering manifests in %s for feature set %v, cluster profile %v and enabled feature gates %v", idir, *requiredFeatureSet, *clusterProfile, enabledFeatureGates.UnsortedList())
func renderDir(renderConfig manifestRenderConfig, idir, odir string, requiredFeatureSet *string, enabledFeatureGates sets.Set[string], clusterProfile *string, majorVersion *uint64, processTemplate bool, skipFiles sets.Set[string], filterGroupKind sets.Set[schema.GroupKind]) error {
klog.Infof("Filtering manifests in %s for feature set %v, cluster profile %v, enabled feature gates %v, and major version %v", idir, *requiredFeatureSet, *clusterProfile, enabledFeatureGates.UnsortedList(), ptr.Deref(majorVersion, 0))

if err := os.MkdirAll(odir, 0666); err != nil {
return err
Expand Down Expand Up @@ -133,7 +140,7 @@ func renderDir(renderConfig manifestRenderConfig, idir, odir string, requiredFea
for _, manifest := range manifests {
if len(filterGroupKind) > 0 && !filterGroupKind.Has(manifest.GVK.GroupKind()) {
klog.Infof("excluding %s because we do not render that group/kind", manifest.String())
} else if err := manifest.Include(nil, requiredFeatureSet, clusterProfile, nil, nil, enabledFeatureGates); err != nil {
} else if err := manifest.Include(nil, requiredFeatureSet, clusterProfile, nil, nil, enabledFeatureGates, majorVersion); err != nil {
klog.Infof("excluding %s: %v", manifest.String(), err)
} else {
filteredManifests = append(filteredManifests, string(manifest.Raw))
Expand Down Expand Up @@ -238,3 +245,27 @@ func parseFeatureGateManifest(featureGateManifestPath string) (*string, sets.Set

return requiredFeatureSet, enabledFeatureGates, nil
}

func loadReleaseVersion(releaseDir string) (semver.Version, error) {
Copy link
Contributor

@DavidHurta DavidHurta Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we start to duplicate some logic. I am curious whether we could leverage the existing loadReleaseMetadata function here instead of manually loading the release version.

The loadReleaseMetadata function also contains some additional validation, such as:

	if metadata.Kind != "cincinnati-metadata-v0" {
		return release, fmt.Errorf("unrecognized Cincinnati metadata kind %q", metadata.Kind)
	}

Copy link
Contributor

@DavidHurta DavidHurta Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like:

func loadReleaseVersion(releaseDir string) (semver.Version, error) {
	release, err := loadReleaseMetadata(releaseDir)
	if err != nil {
		return semver.Version{}, err
	}

	parsedVersion, err := semver.Parse(release.Version)
	if err != nil {
		return semver.Version{}, err
	}
	return parsedVersion, nil
}

path := filepath.Join(releaseDir, cincinnatiJSONFile)
data, err := os.ReadFile(path)
if err != nil {
return semver.Version{}, err
}

var metadata metadata
if err := json.Unmarshal(data, &metadata); err != nil {
return semver.Version{}, fmt.Errorf("unmarshal Cincinnati metadata: %w", err)
}

if metadata.Version == "" {
return semver.Version{}, errors.New("missing required Cincinnati metadata version")
}

parsedVersion, err := semver.Parse(metadata.Version)
if err != nil {
return semver.Version{}, fmt.Errorf("Cincinnati metadata version %q is not a valid semantic version: %v", metadata.Version, err)
}

return parsedVersion, nil
}
Loading