From 57e9293796dc11dcc8de23fa47b1f7386a376d39 Mon Sep 17 00:00:00 2001 From: Forrest Babcock <36795216+neisw@users.noreply.github.com> Date: Sun, 15 Feb 2026 20:10:12 -0500 Subject: [PATCH] Revert "OTA-1546: Add a serial e2e for accept-risks" --- ...hift_payload_cluster-version-operator.json | 12 -- cmd/cluster-version-operator-tests/main.go | 4 +- lib/resourcebuilder/apps.go | 5 +- pkg/cvo/sync_worker.go | 3 +- pkg/external/constants.go | 21 --- pkg/internal/constants.go | 9 -- pkg/start/start.go | 7 +- test/cvo/accept_risks.go | 140 ------------------ test/cvo/cvo.go | 15 +- test/util/util.go | 28 ---- 10 files changed, 17 insertions(+), 227 deletions(-) delete mode 100644 pkg/external/constants.go delete mode 100644 test/cvo/accept_risks.go diff --git a/.openshift-tests-extension/openshift_payload_cluster-version-operator.json b/.openshift-tests-extension/openshift_payload_cluster-version-operator.json index b7eadcb0b3..6f6e9dac86 100644 --- a/.openshift-tests-extension/openshift_payload_cluster-version-operator.json +++ b/.openshift-tests-extension/openshift_payload_cluster-version-operator.json @@ -1,16 +1,4 @@ [ - { - "name": "[Jira:\"Cluster Version Operator\"] cluster-version-operator should work with accept risks", - "labels": { - "Serial": {} - }, - "resources": { - "isolation": {} - }, - "source": "openshift:payload:cluster-version-operator", - "lifecycle": "blocking", - "environmentSelector": {} - }, { "name": "[Jira:\"Cluster Version Operator\"] cluster-version-operator-tests should support passing tests", "labels": {}, diff --git a/cmd/cluster-version-operator-tests/main.go b/cmd/cluster-version-operator-tests/main.go index a2d00dd637..cb3cf29ff5 100644 --- a/cmd/cluster-version-operator-tests/main.go +++ b/cmd/cluster-version-operator-tests/main.go @@ -24,7 +24,7 @@ func main() { Name: "openshift/cluster-version-operator/conformance/parallel", Parents: []string{"openshift/conformance/parallel"}, Qualifiers: []string{ - `!(name.contains("[Serial]") || "Serial" in labels || name.contains("[Slow]"))`, + `!(name.contains("[Serial]") || name.contains("[Slow]"))`, }, }) @@ -33,7 +33,7 @@ func main() { Name: "openshift/cluster-version-operator/conformance/serial", Parents: []string{"openshift/conformance/serial"}, Qualifiers: []string{ - `name.contains("[Serial]") || "Serial" in labels`, + `name.contains("[Serial]")`, }, }) diff --git a/lib/resourcebuilder/apps.go b/lib/resourcebuilder/apps.go index 6d05f41a1f..c9863725bc 100644 --- a/lib/resourcebuilder/apps.go +++ b/lib/resourcebuilder/apps.go @@ -13,7 +13,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog/v2" - "github.com/openshift/cluster-version-operator/pkg/external" "github.com/openshift/cluster-version-operator/pkg/payload" ) @@ -40,7 +39,7 @@ func (b *builder) modifyDeployment(ctx context.Context, deployment *appsv1.Deplo // if we detect the CVO deployment we need to replace the KUBERNETES_SERVICE_HOST env var with the internal load // balancer to be resilient to kube-apiserver rollouts that cause the localhost server to become non-responsive for // multiple minutes. - if deployment.Namespace == external.DefaultCVONamespace && deployment.Name == external.DefaultDeploymentName { + if deployment.Namespace == "openshift-cluster-version" && deployment.Name == "cluster-version-operator" { infrastructureConfig, err := b.configClientv1.Infrastructures().Get(ctx, "cluster", metav1.GetOptions{}) // not found just means that we don't have infrastructure configuration yet, so we should tolerate not found and avoid substitution if err != nil && !errors.IsNotFound(err) { @@ -64,7 +63,7 @@ func (b *builder) modifyDeployment(ctx context.Context, deployment *appsv1.Deplo } err = updatePodSpecWithInternalLoadBalancerKubeService( &deployment.Spec.Template.Spec, - []string{external.DefaultContainerName}, + []string{"cluster-version-operator"}, lbHost, lbPort, ) diff --git a/pkg/cvo/sync_worker.go b/pkg/cvo/sync_worker.go index 3833a9c4a0..8c94b1917f 100644 --- a/pkg/cvo/sync_worker.go +++ b/pkg/cvo/sync_worker.go @@ -26,7 +26,6 @@ import ( configv1 "github.com/openshift/api/config/v1" "github.com/openshift/cluster-version-operator/lib/capability" - "github.com/openshift/cluster-version-operator/pkg/internal" "github.com/openshift/cluster-version-operator/pkg/payload" "github.com/openshift/cluster-version-operator/pkg/payload/precondition" ) @@ -330,7 +329,7 @@ func (w *SyncWorker) syncPayload(ctx context.Context, work *SyncWork) ([]configv // The remainder of this logic is for loading a new payload. // Any filtering as to not needing to reload the payload should be done in the switch before this point. - cvoObjectRef := &corev1.ObjectReference{APIVersion: "config.openshift.io/v1", Kind: "ClusterVersion", Name: internal.DefaultClusterVersionName, Namespace: internal.DefaultCVONamespace} + cvoObjectRef := &corev1.ObjectReference{APIVersion: "config.openshift.io/v1", Kind: "ClusterVersion", Name: "version", Namespace: "openshift-cluster-version"} msg := fmt.Sprintf("Retrieving and verifying payload version=%q image=%q", desired.Version, desired.Image) w.eventRecorder.Eventf(cvoObjectRef, corev1.EventTypeNormal, "RetrievePayload", msg) reporter.ReportPayload(LoadPayloadStatus{ diff --git a/pkg/external/constants.go b/pkg/external/constants.go deleted file mode 100644 index 4cbdd95e67..0000000000 --- a/pkg/external/constants.go +++ /dev/null @@ -1,21 +0,0 @@ -package external - -import "github.com/openshift/cluster-version-operator/pkg/internal" - -// The constants defined here are used by the components, e.g., e2e tests that have no access -// to github.com/openshift/cluster-version-operator/pkg/internal -// See https://pkg.go.dev/cmd/go#hdr-Internal_Directories -const ( - // DefaultCVONamespace is the default namespace for the Cluster Version Operator - DefaultCVONamespace = internal.DefaultCVONamespace - // DefaultClusterVersionName is the default name for the Cluster Version resource managed by the Cluster Version Operator - DefaultClusterVersionName = internal.DefaultClusterVersionName - // DefaultDeploymentName is the default name of the deployment for the Cluster Version Operator - DefaultDeploymentName = internal.DefaultDeploymentName - // DefaultContainerName is the default container name in the deployment for the Cluster Version Operator - DefaultContainerName = internal.DefaultContainerName - - // ConditionalUpdateConditionTypeRecommended is a type of the condition present on a conditional update - // that indicates whether the conditional update is recommended or not - ConditionalUpdateConditionTypeRecommended = internal.ConditionalUpdateConditionTypeRecommended -) diff --git a/pkg/internal/constants.go b/pkg/internal/constants.go index 3f5fa7250c..bce432ebc9 100644 --- a/pkg/internal/constants.go +++ b/pkg/internal/constants.go @@ -7,15 +7,6 @@ import ( ) const ( - // DefaultCVONamespace is the default namespace for the Cluster Version Operator - DefaultCVONamespace = "openshift-cluster-version" - // DefaultClusterVersionName is the default name for the Cluster Version resource managed by the Cluster Version Operator - DefaultClusterVersionName = "version" - // DefaultDeploymentName is the default name of the deployment for the Cluster Version Operator - DefaultDeploymentName = "cluster-version-operator" - // DefaultContainerName is the default container name in the deployment for the Cluster Version Operator - DefaultContainerName = DefaultDeploymentName - ConfigNamespace = "openshift-config" ConfigManagedNamespace = "openshift-config-managed" AdminGatesConfigMap = "admin-gates" diff --git a/pkg/start/start.go b/pkg/start/start.go index ea734b1b82..2a880d82cc 100644 --- a/pkg/start/start.go +++ b/pkg/start/start.go @@ -49,6 +49,9 @@ import ( ) const ( + defaultComponentName = "version" + defaultComponentNamespace = "openshift-cluster-version" + minResyncPeriod = 2 * time.Minute ) @@ -119,8 +122,8 @@ func NewOptions() *Options { PromQLTarget: defaultPromQLTarget, // exposed only for testing - Namespace: defaultEnv("CVO_NAMESPACE", internal.DefaultCVONamespace), - Name: defaultEnv("CVO_NAME", internal.DefaultClusterVersionName), + Namespace: defaultEnv("CVO_NAMESPACE", defaultComponentNamespace), + Name: defaultEnv("CVO_NAME", defaultComponentName), PayloadOverride: os.Getenv("PAYLOAD_OVERRIDE"), ResyncInterval: minResyncPeriod, Exclude: os.Getenv("EXCLUDE_MANIFESTS"), diff --git a/test/cvo/accept_risks.go b/test/cvo/accept_risks.go deleted file mode 100644 index d0916e01c9..0000000000 --- a/test/cvo/accept_risks.go +++ /dev/null @@ -1,140 +0,0 @@ -package cvo - -import ( - "context" - "time" - - g "github.com/onsi/ginkgo/v2" - o "github.com/onsi/gomega" - - configv1 "github.com/openshift/api/config/v1" - configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/client-go/rest" - - "github.com/openshift/cluster-version-operator/pkg/external" - "github.com/openshift/cluster-version-operator/test/util" -) - -var _ = g.Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator`, func() { - - var ( - c *rest.Config - configClient *configv1client.ConfigV1Client - err error - - ctx = context.TODO() - backup configv1.ClusterVersionSpec - ) - - g.BeforeEach(func() { - c, err = util.GetRestConfig() - o.Expect(err).To(o.BeNil()) - configClient, err = configv1client.NewForConfig(c) - o.Expect(err).To(o.BeNil()) - - util.SkipIfNotTechPreviewNoUpgrade(ctx, c) - o.Expect(util.SkipIfHypershift(ctx, c)).To(o.BeNil()) - o.Expect(util.SkipIfMicroshift(ctx, c)).To(o.BeNil()) - - cv, err := configClient.ClusterVersions().Get(ctx, external.DefaultClusterVersionName, metav1.GetOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) - if du := cv.Spec.DesiredUpdate; du != nil { - logger.WithValues("AcceptRisks", du.AcceptRisks).Info("Accept risks before testing") - o.Expect(du.AcceptRisks).To(o.BeEmpty(), "found accept risks") - } - backup = *cv.Spec.DeepCopy() - }) - - g.AfterEach(func() { - cv, err := configClient.ClusterVersions().Get(ctx, external.DefaultClusterVersionName, metav1.GetOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) - cv.Spec = backup - _, err = configClient.ClusterVersions().Update(ctx, cv, metav1.UpdateOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) - }) - - g.It("should work with accept risks", g.Label("Serial"), func() { - cv, err := configClient.ClusterVersions().Get(ctx, external.DefaultClusterVersionName, metav1.GetOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) - - g.By("Using fauxinnati as the upstream and its risks-always channel") - cv.Spec.Upstream = util.FauxinnatiAPIURL - cv.Spec.Channel = "risks-always" - - _, err = configClient.ClusterVersions().Update(ctx, cv, metav1.UpdateOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) - - g.By("Checking that conditional updates shows up in status") - // waiting for the conditional updates to show up - o.Expect(wait.PollUntilContextTimeout(ctx, 30*time.Second, 5*time.Minute, true, func(ctx context.Context) (done bool, err error) { - cv, err = configClient.ClusterVersions().Get(ctx, external.DefaultClusterVersionName, metav1.GetOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) - if len(cv.Status.ConditionalUpdates) == 0 { - return false, nil - } - return true, nil - })).NotTo(o.HaveOccurred(), "no conditional updates found in status") - - g.By("Checking that no conditional updates are recommended") - conditionalUpdatesLength := len(cv.Status.ConditionalUpdates) - var acceptRisks []configv1.AcceptRisk - var releases []configv1.Release - names := sets.New[string]() - for _, cu := range cv.Status.ConditionalUpdates { - releases = append(releases, cu.Release) - o.Expect(cu.RiskNames).NotTo(o.BeEmpty()) - for _, name := range cu.RiskNames { - if names.Has(name) { - continue - } - names.Insert(name) - acceptRisks = append(acceptRisks, configv1.AcceptRisk{Name: name}) - } - o.Expect(cu.Risks).NotTo(o.BeEmpty()) - recommendedCondition := meta.FindStatusCondition(cu.Conditions, external.ConditionalUpdateConditionTypeRecommended) - o.Expect(recommendedCondition).NotTo(o.BeNil()) - o.Expect(recommendedCondition.Status).To(o.Equal(metav1.ConditionFalse)) - } - - g.By("Accepting all risks") - o.Expect(acceptRisks).NotTo(o.BeEmpty()) - if cv.Spec.DesiredUpdate == nil { - cv.Spec.DesiredUpdate = &configv1.Update{ - Image: cv.Status.Desired.Image, - Version: cv.Status.Desired.Version, - Architecture: cv.Status.Desired.Architecture, - } - } - cv.Spec.DesiredUpdate.AcceptRisks = acceptRisks - - _, err = configClient.ClusterVersions().Update(ctx, cv, metav1.UpdateOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) - - g.By("Checking that all conditional updates are recommended") - var releasesNow []configv1.Release - // waiting for the conditional updates to be refreshed - o.Expect(wait.PollUntilContextTimeout(ctx, 30*time.Second, 5*time.Minute, true, func(ctx context.Context) (done bool, err error) { - cv, err = configClient.ClusterVersions().Get(ctx, external.DefaultClusterVersionName, metav1.GetOptions{}) - o.Expect(err).NotTo(o.HaveOccurred()) - o.Expect(cv.Status.ConditionalUpdates).NotTo(o.BeEmpty()) - releasesNow = nil - for _, cu := range cv.Status.ConditionalUpdates { - releasesNow = append(releasesNow, cu.Release) - recommendedCondition := meta.FindStatusCondition(cu.Conditions, external.ConditionalUpdateConditionTypeRecommended) - o.Expect(recommendedCondition).NotTo(o.BeNil()) - if recommendedCondition.Status != metav1.ConditionTrue { - return false, nil - } - } - return true, nil - })).NotTo(o.HaveOccurred(), "no conditional updates are recommended in status after accepting risks") - - o.Expect(cv.Spec.DesiredUpdate.AcceptRisks).To(o.Equal(acceptRisks)) - o.Expect(cv.Status.ConditionalUpdates).To(o.HaveLen(conditionalUpdatesLength)) - o.Expect(releasesNow).To(o.Equal(releases)) - }) -}) diff --git a/test/cvo/cvo.go b/test/cvo/cvo.go index ebdc9f0d06..230383e807 100644 --- a/test/cvo/cvo.go +++ b/test/cvo/cvo.go @@ -1,5 +1,3 @@ -// Package cvo contains end-to-end tests for the Cluster Version Operator. -// The accept_risks test validates the feature about accept risks for conditional updates. package cvo import ( @@ -12,7 +10,6 @@ import ( g "github.com/onsi/ginkgo/v2" o "github.com/onsi/gomega" - "github.com/openshift/cluster-version-operator/pkg/external" "github.com/openshift/cluster-version-operator/test/oc" ocapi "github.com/openshift/cluster-version-operator/test/oc/api" "github.com/openshift/cluster-version-operator/test/util" @@ -20,6 +17,8 @@ import ( var logger = g.GinkgoLogr.WithName("cluster-version-operator-tests") +const cvoNamespace = "openshift-cluster-version" + var _ = g.Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator-tests`, func() { g.It("should support passing tests", func() { o.Expect(true).To(o.BeTrue()) @@ -67,14 +66,14 @@ var _ = g.Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator`, o.Expect(err).NotTo(o.HaveOccurred(), "Failed to determine if cluster is MicroShift") g.By("Checking that the 'openshift.io/run-level' label exists on the namespace and has the empty value") - ns, err := kubeClient.CoreV1().Namespaces().Get(ctx, external.DefaultCVONamespace, metav1.GetOptions{}) - o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get namespace %s", external.DefaultCVONamespace) + ns, err := kubeClient.CoreV1().Namespaces().Get(ctx, cvoNamespace, metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get namespace %s", cvoNamespace) runLevel, exists := ns.Labels["openshift.io/run-level"] - o.Expect(exists).To(o.BeTrue(), "The 'openshift.io/run-level' label on namespace %s does not exist", external.DefaultCVONamespace) - o.Expect(runLevel).To(o.BeEmpty(), "Expected the 'openshift.io/run-level' label value on namespace %s has the empty value, but got %s", external.DefaultCVONamespace, runLevel) + o.Expect(exists).To(o.BeTrue(), "The 'openshift.io/run-level' label on namespace %s does not exist", cvoNamespace) + o.Expect(runLevel).To(o.BeEmpty(), "Expected the 'openshift.io/run-level' label value on namespace %s has the empty value, but got %s", cvoNamespace, runLevel) g.By("Checking that the annotation 'openshift.io/scc annotation' on the CVO pod has the value hostaccess") - podList, err := kubeClient.CoreV1().Pods(external.DefaultCVONamespace).List(ctx, metav1.ListOptions{ + podList, err := kubeClient.CoreV1().Pods(cvoNamespace).List(ctx, metav1.ListOptions{ LabelSelector: "k8s-app=cluster-version-operator", FieldSelector: "status.phase=Running", }) diff --git a/test/util/util.go b/test/util/util.go index 397d5c9926..4d7319a766 100644 --- a/test/util/util.go +++ b/test/util/util.go @@ -5,8 +5,6 @@ import ( "time" g "github.com/onsi/ginkgo/v2" - o "github.com/onsi/gomega" - configv1 "github.com/openshift/api/config/v1" clientconfigv1 "github.com/openshift/client-go/config/clientset/versioned" corev1 "k8s.io/api/core/v1" @@ -108,29 +106,3 @@ func GetKubeClient(restConfig *rest.Config) (kubernetes.Interface, error) { func GetConfigClient(restConfig *rest.Config) (clientconfigv1.Interface, error) { return clientconfigv1.NewForConfig(restConfig) } - -// IsTechPreviewNoUpgrade checks if a cluster is a TechPreviewNoUpgrade cluster -func IsTechPreviewNoUpgrade(ctx context.Context, restConfig *rest.Config) bool { - configClient, err := GetConfigClient(restConfig) - o.Expect(err).NotTo(o.HaveOccurred()) - featureGate, err := configClient.ConfigV1().FeatureGates().Get(ctx, "cluster", metav1.GetOptions{}) - if err != nil { - if apierrors.IsNotFound(err) { - return false - } - o.Expect(err).NotTo(o.HaveOccurred(), "could not retrieve feature-gate: %v", err) - } - return featureGate.Spec.FeatureSet == configv1.TechPreviewNoUpgrade -} - -// SkipIfNotTechPreviewNoUpgrade skips the test if a cluster is not a TechPreviewNoUpgrade cluster -func SkipIfNotTechPreviewNoUpgrade(ctx context.Context, restConfig *rest.Config) { - if !IsTechPreviewNoUpgrade(ctx, restConfig) { - g.Skip("This test is skipped because the Tech Preview NoUpgrade is not enabled") - } -} - -const ( - // fauxinnati mocks Cincinnati Update Graph Server for OpenShift - FauxinnatiAPIURL = "https://fauxinnati-fauxinnati.apps.ota-stage.q2z4.p1.openshiftapps.com/api/upgrades_info/graph" -)