From 28b8a37b7c774259944f60bfce99bcd62bc32962 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Fri, 24 Apr 2026 10:28:11 -0400 Subject: [PATCH 1/6] Move over a few functions from cvo#1379 --- pkg/proposal/controller.go | 121 ++++++++++++++++++++++++++++++++ pkg/proposal/controller_test.go | 119 +++++++++++++++++++++++++++++++ 2 files changed, 240 insertions(+) diff --git a/pkg/proposal/controller.go b/pkg/proposal/controller.go index 4a803cfa9..f7ac04cc0 100644 --- a/pkg/proposal/controller.go +++ b/pkg/proposal/controller.go @@ -3,8 +3,11 @@ package proposal import ( "context" "fmt" + "os" + "strings" "time" + "github.com/blang/semver/v4" ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -53,6 +56,29 @@ func NewController(updatesGetterFunc UpdatesGetterFunc, client ctrlruntimeclient } } +// Config holds configuration for proposal creation. +type Config struct { + Namespace string + Workflow string + PromptConfigMap string // ConfigMap name containing the system prompt +} + +// DefaultConfig returns the default configuration, checking env vars for overrides. +func DefaultConfig() Config { + return Config{ + Namespace: envOrDefault("LIGHTSPEED_PROPOSAL_NAMESPACE", "openshift-lightspeed"), + Workflow: envOrDefault("LIGHTSPEED_PROPOSAL_WORKFLOW", "ota-advisory"), + PromptConfigMap: envOrDefault("LIGHTSPEED_PROMPT_CONFIGMAP", "ota-advisory-prompt"), + } +} + +func envOrDefault(key, def string) string { + if v := os.Getenv(key); v != "" { + return v + } + return def +} + func (c *Controller) Queue() workqueue.TypedRateLimitingInterface[any] { return c.queue } @@ -187,3 +213,98 @@ func getProposals(_ []configv1.Release, _ []configv1.ConditionalUpdate) []*propo }, } } + +// proposalName generates a deterministic proposal name from the version pair. +func proposalName(current, target string) string { + return fmt.Sprintf("ota-%s-to-%s", sanitize(current), sanitize(target)) +} + +// sanitize converts a version string into a valid DNS-1035 label component. +// DNS-1035 requires: lowercase alphanumeric or '-', start with alpha, end with alphanum. +func sanitize(s string) string { + s = strings.ToLower(s) + s = strings.ReplaceAll(s, ".", "-") + s = strings.ReplaceAll(s, " ", "-") + if len(s) > 20 { + s = s[:20] + } + return strings.TrimRight(s, "-") +} + +const ( + updateKindRecommended = "recommended" + updateKindConditional = "conditional" + + updateTypeZStream = "z-stream" + updateTypeMinor = "minor" + updateTypeUnknown = "unknown" +) + +// classifyUpdate returns "z-stream" if major.minor match, otherwise "minor". +func classifyUpdate(current, target string) string { + cv, cerr := semver.Parse(current) + tv, terr := semver.Parse(target) + if cerr != nil || terr != nil { + return updateTypeUnknown + } + if cv.Major == tv.Major && cv.Minor == tv.Minor { + return updateTypeZStream + } + return updateTypeMinor +} + +// buildRequest constructs the proposal request with system prompt, metadata, and readiness data. +func buildRequest(systemPrompt, current, target, channel, updateType, targetType string, + updates []configv1.Release, readinessJSON string) string { + + var b strings.Builder + + if systemPrompt != "" { + b.WriteString(systemPrompt) + b.WriteString("\n\n---\n\n") + } + + fmt.Fprintf(&b, "Current version: OCP %s\n", current) + fmt.Fprintf(&b, "Target version: OCP %s\n", target) + fmt.Fprintf(&b, "Channel: %s\n", channel) + fmt.Fprintf(&b, "Update type: %s\n", updateType) + fmt.Fprintf(&b, "Update path: %s\n\n", targetType) + + if targetType == updateKindConditional { + b.WriteString("WARNING: This target version is available as a CONDITIONAL update.\n") + b.WriteString("OSUS has flagged known risks that may apply to this cluster.\n") + b.WriteString("The assessment MUST evaluate each conditional risk against cluster state.\n\n") + } + + if len(updates) > 1 { + b.WriteString("Other recommended versions available:\n") + count := 0 + for _, u := range updates { + if u.Version != target { + if u.URL != "" { + fmt.Fprintf(&b, " - %s (errata: %s)\n", u.Version, u.URL) + } else { + fmt.Fprintf(&b, " - %s\n", u.Version) + } + count++ + if count >= 5 { + remaining := len(updates) - count - 1 + if remaining > 0 { + fmt.Fprintf(&b, " ... and %d more\n", remaining) + } + break + } + } + } + b.WriteString("\n") + } + + if readinessJSON != "" { + b.WriteString("## Cluster Readiness Data\n\n") + b.WriteString("```json\n") + b.WriteString(readinessJSON) + b.WriteString("\n```\n") + } + + return b.String() +} diff --git a/pkg/proposal/controller_test.go b/pkg/proposal/controller_test.go index 24e27b6e4..db96a8c0c 100644 --- a/pkg/proposal/controller_test.go +++ b/pkg/proposal/controller_test.go @@ -3,6 +3,7 @@ package proposal import ( "context" "fmt" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -72,3 +73,121 @@ func TestController_Sync(t *testing.T) { }) } } + +func TestClassifyUpdate(t *testing.T) { + tests := []struct { + name string + current string + target string + expected string + }{ + {name: "z-stream", current: "4.15.1", target: "4.15.3", expected: "z-stream"}, + {name: "minor", current: "4.15.1", target: "4.16.0", expected: "minor"}, + {name: "major", current: "4.15.1", target: "5.0.0", expected: "minor"}, + {name: "invalid current", current: "bad", target: "4.15.0", expected: "unknown"}, + {name: "invalid target", current: "4.15.0", target: "bad", expected: "unknown"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := classifyUpdate(tt.current, tt.target) + if got != tt.expected { + t.Errorf("classifyUpdate(%q, %q) = %q, want %q", tt.current, tt.target, got, tt.expected) + } + }) + } +} + +func TestProposalName(t *testing.T) { + tests := []struct { + current string + target string + expected string + }{ + {"4.15.1", "4.15.3", "ota-4-15-1-to-4-15-3"}, + {"4.15.1", "4.16.0", "ota-4-15-1-to-4-16-0"}, + } + + for _, tt := range tests { + t.Run(tt.current+"->"+tt.target, func(t *testing.T) { + got := proposalName(tt.current, tt.target) + if got != tt.expected { + t.Errorf("proposalName(%q, %q) = %q, want %q", tt.current, tt.target, got, tt.expected) + } + }) + } +} + +func TestSanitize(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {"4.15.1", "4-15-1"}, + {"Hello World", "hello-world"}, + {"a-very-long-version-string-that-is-too-long", "a-very-long-version"}, + {"trailing-dot.", "trailing-dot"}, + {"trailing-dash-", "trailing-dash"}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + got := sanitize(tt.input) + if got != tt.expected { + t.Errorf("sanitize(%q) = %q, want %q", tt.input, got, tt.expected) + } + }) + } +} + +func TestBuildRequest(t *testing.T) { + updates := []configv1.Release{ + {Version: "4.16.0", URL: "https://example.com/errata/1"}, + {Version: "4.16.1", URL: "https://example.com/errata/2"}, + } + + t.Run("recommended target", func(t *testing.T) { + request := buildRequest("", "4.15.3", "4.16.0", "stable-4.16", "minor", "recommended", updates, "") + if !strings.Contains(request, "Current version: OCP 4.15.3") { + t.Error("request should contain current version") + } + if !strings.Contains(request, "Target version: OCP 4.16.0") { + t.Error("request should contain target version") + } + if !strings.Contains(request, "Update type: minor") { + t.Error("request should contain update type") + } + if !strings.Contains(request, "Update path: recommended") { + t.Error("request should contain update path") + } + if strings.Contains(request, "WARNING") { + t.Error("recommended target should not have warning") + } + if !strings.Contains(request, "Other recommended versions available:") { + t.Error("should list other versions when more than one update") + } + if !strings.Contains(request, "4.16.1") { + t.Error("should list alternative version") + } + }) + + t.Run("conditional target", func(t *testing.T) { + request := buildRequest("", "4.15.3", "4.16.0", "stable-4.16", "minor", "conditional", updates, "") + if !strings.Contains(request, "WARNING") { + t.Error("conditional target should have warning") + } + if !strings.Contains(request, "CONDITIONAL update") { + t.Error("conditional target should mention CONDITIONAL") + } + }) + + t.Run("readiness JSON embedded", func(t *testing.T) { + request := buildRequest("", "4.15.3", "4.16.0", "stable-4.16", "minor", "recommended", updates, `{"checks":{},"meta":{}}`) + if !strings.Contains(request, "## Cluster Readiness Data") { + t.Error("request should contain readiness data header") + } + if !strings.Contains(request, `{"checks":{},"meta":{}}`) { + t.Error("request should contain readiness JSON") + } + }) +} From fa69062f490d84ba594b2f5f33da41498005591c Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Fri, 24 Apr 2026 11:50:39 -0400 Subject: [PATCH 2/6] Manage proposals - Create proposals for each direct update path. - Clean up the proposals if needed. The image used in Proposals is `quay.io/openshift/ci:ocp_5.0_agentic-skills`. It is built in CI. Prow jobs have the pull credentials to access it. For manual testing, https://docs.ci.openshift.org/how-tos/use-registries-in-build-farm/#how-do-i-gain-access-to-qci provides how to retrieve it. ReadinessJson will be implemented with another pull. --- ...hift_payload_cluster-version-operator.json | 1 + pkg/cvo/availableupdates_test.go | 6 + pkg/cvo/cvo.go | 24 +- pkg/cvo/cvo_test.go | 4 + pkg/internal/constants.go | 23 + .../clusterversion/upgradeable.go | 7 +- pkg/proposal/controller.go | 374 ++++-- pkg/proposal/controller_test.go | 1082 ++++++++++++++++- test/cvo/accept_risks.go | 11 +- test/cvo/proposal.go | 34 +- 10 files changed, 1426 insertions(+), 140 deletions(-) diff --git a/.openshift-tests-extension/openshift_payload_cluster-version-operator.json b/.openshift-tests-extension/openshift_payload_cluster-version-operator.json index 9fecf93c2..928c2adf6 100644 --- a/.openshift-tests-extension/openshift_payload_cluster-version-operator.json +++ b/.openshift-tests-extension/openshift_payload_cluster-version-operator.json @@ -101,6 +101,7 @@ "name": "[Jira:\"Cluster Version Operator\"] cluster-version-operator should create proposals", "labels": { "Lifecycle:informing": {}, + "OTA-1966": {}, "Serial": {} }, "resources": { diff --git a/pkg/cvo/availableupdates_test.go b/pkg/cvo/availableupdates_test.go index 71a291f66..aec0c29ae 100644 --- a/pkg/cvo/availableupdates_test.go +++ b/pkg/cvo/availableupdates_test.go @@ -211,6 +211,12 @@ func newOperator(url string, cluster release, promqlMock clusterconditions.Condi fake.NewClientBuilder().Build(), func(_ string) (*configv1.ClusterVersion, error) { return &configv1.ClusterVersion{}, nil }, + func(_ context.Context, namespace, name string, _ metav1.GetOptions) (*corev1.ConfigMap, error) { + return &corev1.ConfigMap{}, nil + }, + func() string { + return operator.release.Version + }, ) return availableUpdates, operator } diff --git a/pkg/cvo/cvo.go b/pkg/cvo/cvo.go index b52491d77..248f4bd0d 100644 --- a/pkg/cvo/cvo.go +++ b/pkg/cvo/cvo.go @@ -324,13 +324,23 @@ func New( optr.configuration = configuration.NewClusterVersionOperatorConfiguration(operatorClient, operatorInformerFactory) - optr.proposalController = proposal.NewController(func() ([]configv1.Release, []configv1.ConditionalUpdate, error) { - availableUpdates := optr.getAvailableUpdates() - if availableUpdates == nil { - return nil, nil, nil - } - return availableUpdates.Updates, availableUpdates.ConditionalUpdates, nil - }, rtClient, cvInformer.Lister().Get) + optr.proposalController = proposal.NewController( + func() ([]configv1.Release, []configv1.ConditionalUpdate, error) { + availableUpdates := optr.getAvailableUpdates() + if availableUpdates == nil { + return nil, nil, nil + } + return availableUpdates.Updates, availableUpdates.ConditionalUpdates, nil + }, + rtClient, + cvInformer.Lister().Get, + func(ctx context.Context, namespace, name string, opts metav1.GetOptions) (*corev1.ConfigMap, error) { + return kubeClient.CoreV1().ConfigMaps(namespace).Get(ctx, name, opts) + }, + func() string { + return optr.release.Version + }, + ) return optr, nil } diff --git a/pkg/cvo/cvo_test.go b/pkg/cvo/cvo_test.go index 92585a1e4..ab3ff4e43 100644 --- a/pkg/cvo/cvo_test.go +++ b/pkg/cvo/cvo_test.go @@ -2764,6 +2764,10 @@ func TestOperator_availableUpdatesSync(t *testing.T) { return nil, nil, nil }, ctrlruntimefake.NewClientBuilder().Build(), func(_ string) (*configv1.ClusterVersion, error) { return &configv1.ClusterVersion{}, nil + }, func(_ context.Context, namespace, name string, _ metav1.GetOptions) (*corev1.ConfigMap, error) { + return &corev1.ConfigMap{}, nil + }, func() string { + return optr.release.Version }) err := optr.availableUpdatesSync(ctx, optr.queueKey()) if err != nil && tt.wantErr == nil { diff --git a/pkg/internal/constants.go b/pkg/internal/constants.go index eac0f36af..568e038db 100644 --- a/pkg/internal/constants.go +++ b/pkg/internal/constants.go @@ -4,6 +4,8 @@ import ( "fmt" "strings" + "github.com/blang/semver/v4" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" @@ -131,3 +133,24 @@ func IsAlertConditionReason(reason string) bool { func AlertConditionMessage(alertName, severity, state, impact, details string) string { return fmt.Sprintf("%s alert %s %s, %s. %s", severity, alertName, state, impact, details) } + +const ( + UpdateTypeMajor = "Major" + UpdateTypeMinor = "Minor" + UpdateTypePatch = "Patch" + UpdateTypeUnknown = "Unknown" +) + +// UpdateType returns the type of the update from the source to the target versions +func UpdateType(source, target semver.Version) string { + if source.Major < target.Major { + return UpdateTypeMajor + } + if source.Major == target.Major && source.Minor < target.Minor { + return UpdateTypeMinor + } + if source.LT(target) { + return UpdateTypePatch + } + return UpdateTypeUnknown +} diff --git a/pkg/payload/precondition/clusterversion/upgradeable.go b/pkg/payload/precondition/clusterversion/upgradeable.go index 74d493491..f6ddb5292 100644 --- a/pkg/payload/precondition/clusterversion/upgradeable.go +++ b/pkg/payload/precondition/clusterversion/upgradeable.go @@ -150,11 +150,8 @@ func majorOrMinorUpdateFrom(status configv1.ClusterVersionStatus, currentVersion } if cond := resourcemerge.FindOperatorStatusCondition(status.Conditions, configv1.OperatorProgressing); cond != nil && cond.Status == configv1.ConditionTrue { - if v.Major < currentVersion.Major { - return completedVersion, "Major" - } - if v.Major == currentVersion.Major && v.Minor < currentVersion.Minor { - return completedVersion, "Minor" + if ut := internal.UpdateType(v, currentVersion); ut == internal.UpdateTypeMajor || ut == internal.UpdateTypeMinor { + return completedVersion, ut } } return "", "" diff --git a/pkg/proposal/controller.go b/pkg/proposal/controller.go index f7ac04cc0..9f270a621 100644 --- a/pkg/proposal/controller.go +++ b/pkg/proposal/controller.go @@ -4,16 +4,19 @@ import ( "context" "fmt" "os" + "regexp" "strings" "time" "github.com/blang/semver/v4" ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" + corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kutilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" @@ -24,19 +27,26 @@ import ( ) type Controller struct { - queueKey string - queue workqueue.TypedRateLimitingInterface[any] - updatesGetterFunc UpdatesGetterFunc - client ctrlruntimeclient.Client - cvGetterFunc cvGetterFunc + queueKey string + queue workqueue.TypedRateLimitingInterface[any] + updatesGetterFunc updatesGetterFunc + client ctrlruntimeclient.Client + cvGetterFunc cvGetterFunc + configMapGetterFunc configMapGetterFunc + getCurrentVersionFunc getCurrentVersionFunc + config Config } const controllerName = "proposal-lifecycle-controller" -type UpdatesGetterFunc func() ([]configv1.Release, []configv1.ConditionalUpdate, error) +type updatesGetterFunc func() ([]configv1.Release, []configv1.ConditionalUpdate, error) type cvGetterFunc func(name string) (*configv1.ClusterVersion, error) +type getCurrentVersionFunc func() string + +type configMapGetterFunc func(ctx context.Context, namespace, name string, opts metav1.GetOptions) (*corev1.ConfigMap, error) + // NewController returns Controller to manage Proposals. // It monitors available and conditional updates, and creates a LightspeedProposal for every target version of them. // It expires (and replace) any previous LightspeedProposals owned by the CVO after 24h. @@ -44,22 +54,30 @@ type cvGetterFunc func(name string) (*configv1.ClusterVersion, error) // that are no longer supported next-hop options (e.g. because a channel change or cluster update), but preserves // LightspeedProposals associated with versions in the ClusterVersion status.history (history already has its own // garbage-collection). -func NewController(updatesGetterFunc UpdatesGetterFunc, client ctrlruntimeclient.Client, cvGetterFunc cvGetterFunc) *Controller { +func NewController( + updatesGetterFunc updatesGetterFunc, + client ctrlruntimeclient.Client, + cvGetterFunc cvGetterFunc, + configMapGetterFunc configMapGetterFunc, + getCurrentVersionFunc getCurrentVersionFunc, +) *Controller { return &Controller{ queueKey: fmt.Sprintf("ClusterVersionOperator/%s", controllerName), queue: workqueue.NewTypedRateLimitingQueueWithConfig[any]( workqueue.DefaultTypedControllerRateLimiter[any](), workqueue.TypedRateLimitingQueueConfig[any]{Name: controllerName}), - updatesGetterFunc: updatesGetterFunc, - client: client, - cvGetterFunc: cvGetterFunc, + updatesGetterFunc: updatesGetterFunc, + client: client, + cvGetterFunc: cvGetterFunc, + configMapGetterFunc: configMapGetterFunc, + getCurrentVersionFunc: getCurrentVersionFunc, + config: DefaultConfig(), } } // Config holds configuration for proposal creation. type Config struct { Namespace string - Workflow string PromptConfigMap string // ConfigMap name containing the system prompt } @@ -67,7 +85,6 @@ type Config struct { func DefaultConfig() Config { return Config{ Namespace: envOrDefault("LIGHTSPEED_PROPOSAL_NAMESPACE", "openshift-lightspeed"), - Workflow: envOrDefault("LIGHTSPEED_PROPOSAL_WORKFLOW", "ota-advisory"), PromptConfigMap: envOrDefault("LIGHTSPEED_PROMPT_CONFIGMAP", "ota-advisory-prompt"), } } @@ -102,18 +119,47 @@ func (c *Controller) Sync(ctx context.Context, key string) error { klog.V(i.Debug).Infof("Got available updates: %#v", updates) klog.V(i.Debug).Infof("Got conditional updates: %#v", conditionalUpdates) - proposals := getProposals(updates, conditionalUpdates) - - var errs []error - cv, err := c.cvGetterFunc(i.DefaultClusterVersionName) if err != nil { klog.V(i.Normal).Infof("Failed to get ClusterVersion %s: %v", i.DefaultClusterVersionName, err) - errs = append(errs, err) - } else if err := deleteProposals(c.client, updates, conditionalUpdates, cv.Status.History); err != nil { + return fmt.Errorf("failed to get ClusterVersion %s: %w", i.DefaultClusterVersionName, err) + } + + currentVersion := c.getCurrentVersionFunc() + + var errs []error + if err := deleteProposals(ctx, c.client, updates, conditionalUpdates, cv.Status.History, currentVersion); err != nil { errs = append(errs, err) } + if len(updates) == 0 && len(conditionalUpdates) == 0 { + return kutilerrors.NewAggregate(errs) + } + + var prompt string + promptConfigMap, err := c.configMapGetterFunc(ctx, c.config.Namespace, c.config.PromptConfigMap, metav1.GetOptions{}) + if err != nil { + klog.V(i.Normal).Infof("Failed to get prompt ConfigMap %s/%s: %v", c.config.Namespace, c.config.PromptConfigMap, err) + errs = append(errs, fmt.Errorf("failed to get prompt ConfigMap %s/%s: %w", c.config.Namespace, c.config.PromptConfigMap, err)) + return kutilerrors.NewAggregate(errs) + } + promptKey := "prompt" + if v, ok := promptConfigMap.Data[promptKey]; ok { + prompt = v + } else { + klog.V(i.Normal).Infof("ConfigMap %s/%s has no key %s in data", c.config.Namespace, c.config.PromptConfigMap, promptKey) + errs = append(errs, fmt.Errorf("failed to get key/%s from ConfigMap %s/%s", promptKey, c.config.Namespace, c.config.PromptConfigMap)) + return kutilerrors.NewAggregate(errs) + } + + // TODO: Implement it + readinessJSON := "{}" + proposals, err := getProposals(updates, conditionalUpdates, c.config.Namespace, currentVersion, cv.Spec.Channel, prompt, readinessJSON) + if err != nil { + klog.V(i.Normal).Infof("Getting proposals hit an error: %v", err) + return kutilerrors.NewAggregate(append(errs, err)) + } + for _, proposal := range proposals { existing := &proposalv1alpha1.Proposal{} err := c.client.Get(ctx, ctrlruntimeclient.ObjectKey{Name: proposal.Name, Namespace: proposal.Namespace}, existing) @@ -124,10 +170,12 @@ func (c *Controller) Sync(ctx context.Context, key string) error { continue } } else { + if !ownedByCVO(existing) { + klog.V(i.Normal).Infof("Ignored proposal %s/%s not owned by CVO", proposal.Namespace, proposal.Name) + continue + } if expired(existing) { - err := c.client.Delete(ctx, existing) - if err != nil && !kerrors.IsNotFound(err) { - klog.V(i.Normal).Infof("Failed to delete proposal %s/%s: %v", proposal.Namespace, proposal.Name, err) + if err := deleteProposal(ctx, c.client, existing, "expired"); err != nil { errs = append(errs, err) continue } @@ -137,7 +185,7 @@ func (c *Controller) Sync(ctx context.Context, key string) error { } } - if c.client.Create(ctx, proposal) != nil { + if err := c.client.Create(ctx, proposal); err != nil { if !kerrors.IsAlreadyExists(err) { klog.V(i.Normal).Infof("Failed to create proposal %s/%s: %v", proposal.Namespace, proposal.Name, err) errs = append(errs, err) @@ -152,92 +200,231 @@ func (c *Controller) Sync(ctx context.Context, key string) error { return kutilerrors.NewAggregate(errs) } -// TODO: -func expired(_ *proposalv1alpha1.Proposal) bool { - return false +func ownedByCVO(p *proposalv1alpha1.Proposal) bool { + if p == nil { + return false + } + return p.Labels[labelKeySource] == labelValueSource +} + +func expired(p *proposalv1alpha1.Proposal) bool { + if p == nil { + return false + } + return time.Now().After(p.CreationTimestamp.Add(proposalExpiration)) +} + +func deleteProposals(ctx context.Context, client ctrlruntimeclient.Client, availableUpdates []configv1.Release, conditionalUpdates []configv1.ConditionalUpdate, history []configv1.UpdateHistory, currentVersion string) error { + targets := sets.New[string]() + for _, update := range availableUpdates { + targets.Insert(labelValueFromVersion(update.Version)) + } + for _, update := range conditionalUpdates { + targets.Insert(labelValueFromVersion(update.Release.Version)) + } + associatedWithHistory := sets.New[string]() + for _, h := range history { + associatedWithHistory.Insert(labelValueFromVersion(h.Version)) + } + + list := &proposalv1alpha1.ProposalList{} + if err := client.List(ctx, list, ctrlruntimeclient.MatchingLabels(CVOProposalLabels)); err != nil { + return fmt.Errorf("failed to list proposals: %w", err) + } + var errs []error + for _, proposal := range list.Items { + if !ownedByCVO(&proposal) { + klog.V(i.Debug).Infof("Keeping proposal %s/%s not owned by CVO", proposal.Namespace, proposal.Name) + continue + } + cv, cvOk := proposal.Labels[labelKeyCurrentVersion] + tv, tvOk := proposal.Labels[labelKeyTargetVersion] + if cvOk && tvOk && cv == currentVersion && targets.Has(tv) { + klog.V(i.Debug).Infof("Keeping relevant proposal %s/%s from %s to %s", proposal.Namespace, proposal.Name, cv, tv) + continue + } + if tvOk && associatedWithHistory.Has(tv) { + klog.V(i.Debug).Infof("Keeping proposal %s/%s for a version %s associated with history", proposal.Namespace, proposal.Name, tv) + continue + } + err := deleteProposal(ctx, client, &proposal, "irrelevant") + if err != nil { + errs = append(errs, err) + } + } + + return kutilerrors.NewAggregate(errs) } -// TODO: -func deleteProposals(_ ctrlruntimeclient.Client, _ []configv1.Release, _ []configv1.ConditionalUpdate, _ []configv1.UpdateHistory) error { +func deleteProposal(ctx context.Context, client ctrlruntimeclient.Client, proposal *proposalv1alpha1.Proposal, adjective string) error { + if proposal == nil { + return nil + } + klog.V(i.Normal).Infof("Deleting %s proposal %s/%s ...", adjective, proposal.Namespace, proposal.Name) + err := client.Delete(ctx, proposal) + if err == nil { + klog.V(i.Normal).Infof("Deleted %s proposal %s/%s", adjective, proposal.Namespace, proposal.Name) + return nil + } + + if !kerrors.IsNotFound(err) { + klog.V(i.Normal).Infof("Failed to delete %s proposal %s/%s: %v", adjective, proposal.Namespace, proposal.Name, err) + return err + } + + klog.V(i.Normal).Infof("Failed to delete not-found proposal %s/%s", proposal.Namespace, proposal.Name) return nil } -// TODO: make it real -func getProposals(_ []configv1.Release, _ []configv1.ConditionalUpdate) []*proposalv1alpha1.Proposal { - return []*proposalv1alpha1.Proposal{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-proposal", - Namespace: i.DefaultCVONamespace, +func getProposals( + availableUpdates []configv1.Release, + conditionalUpdates []configv1.ConditionalUpdate, + namespace string, + currentVersion, channel, + systemPrompt string, + readinessJSON string, +) ([]*proposalv1alpha1.Proposal, error) { + var errs []error + var proposals []*proposalv1alpha1.Proposal + for _, au := range availableUpdates { + targetVersion := au.Version + if proposal, err := getProposal(namespace, currentVersion, targetVersion, channel, updateKindRecommended, systemPrompt, readinessJSON, availableUpdates); err != nil { + errs = append(errs, err) + continue + } else { + proposals = append(proposals, proposal) + } + } + + for _, cu := range conditionalUpdates { + targetVersion := cu.Release.Version + if proposal, err := getProposal(namespace, currentVersion, targetVersion, channel, updateKindConditional, systemPrompt, readinessJSON, availableUpdates); err != nil { + errs = append(errs, err) + continue + } else { + proposals = append(proposals, proposal) + } + } + + return proposals, kutilerrors.NewAggregate(errs) +} + +func getProposal(namespace, currentVersion, targetVersion, channel, updateKind, systemPrompt, readinessJSON string, availableUpdates []configv1.Release) (*proposalv1alpha1.Proposal, error) { + + var errs []error + for _, v := range []string{currentVersion, targetVersion} { + if _, err := semver.Parse(v); err != nil { + errs = append(errs, fmt.Errorf("invalid version %s: %w", v, err)) + } + } + if len(errs) > 0 { + return nil, kutilerrors.NewAggregate(errs) + } + + name := proposalName(currentVersion, targetVersion) + updateType := classifyUpdate(currentVersion, targetVersion) + request := buildRequest(systemPrompt, currentVersion, targetVersion, channel, updateType, updateKind, availableUpdates, readinessJSON) + return &proposalv1alpha1.Proposal{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: map[string]string{ + labelKeySource: labelValueSource, + labelKeyCurrentVersion: labelValueFromVersion(currentVersion), + labelKeyTargetVersion: labelValueFromVersion(targetVersion), + "agentic.openshift.io/update-type": updateType, }, - // Feed required fields only to pass API server validation - // The agent does not exist but that should cause no trouble because no controller watches it before lightspeed-operator is installed on the cluster - Spec: proposalv1alpha1.ProposalSpec{ - // TODO: make a real request - Request: "some-request-todo", - // CVO consumes some Agent maintained by the cluster admin - Analysis: proposalv1alpha1.ProposalStep{ - Agent: "smart", - }, - Tools: proposalv1alpha1.ToolsSpec{ - Skills: []proposalv1alpha1.SkillsSource{ - { - // TODO: Use the image built from https://github.com/openshift/agentic-skills - Image: "quay.io/harpatil/ocp-skills:latest", - Paths: []string{ - "/skills/cluster-update/update-advisor", - "/skills/cluster-update/product-lifecycle", - "/skills/monitoring/prometheus", - "/skills/documentation/openshift", - "/skills/documentation/kubernetes", - "/skills/support/jira", - }, + }, + Spec: proposalv1alpha1.ProposalSpec{ + Request: request, + // CVO consumes some Agent maintained by the cluster admin + Analysis: proposalv1alpha1.ProposalStep{ + Agent: "smart", + }, + Tools: proposalv1alpha1.ToolsSpec{ + Skills: []proposalv1alpha1.SkillsSource{ + { + // TODO: OTA-1980: Use the Production image built from https://github.com/openshift/agentic-skills + Image: "quay.io/openshift/ci:ocp_5.0_agentic-skills", + Paths: []string{ + "/skills/cluster-update/update-advisor", + "/skills/cluster-update/product-lifecycle", + "/skills/documentation/openshift", + "/skills/documentation/kubernetes", }, }, }, - AnalysisOutput: proposalv1alpha1.AnalysisOutput{ - Mode: proposalv1alpha1.AnalysisOutputModeMinimal, - // TODO: make a real schema - Schema: &apiextensionsv1.JSONSchemaProps{ - Type: "object", - Description: "Proposal analysis — lightweight custom output", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "property-name-todo": { - Type: "string", - Description: "todo", + }, + AnalysisOutput: proposalv1alpha1.AnalysisOutput{ + Mode: proposalv1alpha1.AnalysisOutputModeMinimal, + // TODO: sync up from https://github.com/openshift/cluster-update-console-plugin/blob/main/src/components/update-plan/ActivePlanView.tsx + // For now, it is an object with a required property analysisData that is a map from string to any. + Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "object", + Description: "Proposal analysis — lightweight custom output", + Required: []string{"analysisData"}, + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "analysisData": { + Type: "object", + Description: "Analysis data", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Allows: true, }, }, }, }, }, }, + }, nil +} + +// https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set +func labelValueFromVersion(version string) string { + return trim63(version) +} + +func trim63(value string) string { + if len(value) > 63 { + return value[:60] + "xxx" } + return value } // proposalName generates a deterministic proposal name from the version pair. func proposalName(current, target string) string { - return fmt.Sprintf("ota-%s-to-%s", sanitize(current), sanitize(target)) + return toDNS1035(fmt.Sprintf("ota-%s-to-%s", current, target)) } -// sanitize converts a version string into a valid DNS-1035 label component. -// DNS-1035 requires: lowercase alphanumeric or '-', start with alpha, end with alphanum. -func sanitize(s string) string { - s = strings.ToLower(s) - s = strings.ReplaceAll(s, ".", "-") - s = strings.ReplaceAll(s, " ", "-") - if len(s) > 20 { - s = s[:20] - } - return strings.TrimRight(s, "-") +var alphanumericRegex = regexp.MustCompile(`[^a-z0-9]+`) + +// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#rfc-1035-label-names +func toDNS1035(name string) string { + // Convert to lowercase + ret := strings.ToLower(name) + + // Replace any non-alphanumeric character with a hyphen + ret = alphanumericRegex.ReplaceAllString(ret, "-") + + // Trim hyphens from the ends + ret = strings.Trim(ret, "-") + + return trim63(ret) } const ( - updateKindRecommended = "recommended" - updateKindConditional = "conditional" + labelKeySource = "agentic.openshift.io/source" + labelValueSource = "cluster-version-operator" + labelKeyCurrentVersion = "agentic.openshift.io/current-version" + labelKeyTargetVersion = "agentic.openshift.io/target-version" + + updateKindRecommended = "Recommended" + updateKindConditional = "Conditional" - updateTypeZStream = "z-stream" - updateTypeMinor = "minor" - updateTypeUnknown = "unknown" + proposalExpiration = 24 * time.Hour +) + +var ( + CVOProposalLabels = map[string]string{labelKeySource: labelValueSource} ) // classifyUpdate returns "z-stream" if major.minor match, otherwise "minor". @@ -245,12 +432,9 @@ func classifyUpdate(current, target string) string { cv, cerr := semver.Parse(current) tv, terr := semver.Parse(target) if cerr != nil || terr != nil { - return updateTypeUnknown - } - if cv.Major == tv.Major && cv.Minor == tv.Minor { - return updateTypeZStream + return i.UpdateTypeUnknown } - return updateTypeMinor + return i.UpdateType(cv, tv) } // buildRequest constructs the proposal request with system prompt, metadata, and readiness data. @@ -264,11 +448,11 @@ func buildRequest(systemPrompt, current, target, channel, updateType, targetType b.WriteString("\n\n---\n\n") } - fmt.Fprintf(&b, "Current version: OCP %s\n", current) - fmt.Fprintf(&b, "Target version: OCP %s\n", target) - fmt.Fprintf(&b, "Channel: %s\n", channel) - fmt.Fprintf(&b, "Update type: %s\n", updateType) - fmt.Fprintf(&b, "Update path: %s\n\n", targetType) + _, _ = fmt.Fprintf(&b, "Current version: OCP %s\n", current) + _, _ = fmt.Fprintf(&b, "Target version: OCP %s\n", target) + _, _ = fmt.Fprintf(&b, "Channel: %s\n", channel) + _, _ = fmt.Fprintf(&b, "Update type: %s\n", updateType) + _, _ = fmt.Fprintf(&b, "Update path: %s\n\n", targetType) if targetType == updateKindConditional { b.WriteString("WARNING: This target version is available as a CONDITIONAL update.\n") @@ -282,15 +466,15 @@ func buildRequest(systemPrompt, current, target, channel, updateType, targetType for _, u := range updates { if u.Version != target { if u.URL != "" { - fmt.Fprintf(&b, " - %s (errata: %s)\n", u.Version, u.URL) + _, _ = fmt.Fprintf(&b, " - %s (errata: %s)\n", u.Version, u.URL) } else { - fmt.Fprintf(&b, " - %s\n", u.Version) + _, _ = fmt.Fprintf(&b, " - %s\n", u.Version) } count++ if count >= 5 { remaining := len(updates) - count - 1 if remaining > 0 { - fmt.Fprintf(&b, " ... and %d more\n", remaining) + _, _ = fmt.Fprintf(&b, " ... and %d more\n", remaining) } break } diff --git a/pkg/proposal/controller_test.go b/pkg/proposal/controller_test.go index db96a8c0c..847b79537 100644 --- a/pkg/proposal/controller_test.go +++ b/pkg/proposal/controller_test.go @@ -5,11 +5,18 @@ import ( "fmt" "strings" "testing" + "time" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kutilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/kubernetes/scheme" configv1 "github.com/openshift/api/config/v1" @@ -26,7 +33,7 @@ func init() { func TestController_Sync(t *testing.T) { tests := []struct { name string - updatesGetterFunc UpdatesGetterFunc + updatesGetterFunc updatesGetterFunc client ctrlruntimeclient.Client cvGetterFunc cvGetterFunc expected error @@ -35,10 +42,21 @@ func TestController_Sync(t *testing.T) { { name: "basic case", updatesGetterFunc: func() ([]configv1.Release, []configv1.ConditionalUpdate, error) { - return nil, nil, nil + return []configv1.Release{ + { + Version: "5.0.0-ec.0", + }, + }, nil, nil }, cvGetterFunc: func(_ string) (*configv1.ClusterVersion, error) { - return &configv1.ClusterVersion{}, nil + return &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "version", + }, + Spec: configv1.ClusterVersionSpec{ + Channel: "stable-5.0", + }, + }, nil }, client: fake.NewClientBuilder().Build(), verifyFunc: func(client ctrlruntimeclient.Client) error { @@ -46,8 +64,72 @@ func TestController_Sync(t *testing.T) { if err := client.List(context.Background(), proposals); err != nil { return err } - if len(proposals.Items) == 0 { - return fmt.Errorf("expected proposals, none") + expect := &proposalv1alpha1.ProposalList{ + Items: []proposalv1alpha1.Proposal{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "ota-4-22-1-to-5-0-0-ec-0", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + "agentic.openshift.io/current-version": "4.22.1", + "agentic.openshift.io/source": "cluster-version-operator", + "agentic.openshift.io/target-version": "5.0.0-ec.0", + "agentic.openshift.io/update-type": "Major", + }, + }, + Spec: proposalv1alpha1.ProposalSpec{ + Request: `prompt-abc + +--- + +Current version: OCP 4.22.1 +Target version: OCP 5.0.0-ec.0 +Channel: stable-5.0 +Update type: Major +Update path: Recommended + +## Cluster Readiness Data + +` + "```json\n" + + `{}` + "\n```\n", + Analysis: proposalv1alpha1.ProposalStep{ + Agent: "smart", + }, + Tools: proposalv1alpha1.ToolsSpec{ + Skills: []proposalv1alpha1.SkillsSource{ + { + Image: "quay.io/openshift/ci:ocp_5.0_agentic-skills", + Paths: []string{ + "/skills/cluster-update/update-advisor", + "/skills/cluster-update/product-lifecycle", + "/skills/documentation/openshift", + "/skills/documentation/kubernetes", + }, + }, + }, + }, + AnalysisOutput: proposalv1alpha1.AnalysisOutput{ + Mode: proposalv1alpha1.AnalysisOutputModeMinimal, + Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "object", + Description: "Proposal analysis — lightweight custom output", + Required: []string{"analysisData"}, + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "analysisData": { + Type: "object", + Description: "Analysis data", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Allows: true, + }, + }, + }, + }, + }, + }, + }, + }} + if diff := cmp.Diff(expect, proposals, cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ResourceVersion")); diff != "" { + return fmt.Errorf("unexpected ProposalList (-want, +got) = \n%v", diff) } return nil }, @@ -55,7 +137,16 @@ func TestController_Sync(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c := NewController(tt.updatesGetterFunc, tt.client, tt.cvGetterFunc) + c := NewController(tt.updatesGetterFunc, tt.client, tt.cvGetterFunc, func(_ context.Context, namespace, name string, _ metav1.GetOptions) (*corev1.ConfigMap, error) { + return &corev1.ConfigMap{ + Data: map[string]string{ + "prompt": "prompt-abc", + "foo": "bar", + }, + }, nil + }, func() string { + return "4.22.1" + }) actual := c.Sync(context.Background(), tt.name) if diff := cmp.Diff(tt.expected, actual, cmp.Transformer("Error", func(e error) string { if e == nil { @@ -81,11 +172,11 @@ func TestClassifyUpdate(t *testing.T) { target string expected string }{ - {name: "z-stream", current: "4.15.1", target: "4.15.3", expected: "z-stream"}, - {name: "minor", current: "4.15.1", target: "4.16.0", expected: "minor"}, - {name: "major", current: "4.15.1", target: "5.0.0", expected: "minor"}, - {name: "invalid current", current: "bad", target: "4.15.0", expected: "unknown"}, - {name: "invalid target", current: "4.15.0", target: "bad", expected: "unknown"}, + {name: "z-stream", current: "4.15.1", target: "4.15.3", expected: "Patch"}, + {name: "minor", current: "4.15.1", target: "4.16.0", expected: "Minor"}, + {name: "major", current: "4.15.1", target: "5.0.0", expected: "Major"}, + {name: "invalid current", current: "bad", target: "4.15.0", expected: "Unknown"}, + {name: "invalid target", current: "4.15.0", target: "bad", expected: "Unknown"}, } for _, tt := range tests { @@ -118,21 +209,18 @@ func TestProposalName(t *testing.T) { } } -func TestSanitize(t *testing.T) { +func Test_labelValueFromVersion(t *testing.T) { tests := []struct { input string expected string }{ - {"4.15.1", "4-15-1"}, - {"Hello World", "hello-world"}, - {"a-very-long-version-string-that-is-too-long", "a-very-long-version"}, - {"trailing-dot.", "trailing-dot"}, - {"trailing-dash-", "trailing-dash"}, + {"4.15.1", "4.15.1"}, + {"4.15.1-a-very-long-version-string-that-is-too-long-long-version-string-that-is-too-long", "4.15.1-a-very-long-version-string-that-is-too-long-long-versxxx"}, } for _, tt := range tests { t.Run(tt.input, func(t *testing.T) { - got := sanitize(tt.input) + got := labelValueFromVersion(tt.input) if got != tt.expected { t.Errorf("sanitize(%q) = %q, want %q", tt.input, got, tt.expected) } @@ -172,7 +260,7 @@ func TestBuildRequest(t *testing.T) { }) t.Run("conditional target", func(t *testing.T) { - request := buildRequest("", "4.15.3", "4.16.0", "stable-4.16", "minor", "conditional", updates, "") + request := buildRequest("", "4.15.3", "4.16.0", "stable-4.16", "minor", "Conditional", updates, "") if !strings.Contains(request, "WARNING") { t.Error("conditional target should have warning") } @@ -182,7 +270,7 @@ func TestBuildRequest(t *testing.T) { }) t.Run("readiness JSON embedded", func(t *testing.T) { - request := buildRequest("", "4.15.3", "4.16.0", "stable-4.16", "minor", "recommended", updates, `{"checks":{},"meta":{}}`) + request := buildRequest("", "4.15.3", "4.16.0", "stable-4.16", "minor", "Recommended", updates, `{"checks":{},"meta":{}}`) if !strings.Contains(request, "## Cluster Readiness Data") { t.Error("request should contain readiness data header") } @@ -191,3 +279,957 @@ func TestBuildRequest(t *testing.T) { } }) } + +func TestDeleteProposals(t *testing.T) { + tests := []struct { + name string + availableUpdates []configv1.Release + conditionalUpdates []configv1.ConditionalUpdate + history []configv1.UpdateHistory + currentVersion string + existingProposals []proposalv1alpha1.Proposal + expectedDeleted []string + expectedKept []string + }{ + { + name: "keeps relevant proposals matching current version and target", + availableUpdates: []configv1.Release{ + {Version: "4.16.0"}, + {Version: "4.16.1"}, + }, + currentVersion: "4.15.3", + existingProposals: []proposalv1alpha1.Proposal{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "proposal-4-15-3-to-4-16-0", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + labelKeySource: labelValueSource, + labelKeyCurrentVersion: "4.15.3", + labelKeyTargetVersion: "4.16.0", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "proposal-4-15-3-to-4-16-1", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + labelKeySource: labelValueSource, + labelKeyCurrentVersion: "4.15.3", + labelKeyTargetVersion: "4.16.1", + }, + }, + }, + }, + expectedKept: []string{"proposal-4-15-3-to-4-16-0", "proposal-4-15-3-to-4-16-1"}, + expectedDeleted: []string{}, + }, + { + name: "deletes proposals with outdated current version", + availableUpdates: []configv1.Release{ + {Version: "4.16.0"}, + }, + currentVersion: "4.15.3", + existingProposals: []proposalv1alpha1.Proposal{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "proposal-4-15-2-to-4-16-0", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + labelKeySource: labelValueSource, + labelKeyCurrentVersion: "4.15.2", + labelKeyTargetVersion: "4.16.0", + }, + }, + }, + }, + expectedKept: []string{}, + expectedDeleted: []string{"proposal-4-15-2-to-4-16-0"}, + }, + { + name: "deletes proposals for targets no longer in available updates", + availableUpdates: []configv1.Release{ + {Version: "4.16.1"}, + }, + currentVersion: "4.15.3", + existingProposals: []proposalv1alpha1.Proposal{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "proposal-4-15-3-to-4-16-0", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + labelKeySource: labelValueSource, + labelKeyCurrentVersion: "4.15.3", + labelKeyTargetVersion: "4.16.0", + }, + }, + }, + }, + expectedKept: []string{}, + expectedDeleted: []string{"proposal-4-15-3-to-4-16-0"}, + }, + { + name: "keeps proposals associated with history", + availableUpdates: []configv1.Release{ + {Version: "4.16.2"}, + }, + history: []configv1.UpdateHistory{ + {Version: "4.16.1"}, + }, + currentVersion: "4.16.1", + existingProposals: []proposalv1alpha1.Proposal{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "proposal-old-to-4-16-1", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + labelKeySource: labelValueSource, + labelKeyCurrentVersion: "4.15.0", + labelKeyTargetVersion: "4.16.1", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "proposal-old-to-4-15-3", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + labelKeySource: labelValueSource, + labelKeyCurrentVersion: "4.15.0", + labelKeyTargetVersion: "4.15.3", + }, + }, + }, + }, + expectedKept: []string{"proposal-old-to-4-16-1"}, + expectedDeleted: []string{"proposal-old-to-4-15-3"}, + }, + { + name: "keeps proposals not owned by CVO", + availableUpdates: []configv1.Release{ + {Version: "4.16.0"}, + }, + currentVersion: "4.15.3", + existingProposals: []proposalv1alpha1.Proposal{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "user-created-proposal", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + labelKeySource: "manual", + labelKeyCurrentVersion: "4.14.0", + labelKeyTargetVersion: "4.15.0", + }, + }, + }, + }, + expectedKept: []string{"user-created-proposal"}, + expectedDeleted: []string{}, + }, + { + name: "handles conditional updates", + conditionalUpdates: []configv1.ConditionalUpdate{ + {Release: configv1.Release{Version: "4.16.2"}}, + }, + currentVersion: "4.15.3", + existingProposals: []proposalv1alpha1.Proposal{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "proposal-4-15-3-to-4-16-2", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + labelKeySource: labelValueSource, + labelKeyCurrentVersion: "4.15.3", + labelKeyTargetVersion: "4.16.2", + }, + }, + }, + }, + expectedKept: []string{"proposal-4-15-3-to-4-16-2"}, + expectedDeleted: []string{}, + }, + { + name: "deletes proposals with missing labels", + availableUpdates: []configv1.Release{ + {Version: "4.16.0"}, + }, + currentVersion: "4.15.3", + existingProposals: []proposalv1alpha1.Proposal{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "proposal-missing-labels", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + labelKeySource: labelValueSource, + // Missing current and target version labels + }, + }, + }, + }, + expectedKept: []string{}, + expectedDeleted: []string{"proposal-missing-labels"}, + }, + { + name: "handles empty updates and history", + availableUpdates: []configv1.Release{}, + currentVersion: "4.15.3", + existingProposals: []proposalv1alpha1.Proposal{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "proposal-4-15-3-to-4-16-0", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + labelKeySource: labelValueSource, + labelKeyCurrentVersion: "4.15.3", + labelKeyTargetVersion: "4.16.0", + }, + }, + }, + }, + expectedKept: []string{}, + expectedDeleted: []string{"proposal-4-15-3-to-4-16-0"}, + }, + { + name: "mixed scenario: keep some, delete some", + availableUpdates: []configv1.Release{ + {Version: "4.16.1"}, + }, + conditionalUpdates: []configv1.ConditionalUpdate{ + {Release: configv1.Release{Version: "4.16.2"}}, + }, + history: []configv1.UpdateHistory{ + {Version: "4.15.3"}, + {Version: "4.16.0"}, + }, + currentVersion: "4.16.0", + existingProposals: []proposalv1alpha1.Proposal{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "keep-relevant", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + labelKeySource: labelValueSource, + labelKeyCurrentVersion: "4.16.0", + labelKeyTargetVersion: "4.16.2", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "keep-in-history", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + labelKeySource: labelValueSource, + labelKeyCurrentVersion: "4.15.0", + labelKeyTargetVersion: "4.15.3", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "delete-outdated", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + labelKeySource: labelValueSource, + labelKeyCurrentVersion: "4.15.0", + labelKeyTargetVersion: "4.15.10", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "keep-user-created", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + labelKeySource: "manual", + labelKeyCurrentVersion: "4.14.0", + labelKeyTargetVersion: "4.15.0", + }, + }, + }, + }, + expectedKept: []string{"keep-relevant", "keep-in-history", "keep-user-created"}, + expectedDeleted: []string{"delete-outdated"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + + // Create fake client with existing proposals + clientBuilder := fake.NewClientBuilder() + for _, p := range tt.existingProposals { + clientBuilder.WithObjects(&p) + } + client := clientBuilder.Build() + + err := deleteProposals(ctx, client, tt.availableUpdates, tt.conditionalUpdates, tt.history, tt.currentVersion) + if err != nil { + t.Errorf("deleteProposals() returned unexpected error: %v", err) + } + + for _, name := range tt.expectedKept { + proposal := &proposalv1alpha1.Proposal{} + err := client.Get(ctx, ctrlruntimeclient.ObjectKey{Name: name, Namespace: "openshift-lightspeed"}, proposal) + if err != nil { + t.Errorf("expected proposal %s to be kept, but got error: %v", name, err) + } + } + + for _, name := range tt.expectedDeleted { + proposal := &proposalv1alpha1.Proposal{} + err := client.Get(ctx, ctrlruntimeclient.ObjectKey{Name: name, Namespace: "openshift-lightspeed"}, proposal) + if err == nil { + t.Errorf("expected proposal %s to be deleted, but it still exists", name) + } + } + }) + } +} + +func TestDeleteProposal(t *testing.T) { + tests := []struct { + name string + proposal *proposalv1alpha1.Proposal + adjective string + setupClient func() ctrlruntimeclient.Client + expect error + shouldBeDeleted bool + }{ + { + name: "successfully deletes existing proposal", + proposal: &proposalv1alpha1.Proposal{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-proposal", + Namespace: "openshift-lightspeed", + }, + }, + adjective: "expired", + setupClient: func() ctrlruntimeclient.Client { + return fake.NewClientBuilder().WithObjects(&proposalv1alpha1.Proposal{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-proposal", + Namespace: "openshift-lightspeed", + }, + }).Build() + }, + shouldBeDeleted: true, + }, + { + name: "handles not found error gracefully", + proposal: &proposalv1alpha1.Proposal{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nonexistent-proposal", + Namespace: "openshift-lightspeed", + }, + }, + adjective: "irrelevant", + setupClient: func() ctrlruntimeclient.Client { + return fake.NewClientBuilder().Build() + }, + shouldBeDeleted: true, + }, + { + name: "handles nil proposal", + adjective: "nil", + setupClient: func() ctrlruntimeclient.Client { + return fake.NewClientBuilder().Build() + }, + shouldBeDeleted: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + client := tt.setupClient() + + err := deleteProposal(ctx, client, tt.proposal, tt.adjective) + + if diff := cmp.Diff(tt.expect, err, cmp.Transformer("Error", func(e error) string { + if e == nil { + return "" + } + return e.Error() + })); diff != "" { + t.Errorf("unexpected error (-want +got):\n%s", diff) + } + + if tt.proposal != nil { + proposal := &proposalv1alpha1.Proposal{} + err = client.Get(ctx, ctrlruntimeclient.ObjectKey{ + Name: tt.proposal.Name, + Namespace: tt.proposal.Namespace, + }, proposal) + if tt.shouldBeDeleted { + if !kerrors.IsNotFound(err) { + t.Error("expected proposal to be deleted but it still exists") + } + } else { + if err != nil { + t.Errorf("expected no error, but got error: %v", err) + } + } + } + }) + } +} + +func TestOwnedByCVO(t *testing.T) { + tests := []struct { + name string + proposal *proposalv1alpha1.Proposal + expected bool + }{ + { + name: "nil proposal", + }, + { + name: "proposal owned by CVO", + proposal: &proposalv1alpha1.Proposal{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + labelKeySource: labelValueSource, + }, + }, + }, + expected: true, + }, + { + name: "proposal not owned by CVO", + proposal: &proposalv1alpha1.Proposal{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + labelKeySource: "manual", + }, + }, + }, + }, + { + name: "proposal with missing labels", + proposal: &proposalv1alpha1.Proposal{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + }, + }, + }, + { + name: "proposal with nil labels", + proposal: &proposalv1alpha1.Proposal{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ownedByCVO(tt.proposal) + if result != tt.expected { + t.Errorf("ownedByCVO() = %v, want %v", result, tt.expected) + } + }) + } +} + +func TestGetProposals(t *testing.T) { + tests := []struct { + name string + availableUpdates []configv1.Release + conditionalUpdates []configv1.ConditionalUpdate + namespace string + currentVersion string + channel string + systemPrompt string + readinessJSON string + expected []*proposalv1alpha1.Proposal + expectError error + }{ + { + name: "available updates only", + availableUpdates: []configv1.Release{ + {Version: "4.16.0"}, + {Version: "4.16.1"}, + }, + namespace: "openshift-lightspeed", + currentVersion: "4.15.3", + channel: "stable-4.16", + systemPrompt: "Test prompt", + readinessJSON: `{"test": "data"}`, + expected: []*proposalv1alpha1.Proposal{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "ota-4-15-3-to-4-16-0", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + "agentic.openshift.io/current-version": "4.15.3", + "agentic.openshift.io/source": "cluster-version-operator", + "agentic.openshift.io/target-version": "4.16.0", + "agentic.openshift.io/update-type": "Minor", + }, + }, + Spec: proposalv1alpha1.ProposalSpec{ + Request: `Test prompt + +--- + +Current version: OCP 4.15.3 +Target version: OCP 4.16.0 +Channel: stable-4.16 +Update type: Minor +Update path: Recommended + +Other recommended versions available: + - 4.16.1 + +## Cluster Readiness Data + +` + "```json\n" + + `{"test": "data"}` + "\n```\n", + Analysis: proposalv1alpha1.ProposalStep{ + Agent: "smart", + }, + Tools: proposalv1alpha1.ToolsSpec{ + Skills: []proposalv1alpha1.SkillsSource{ + { + Image: "quay.io/openshift/ci:ocp_5.0_agentic-skills", + Paths: []string{ + "/skills/cluster-update/update-advisor", + "/skills/cluster-update/product-lifecycle", + "/skills/documentation/openshift", + "/skills/documentation/kubernetes", + }, + }, + }, + }, + AnalysisOutput: proposalv1alpha1.AnalysisOutput{ + Mode: proposalv1alpha1.AnalysisOutputModeMinimal, + Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "object", + Description: "Proposal analysis — lightweight custom output", + Required: []string{"analysisData"}, + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "analysisData": { + Type: "object", + Description: "Analysis data", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Allows: true, + }, + }, + }, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "ota-4-15-3-to-4-16-1", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + "agentic.openshift.io/current-version": "4.15.3", + "agentic.openshift.io/source": "cluster-version-operator", + "agentic.openshift.io/target-version": "4.16.1", + "agentic.openshift.io/update-type": "Minor", + }, + }, + Spec: proposalv1alpha1.ProposalSpec{ + Request: `Test prompt + +--- + +Current version: OCP 4.15.3 +Target version: OCP 4.16.1 +Channel: stable-4.16 +Update type: Minor +Update path: Recommended + +Other recommended versions available: + - 4.16.0 + +## Cluster Readiness Data + +` + "```json\n" + + `{"test": "data"}` + "\n```\n", + Analysis: proposalv1alpha1.ProposalStep{ + Agent: "smart", + }, + Tools: proposalv1alpha1.ToolsSpec{ + Skills: []proposalv1alpha1.SkillsSource{ + { + Image: "quay.io/openshift/ci:ocp_5.0_agentic-skills", + Paths: []string{ + "/skills/cluster-update/update-advisor", + "/skills/cluster-update/product-lifecycle", + "/skills/documentation/openshift", + "/skills/documentation/kubernetes", + }, + }, + }, + }, + AnalysisOutput: proposalv1alpha1.AnalysisOutput{ + Mode: proposalv1alpha1.AnalysisOutputModeMinimal, + Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "object", + Description: "Proposal analysis — lightweight custom output", + Required: []string{"analysisData"}, + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "analysisData": { + Type: "object", + Description: "Analysis data", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Allows: true, + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "both available and conditional updates", + availableUpdates: []configv1.Release{ + {Version: "4.16.0"}, + {Version: "4.16.1"}, + }, + conditionalUpdates: []configv1.ConditionalUpdate{ + {Release: configv1.Release{Version: "4.16.2"}}, + {Release: configv1.Release{Version: "4.16.3"}}, + }, + namespace: "openshift-lightspeed", + currentVersion: "4.15.3", + channel: "stable-4.16", + expected: []*proposalv1alpha1.Proposal{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "ota-4-15-3-to-4-16-0", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + "agentic.openshift.io/current-version": "4.15.3", + "agentic.openshift.io/source": "cluster-version-operator", + "agentic.openshift.io/target-version": "4.16.0", + "agentic.openshift.io/update-type": "Minor", + }, + }, + Spec: proposalv1alpha1.ProposalSpec{ + Request: `Current version: OCP 4.15.3 +Target version: OCP 4.16.0 +Channel: stable-4.16 +Update type: Minor +Update path: Recommended + +Other recommended versions available: + - 4.16.1 + +`, + Analysis: proposalv1alpha1.ProposalStep{ + Agent: "smart", + }, + Tools: proposalv1alpha1.ToolsSpec{ + Skills: []proposalv1alpha1.SkillsSource{ + { + Image: "quay.io/openshift/ci:ocp_5.0_agentic-skills", + Paths: []string{ + "/skills/cluster-update/update-advisor", + "/skills/cluster-update/product-lifecycle", + "/skills/documentation/openshift", + "/skills/documentation/kubernetes", + }, + }, + }, + }, + AnalysisOutput: proposalv1alpha1.AnalysisOutput{ + Mode: proposalv1alpha1.AnalysisOutputModeMinimal, + Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "object", + Description: "Proposal analysis — lightweight custom output", + Required: []string{"analysisData"}, + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "analysisData": { + Type: "object", + Description: "Analysis data", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Allows: true, + }, + }, + }, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "ota-4-15-3-to-4-16-1", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + "agentic.openshift.io/current-version": "4.15.3", + "agentic.openshift.io/source": "cluster-version-operator", + "agentic.openshift.io/target-version": "4.16.1", + "agentic.openshift.io/update-type": "Minor", + }, + }, + Spec: proposalv1alpha1.ProposalSpec{ + Request: `Current version: OCP 4.15.3 +Target version: OCP 4.16.1 +Channel: stable-4.16 +Update type: Minor +Update path: Recommended + +Other recommended versions available: + - 4.16.0 + +`, + Analysis: proposalv1alpha1.ProposalStep{ + Agent: "smart", + }, + Tools: proposalv1alpha1.ToolsSpec{ + Skills: []proposalv1alpha1.SkillsSource{ + { + Image: "quay.io/openshift/ci:ocp_5.0_agentic-skills", + Paths: []string{ + "/skills/cluster-update/update-advisor", + "/skills/cluster-update/product-lifecycle", + "/skills/documentation/openshift", + "/skills/documentation/kubernetes", + }, + }, + }, + }, + AnalysisOutput: proposalv1alpha1.AnalysisOutput{ + Mode: proposalv1alpha1.AnalysisOutputModeMinimal, + Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "object", + Description: "Proposal analysis — lightweight custom output", + Required: []string{"analysisData"}, + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "analysisData": { + Type: "object", + Description: "Analysis data", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Allows: true, + }, + }, + }, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "ota-4-15-3-to-4-16-2", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + "agentic.openshift.io/current-version": "4.15.3", + "agentic.openshift.io/source": "cluster-version-operator", + "agentic.openshift.io/target-version": "4.16.2", + "agentic.openshift.io/update-type": "Minor", + }, + }, + Spec: proposalv1alpha1.ProposalSpec{ + Request: `Current version: OCP 4.15.3 +Target version: OCP 4.16.2 +Channel: stable-4.16 +Update type: Minor +Update path: Conditional + +WARNING: This target version is available as a CONDITIONAL update. +OSUS has flagged known risks that may apply to this cluster. +The assessment MUST evaluate each conditional risk against cluster state. + +Other recommended versions available: + - 4.16.0 + - 4.16.1 + +`, + Analysis: proposalv1alpha1.ProposalStep{ + Agent: "smart", + }, + Tools: proposalv1alpha1.ToolsSpec{ + Skills: []proposalv1alpha1.SkillsSource{ + { + Image: "quay.io/openshift/ci:ocp_5.0_agentic-skills", + Paths: []string{ + "/skills/cluster-update/update-advisor", + "/skills/cluster-update/product-lifecycle", + "/skills/documentation/openshift", + "/skills/documentation/kubernetes", + }, + }, + }, + }, + AnalysisOutput: proposalv1alpha1.AnalysisOutput{ + Mode: proposalv1alpha1.AnalysisOutputModeMinimal, + Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "object", + Description: "Proposal analysis — lightweight custom output", + Required: []string{"analysisData"}, + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "analysisData": { + Type: "object", + Description: "Analysis data", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Allows: true, + }, + }, + }, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "ota-4-15-3-to-4-16-3", + Namespace: "openshift-lightspeed", + Labels: map[string]string{ + "agentic.openshift.io/current-version": "4.15.3", + "agentic.openshift.io/source": "cluster-version-operator", + "agentic.openshift.io/target-version": "4.16.3", + "agentic.openshift.io/update-type": "Minor", + }, + }, + Spec: proposalv1alpha1.ProposalSpec{ + Request: `Current version: OCP 4.15.3 +Target version: OCP 4.16.3 +Channel: stable-4.16 +Update type: Minor +Update path: Conditional + +WARNING: This target version is available as a CONDITIONAL update. +OSUS has flagged known risks that may apply to this cluster. +The assessment MUST evaluate each conditional risk against cluster state. + +Other recommended versions available: + - 4.16.0 + - 4.16.1 + +`, + Analysis: proposalv1alpha1.ProposalStep{ + Agent: "smart", + }, + Tools: proposalv1alpha1.ToolsSpec{ + Skills: []proposalv1alpha1.SkillsSource{ + { + Image: "quay.io/openshift/ci:ocp_5.0_agentic-skills", + Paths: []string{ + "/skills/cluster-update/update-advisor", + "/skills/cluster-update/product-lifecycle", + "/skills/documentation/openshift", + "/skills/documentation/kubernetes", + }, + }, + }, + }, + AnalysisOutput: proposalv1alpha1.AnalysisOutput{ + Mode: proposalv1alpha1.AnalysisOutputModeMinimal, + Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "object", + Description: "Proposal analysis — lightweight custom output", + Required: []string{"analysisData"}, + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "analysisData": { + Type: "object", + Description: "Analysis data", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Allows: true, + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "empty updates", + namespace: "openshift-lightspeed", + currentVersion: "4.15.3", + channel: "stable-4.16", + }, + { + name: "invalid version in available update", + availableUpdates: []configv1.Release{ + {Version: "invalid-version"}, + }, + namespace: "openshift-lightspeed", + currentVersion: "4.15.3", + channel: "stable-4.16", + expectError: kutilerrors.NewAggregate([]error{fmt.Errorf("invalid version invalid-version: No Major.Minor.Patch elements found")}), + }, + { + name: "invalid current version", + availableUpdates: []configv1.Release{ + {Version: "4.16.0"}, + }, + namespace: "openshift-lightspeed", + currentVersion: "bad-version", + channel: "stable-4.16", + expectError: kutilerrors.NewAggregate([]error{fmt.Errorf("invalid version bad-version: No Major.Minor.Patch elements found")}), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + proposals, err := getProposals( + tt.availableUpdates, + tt.conditionalUpdates, + tt.namespace, + tt.currentVersion, + tt.channel, + tt.systemPrompt, + tt.readinessJSON, + ) + + if diff := cmp.Diff(err, tt.expectError, cmp.Transformer("Error", func(e error) string { + if e == nil { + return "" + } + return e.Error() + })); diff != "" { + t.Errorf("unexpected error (-want +got):\n%s", diff) + } + + if diff := cmp.Diff(tt.expected, proposals); diff != "" { + t.Errorf("unexpected proposals (-want +got):\n%s", diff) + } + }) + } +} + +func Test_expired(t *testing.T) { + tests := []struct { + name string + proposal *proposalv1alpha1.Proposal + expected bool + }{ + { + name: "nil proposal", + }, + { + name: "proposal not expired", + proposal: &proposalv1alpha1.Proposal{ + ObjectMeta: metav1.ObjectMeta{ + CreationTimestamp: metav1.Time{Time: time.Now().Add(-12 * time.Hour)}, + }, + }, + }, + { + name: "proposal expired", + proposal: &proposalv1alpha1.Proposal{ + ObjectMeta: metav1.ObjectMeta{ + CreationTimestamp: metav1.Time{Time: time.Now().Add(-25 * time.Hour)}, + }, + }, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := expired(tt.proposal) + if result != tt.expected { + t.Errorf("expired() = %v, want %v", result, tt.expected) + } + }) + } +} diff --git a/test/cvo/accept_risks.go b/test/cvo/accept_risks.go index a37e12888..0246fe9a1 100644 --- a/test/cvo/accept_risks.go +++ b/test/cvo/accept_risks.go @@ -13,6 +13,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" @@ -184,14 +185,14 @@ var _ = g.Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator`, o.Expect(err).NotTo(o.HaveOccurred()) versions := sets.New[string]() for _, cu := range cv.Status.ConditionalUpdates { - o.Expect(versions.Has(cu.Release.Version)).To(o.BeFalse(), "ConditionalUpdates.Release.Version %s is duplicated. cv/version is:\n%s", cu.Release.Version, getYaml(*cv)) + o.Expect(versions.Has(cu.Release.Version)).To(o.BeFalse(), "ConditionalUpdates.Release.Version %s is duplicated. cv/version is:\n%s", cu.Release.Version, getYaml(cv)) versions.Insert(cu.Release.Version) } g.By("Checking that there are no duplicated versions in available updates") versions = sets.New[string]() for _, u := range cv.Status.AvailableUpdates { - o.Expect(versions.Has(u.Version)).To(o.BeFalse(), "AvailableUpdates.Version %s is duplicated. cv/version is:\n%s", u.Version, getYaml(*cv)) + o.Expect(versions.Has(u.Version)).To(o.BeFalse(), "AvailableUpdates.Version %s is duplicated. cv/version is:\n%s", u.Version, getYaml(cv)) versions.Insert(u.Version) } @@ -304,10 +305,10 @@ var _ = g.Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator`, }) }) -func getYaml(cv configv1.ClusterVersion) string { - raw, err := yaml.Marshal(cv) +func getYaml(obj runtime.Object) string { + raw, err := yaml.Marshal(obj) if err != nil { - logger.Error(err, "failed to marshal ClusterVersion") + logger.Error(err, "failed to marshal", "Kind", obj.GetObjectKind().GroupVersionKind().String()) return "" } return string(raw) diff --git a/test/cvo/proposal.go b/test/cvo/proposal.go index ffae59b81..d46fc8c59 100644 --- a/test/cvo/proposal.go +++ b/test/cvo/proposal.go @@ -2,6 +2,8 @@ package cvo import ( "context" + "fmt" + "strings" "time" g "github.com/onsi/ginkgo/v2" @@ -13,6 +15,7 @@ import ( kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -22,6 +25,7 @@ import ( proposalv1alpha1 "github.com/openshift/lightspeed-agentic-operator/api/v1alpha1" "github.com/openshift/cluster-version-operator/pkg/external" + "github.com/openshift/cluster-version-operator/pkg/proposal" "github.com/openshift/cluster-version-operator/test/util" ) @@ -36,6 +40,7 @@ var _ = g.Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator`, var ( c *rest.Config + kubeClient kubernetes.Interface configClient *configv1client.ConfigV1Client apiExtensionsClient apiextensionsclientset.Interface rtClient ctrlruntimeclient.Client @@ -53,6 +58,8 @@ var _ = g.Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator`, o.Expect(util.SkipIfHypershift(ctx, c)).To(o.BeNil()) o.Expect(util.SkipIfMicroshift(ctx, c)).To(o.BeNil()) + kubeClient, err = util.GetKubeClient(c) + o.Expect(err).To(o.BeNil()) configClient, err = configv1client.NewForConfig(c) o.Expect(err).To(o.BeNil()) @@ -92,7 +99,7 @@ var _ = g.Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator`, } }) - g.It("should create proposals", g.Label("Serial"), oteginkgo.Informing(), func() { + g.It("should create proposals", g.Label("OTA-1966"), g.Label("Serial"), oteginkgo.Informing(), func() { o.Expect(util.SkipIfNetworkRestricted(ctx, c, util.FauxinnatiAPIURL)).To(o.BeNil()) util.SkipIfNotTechPreviewNoUpgrade(ctx, c) @@ -101,21 +108,32 @@ var _ = g.Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator`, g.By("Using fauxinnati as the upstream and its simple channel") cv.Spec.Upstream = util.FauxinnatiAPIURL - cv.Spec.Channel = "simple" + channel := "simple" + cv.Spec.Channel = channel _, err = configClient.ClusterVersions().Update(ctx, cv, metav1.UpdateOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) needRecover = true + now := time.Now() + + g.By("Checking if the namespace exists") + _, err = kubeClient.CoreV1().Namespaces().Get(ctx, proposal.DefaultConfig().Namespace, metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) g.By("Checking if the proposal are created") + var proposals proposalv1alpha1.ProposalList o.Expect(wait.PollUntilContextTimeout(ctx, 30*time.Second, 5*time.Minute, true, func(ctx context.Context) (done bool, err error) { - proposals := proposalv1alpha1.ProposalList{} - err = rtClient.List(ctx, &proposals, ctrlruntimeclient.InNamespace(external.DefaultCVONamespace)) + proposals = proposalv1alpha1.ProposalList{} + err = rtClient.List(ctx, &proposals, ctrlruntimeclient.InNamespace(proposal.DefaultConfig().Namespace), + ctrlruntimeclient.MatchingLabels(proposal.CVOProposalLabels)) o.Expect(err).NotTo(o.HaveOccurred()) - if len(proposals.Items) == 0 { - return false, nil + for _, proposal := range proposals.Items { + if proposal.CreationTimestamp.After(now) || + strings.Contains(proposal.Spec.Request, fmt.Sprintf("Channel: %s", channel)) { + return true, nil + } } - return true, nil - })).NotTo(o.HaveOccurred(), "no proposals found") + return false, nil + })).NotTo(o.HaveOccurred(), "expected proposals not found", "now", now.Format(time.RFC3339), "proposals are:\n%s", getYaml(&proposals)) }) }) From d38fb549e9dd55e97e8c67ae6af654ebe0fd6aa5 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Fri, 24 Apr 2026 12:41:24 -0400 Subject: [PATCH 3/6] Reconcile proposals if cv.status.history got pruned --- pkg/cvo/status.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/cvo/status.go b/pkg/cvo/status.go index 1687e8d06..efa56fb01 100644 --- a/pkg/cvo/status.go +++ b/pkg/cvo/status.go @@ -182,6 +182,12 @@ func (optr *Operator) syncStatus(ctx context.Context, original, config *configv1 if klog.V(6).Enabled() { klog.Infof("Apply config: %s", cmp.Diff(original, config)) } + if optr.shouldEnableProposalController() { + if original != nil && len(config.Status.History) < len(original.Status.History) { + klog.V(internal.Normal).Infof("Reconciling proposals because ClusterVersion.status.history got pruned") + optr.proposalController.Queue().Add(optr.proposalController.QueueKey()) + } + } updated, err := applyClusterVersionStatus(ctx, optr.client.ConfigV1(), config, original) optr.rememberLastUpdate(updated) return err From 42efd6d3ec332425bc951aa8c98e2473eb19c895 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Tue, 28 Apr 2026 14:15:52 -0400 Subject: [PATCH 4/6] OTA-1971: Add ConfigMap/ota-advisory-prompt The manifest is taken from [1]. It is used to build the request for the proposals. The Workflow is removed from the implementation. There is no CRD defined for it. CVO does not maintain its own Agent. For testing purpose, it uses a default agent `smart` maintained by a cluster admin. [1]. https://github.com/openshift/cluster-version-operator/pull/1379/commits/07f4de97ec88093db60fc1129c091021fa7706ec --- ...ersion-operator_50_lightspeed-prompts.yaml | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 install/0000_00_cluster-version-operator_50_lightspeed-prompts.yaml diff --git a/install/0000_00_cluster-version-operator_50_lightspeed-prompts.yaml b/install/0000_00_cluster-version-operator_50_lightspeed-prompts.yaml new file mode 100644 index 000000000..18fd119cc --- /dev/null +++ b/install/0000_00_cluster-version-operator_50_lightspeed-prompts.yaml @@ -0,0 +1,29 @@ +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ota-advisory-prompt + namespace: openshift-lightspeed + annotations: + release.openshift.io/feature-gate: LightspeedProposals +data: + prompt: | + You are an OpenShift upgrade advisor. Analyze the cluster readiness + data in the proposal request and produce an upgrade risk assessment. + + The request contains a "Cluster Readiness Data" section with a JSON + block. This was collected by the Cluster Version Operator — do not + re-collect it. Parse the JSON, evaluate each check's results, and + classify findings as blockers, warnings, or informational. + + Use the ota-upgrade-advisor skill for the decision framework and + blocker classification rules. When findings need deeper investigation, + use prometheus, platform-docs, redhat-support, or product-lifecycle + skills. + + When the readiness data includes olm_operator_lifecycle results, use + the product-lifecycle skill to cross-reference each operator's package + name against the Red Hat Product Life Cycle API. Report support phase, + EOL dates, and OCP compatibility from PLCC alongside the OLM data. + + Do not guess or assume cluster state. Do not execute upgrade commands. From dfbeea4351cd4e5913090b3c050bd6f6ed999182 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Tue, 28 Apr 2026 14:22:15 -0400 Subject: [PATCH 5/6] Add manifest for ns/openshift-lightspeed --- ...version-operator_45_openshift-lightspeed_namespace.yaml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 install/0000_00_cluster-version-operator_45_openshift-lightspeed_namespace.yaml diff --git a/install/0000_00_cluster-version-operator_45_openshift-lightspeed_namespace.yaml b/install/0000_00_cluster-version-operator_45_openshift-lightspeed_namespace.yaml new file mode 100644 index 000000000..c6fa7425f --- /dev/null +++ b/install/0000_00_cluster-version-operator_45_openshift-lightspeed_namespace.yaml @@ -0,0 +1,7 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: openshift-lightspeed + annotations: + kubernetes.io/description: This manifest is only for testing purpose and will be removed when its own operator becomes available on the cluster. + include.release.openshift.io/self-managed-high-availability: "true" From 1a0dcef26b7370041d28594498eb7af33d036d18 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Wed, 29 Apr 2026 10:19:09 -0400 Subject: [PATCH 6/6] Fix CR's annotations --- ...0000_00_cluster-version-operator_50_lightspeed-prompts.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/install/0000_00_cluster-version-operator_50_lightspeed-prompts.yaml b/install/0000_00_cluster-version-operator_50_lightspeed-prompts.yaml index 18fd119cc..c22cf996d 100644 --- a/install/0000_00_cluster-version-operator_50_lightspeed-prompts.yaml +++ b/install/0000_00_cluster-version-operator_50_lightspeed-prompts.yaml @@ -5,7 +5,8 @@ metadata: name: ota-advisory-prompt namespace: openshift-lightspeed annotations: - release.openshift.io/feature-gate: LightspeedProposals + include.release.openshift.io/self-managed-high-availability: "true" + release.openshift.io/feature-set: TechPreviewNoUpgrade data: prompt: | You are an OpenShift upgrade advisor. Analyze the cluster readiness