diff --git a/.openshift-tests-extension/openshift_payload_cluster-version-operator.json b/.openshift-tests-extension/openshift_payload_cluster-version-operator.json index 9fecf93c23..928c2adf63 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/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 0000000000..c6fa7425f0 --- /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" 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 0000000000..c22cf996dd --- /dev/null +++ b/install/0000_00_cluster-version-operator_50_lightspeed-prompts.yaml @@ -0,0 +1,30 @@ +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: ota-advisory-prompt + namespace: openshift-lightspeed + annotations: + 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 + 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. diff --git a/pkg/cvo/availableupdates_test.go b/pkg/cvo/availableupdates_test.go index 71a291f660..aec0c29ae3 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 b52491d778..248f4bd0d6 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 92585a1e42..ab3ff4e43d 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/cvo/status.go b/pkg/cvo/status.go index 1687e8d060..efa56fb01d 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 diff --git a/pkg/internal/constants.go b/pkg/internal/constants.go index eac0f36af3..568e038dbc 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 74d4934915..f6ddb52921 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 4a803cfa91..9f270a6212 100644 --- a/pkg/proposal/controller.go +++ b/pkg/proposal/controller.go @@ -3,14 +3,20 @@ package proposal 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" @@ -21,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. @@ -41,18 +54,48 @@ 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 + 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"), + 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 } @@ -76,16 +119,45 @@ 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) + 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) - } else if err := deleteProposals(c.client, updates, conditionalUpdates, cv.Status.History); 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 { @@ -98,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 } @@ -111,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) @@ -126,64 +200,295 @@ 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)) } -// TODO: -func deleteProposals(_ ctrlruntimeclient.Client, _ []configv1.Release, _ []configv1.ConditionalUpdate, _ []configv1.UpdateHistory) error { +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) +} + +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 toDNS1035(fmt.Sprintf("ota-%s-to-%s", current, target)) +} + +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 ( + 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" + + proposalExpiration = 24 * time.Hour +) + +var ( + CVOProposalLabels = map[string]string{labelKeySource: labelValueSource} +) + +// 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 i.UpdateTypeUnknown + } + return i.UpdateType(cv, tv) +} + +// 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 24e27b6e45..847b795379 100644 --- a/pkg/proposal/controller_test.go +++ b/pkg/proposal/controller_test.go @@ -3,12 +3,20 @@ package proposal import ( "context" "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" @@ -25,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 @@ -34,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 { @@ -45,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 }, @@ -54,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 { @@ -72,3 +164,1072 @@ 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: "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 { + 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 Test_labelValueFromVersion(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {"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 := labelValueFromVersion(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") + } + }) +} + +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 a37e12888d..0246fe9a1f 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 ffae59b814..d46fc8c59f 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)) }) })