From 2cc50212d1d73538708e8059077546406a102d4b Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Tue, 27 Jan 2026 09:55:21 +0800 Subject: [PATCH] Add a unit test to guide troubleshooting The failures in CI caused by a new feature set in o/api are not easy to troubleshoot. This pull adds a unit test it gives hint of what to check and a potential fix. Hopefully this will reduce the time to troubleshoot the case. I believe it will pay off the time we spend on maintaining the set `cvoSkipFeatureSets`. Recently I have seen a similar thing in `o/oc` [1]. [1]. https://github.com/openshift/oc/blob/12e2f59e4bac8586da8ab10bfb243862ff0444f5/pkg/helpers/describe/describer_test.go#L112 --- pkg/payload/render.go | 24 ++++++++++++++++-------- pkg/payload/render_test.go | 21 +++++++++++++++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/pkg/payload/render.go b/pkg/payload/render.go index 03c6257ef1..57aba98cb7 100644 --- a/pkg/payload/render.go +++ b/pkg/payload/render.go @@ -126,14 +126,7 @@ func renderDir(renderConfig manifestRenderConfig, idir, odir string, requiredFea if skipFiles.Has(file.Name()) { continue } - if strings.Contains(file.Name(), "CustomNoUpgrade") || - strings.Contains(file.Name(), "TechPreviewNoUpgrade") || - strings.Contains(file.Name(), "DevPreviewNoUpgrade") || - strings.Contains(file.Name(), "OKD") { - // CustomNoUpgrade, TechPreviewNoUpgrade, DevPreviewNoUpgrade, and OKD may add features to manifests like the ClusterVersion CRD, - // but we do not need those features during bootstrap-render time. In those clusters, the production - // CVO will be along shortly to update the manifests and deliver the gated features. - // fixme: now that we have requiredFeatureSet, use it to do Manifest.Include() filtering here instead of making filename assumptions + if skipFeatureSets(file.Name()) { continue } @@ -199,6 +192,21 @@ func renderDir(renderConfig manifestRenderConfig, idir, odir string, requiredFea return nil } +// CustomNoUpgrade, TechPreviewNoUpgrade, DevPreviewNoUpgrade, and OKD may add features to manifests like the ClusterVersion CRD, +// but we do not need those features during bootstrap-render time. In those clusters, the production +// CVO will be along shortly to update the manifests and deliver the gated features. +// fixme: now that we have requiredFeatureSet, use it to do Manifest.Include() filtering here instead of making filename assumptions +var cvoSkipFeatureSets = sets.New[string]("CustomNoUpgrade", "TechPreviewNoUpgrade", "DevPreviewNoUpgrade", "OKD") + +func skipFeatureSets(filename string) bool { + for featureSet := range cvoSkipFeatureSets { + if strings.HasPrefix(filename, featureSet) { + return true + } + } + return false +} + type manifestRenderConfig struct { ReleaseImage string ClusterProfile string diff --git a/pkg/payload/render_test.go b/pkg/payload/render_test.go index efaafc427a..9b1257ff5f 100644 --- a/pkg/payload/render_test.go +++ b/pkg/payload/render_test.go @@ -6,6 +6,8 @@ import ( "testing" "github.com/google/go-cmp/cmp" + configv1 "github.com/openshift/api/config/v1" + "k8s.io/apimachinery/pkg/util/sets" ) func TestRenderManifest(t *testing.T) { @@ -58,3 +60,22 @@ func TestRenderManifest(t *testing.T) { }) } } + +func Test_cvoKnownFeatureSets(t *testing.T) { + unknown := sets.New[string]() + known := cvoSkipFeatureSets.Clone().Insert(string(configv1.Default)) + for _, fs := range append(configv1.AllFixedFeatureSets, configv1.CustomNoUpgrade) { + candidate := string(fs) + if !known.Has(candidate) { + unknown.Insert(candidate) + } + } + if unknown.Len() != 0 { + t.Errorf("found unknown feature sets [%s] not recognized by CVO. If it is a result of bump o/api e.g., "+ + "https://github.com/openshift/cluster-version-operator/pull/1302/commits/dd3b8335f7f443b5ab6ffcfd78236832cb922753 "+ + "and it led to failing CVO/Hypershift e2e tests, please try to fix them by pulls like "+ + "https://github.com/openshift/hypershift/pull/7557 and "+ + "https://github.com/openshift/cluster-version-operator/pull/1302/commits/981361e85b58ec77dada253e63f79f8ef78a8bd1. "+ + "Otherwise, adding them to cvoSkipFeatureSets fixes this unit test", strings.Join(unknown.UnsortedList(), ", ")) + } +}