feat(controller): address PR #73 review comments for OCP RAG

- Rename fields to include RAG (OCPRAGVersionOverride, ActiveOCPRAGVersion)
- Default EnableOCPRAG to false (standard image doesn't include OCP docs)
- Simplify status: remove DetectedOCPVersion, OCPVersionFallback, print column
- Remove priority field from RAG configs (not in OLSConfig API)
- Remove override validation to support custom RAG images
- Add ClusterVersion watch for OCP upgrade detection
- Improve error handling and add design decision comments
- Update tests for new return types and removed validations

Addresses feedback from @lpiwowar and @Akrog.

JIRA: OSPRH-23918

Author: omkarjoshi0304

api/v1beta1/conditions.go

@@ -1,9 +1,12 @@
 /*
 Copyright 2025.
+
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
+
     http://www.apache.org/licenses/LICENSE-2.0
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -23,12 +26,15 @@ const (
 	// is configured and operational
 	OpenStackLightspeedReadyCondition condition.Type = "OpenStackLightspeedReady"
 
-	// OpenShift Lightspeed Operator Status=True condition which indicates if OpenShift Lightspeed is installed and
-	// operational and it can be used by OpenStack Lihgtspeed operator.
+	// OpenShiftLightspeedOperatorReadyCondition Status=True condition which indicates if OpenShift Lightspeed is installed and
+	// operational and it can be used by OpenStack Lightspeed operator.
 	OpenShiftLightspeedOperatorReadyCondition condition.Type = "OpenShiftLightspeedOperatorReady"
 
-	// OCPVersionCondition Status=True condition which indicates OCP version detection and resolution
-	OCPVersionCondition condition.Type = "OCPVersionResolved"
+	// OCPRAGCondition Status=True condition which indicates the OCP RAG version resolution status
+	OCPRAGCondition condition.Type = "OCPRAGReady"
+
+	// OCPVersionCondition - DEPRECATED: Use OCPRAGCondition instead (keeping for backward compatibility)
+	OCPVersionCondition condition.Type = "OCPRAGReady"
 )
 
 // Common Messages used by API objects.
@@ -45,18 +51,27 @@ const (
 	// OpenShiftLightspeedOperatorWaiting
 	OpenShiftLightspeedOperatorWaiting = "Waiting for the OpenShift Lightspeed operator to deploy."
 
-	// OpenShiftLigthspeedOperatorReady
+	// OpenShiftLightspeedOperatorReady
 	OpenShiftLightspeedOperatorReady = "OpenShift Lightspeed operator is ready."
 
-	// OCPVersionDetected message when OCP version is detected and resolved successfully
-	OCPVersionDetected = "OCP version detected and resolved successfully"
+	// OCPRAGDisabledMessage
+	OCPRAGDisabledMessage = "OCP RAG is disabled"
 
-	// OCPVersionFallback message when using 'latest' OCP documentation as fallback
-	OCPVersionFallback = "Using 'latest' OCP documentation as fallback for unsupported cluster version"
+	// OCPRAGVersionResolvedMessage
+	OCPRAGVersionResolvedMessage = "OCP RAG version resolved: %s"
 
-	// OCPVersionOverrideInvalid message when OCP version override is invalid
-	OCPVersionOverrideInvalid = "OCP version override is invalid"
+	// OCPRAGVersionFallbackMessage
+	OCPRAGVersionFallbackMessage = "Cluster version %s is not explicitly supported. Using 'latest' OCP documentation. Supported versions: %v"
 
-	// OCPVersionDetectionFailed message when OCP version detection fails
+	// OCPRAGDetectionFailedMessage
+	OCPRAGDetectionFailedMessage = "Failed to detect OCP cluster version"
+
+	// OCPRAGOverrideInvalidMessage
+	OCPRAGOverrideInvalidMessage = "Invalid OCP RAG version override"
+
+	// OCPVersionDetectionFailed - DEPRECATED: Use OCPRAGDetectionFailedMessage
 	OCPVersionDetectionFailed = "Failed to detect OCP cluster version"
+
+	// OCPVersionOverrideInvalid - DEPRECATED: Use OCPRAGOverrideInvalidMessage
+	OCPVersionOverrideInvalid = "Invalid OCP RAG version override"
 )

api/v1beta1/openstacklightspeed_types.go

@@ -39,14 +39,14 @@ type OpenStackLightspeedSpec struct {
 	RAGImage string `json:"ragImage"`
 
 	// +kubebuilder:validation:Optional
-	// +kubebuilder:default=true
+	// +kubebuilder:default=false
 	// EnableOCPRAG enables automatic OCP documentation based on cluster version
 	EnableOCPRAG bool `json:"enableOCPRAG,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// OCPVersionOverride allows forcing a specific OCP version instead of auto-detection.
 	// Format should be like "4.15", "4.16", etc.
-	OCPVersionOverride string `json:"ocpVersionOverride,omitempty"`
+	OCPRAGVersionOverride string `json:"ocpVersionOverride,omitempty"`
 }
 
 // OpenStackLightspeedCore defines the desired state of OpenStackLightspeed
@@ -117,22 +117,15 @@ type OpenStackLightspeedStatus struct {
 	// ObservedGeneration - the most recent generation observed for this object.
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 
-	// DetectedOCPVersion contains the auto-detected OCP cluster version
-	DetectedOCPVersion string `json:"detectedOCPVersion,omitempty"`
-
 	// ActiveOCPVersion contains the OCP version being used for RAG configuration
 	// Will be one of: "4.16", "4.18", "latest", or empty if OCP RAG is disabled
-	ActiveOCPVersion string `json:"activeOCPVersion,omitempty"`
-
-	// OCPVersionFallback indicates if using 'latest' as fallback for unsupported version
-	OCPVersionFallback bool `json:"ocpVersionFallback,omitempty"`
+	ActiveOCPRAGVersion string `json:"activeOCPVersion,omitempty"`
 }
 
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.conditions[0].status",description="Status"
 // +kubebuilder:printcolumn:name="Message",type="string",JSONPath=".status.conditions[0].message",description="Message"
-// +kubebuilder:printcolumn:name="OCP Version",type="string",JSONPath=".status.activeOCPVersion"
 
 // OpenStackLightspeed is the Schema for the openstacklightspeeds API
 type OpenStackLightspeed struct {

bundle/manifests/openstack-lightspeed-operator.clusterserviceversion.yaml

@@ -88,6 +88,7 @@ spec:
           verbs:
           - get
           - list
+          - watch
         - apiGroups:
           - lightspeed.openstack.org
           resources:

config/crd/bases/lightspeed.openstack.org_openstacklightspeeds.yaml

@@ -23,9 +23,6 @@ spec:
       jsonPath: .status.conditions[0].message
       name: Message
       type: string
-    - jsonPath: .status.activeOCPVersion
-      name: OCP Version
-      type: string
     name: v1beta1
     schema:
       openAPIV3Schema:
@@ -62,7 +59,7 @@ spec:
                   operator is located
                 type: string
               enableOCPRAG:
-                default: true
+                default: false
                 description: EnableOCPRAG enables automatic OCP documentation based
                   on cluster version
                 type: boolean
@@ -178,19 +175,11 @@ spec:
                   - type
                   type: object
                 type: array
-              detectedOCPVersion:
-                description: DetectedOCPVersion contains the auto-detected OCP cluster
-                  version
-                type: string
               observedGeneration:
                 description: ObservedGeneration - the most recent generation observed
                   for this object.
                 format: int64
                 type: integer
-              ocpVersionFallback:
-                description: OCPVersionFallback indicates if using 'latest' as fallback
-                  for unsupported version
-                type: boolean
             type: object
         type: object
     served: true

config/rbac/role.yaml

@@ -11,6 +11,7 @@ rules:
   verbs:
   - get
   - list
+  - watch
 - apiGroups:
   - lightspeed.openstack.org
   resources:

internal/controller/funcs.go

@@ -137,16 +137,15 @@ func GetOLSConfig(ctx context.Context, helper *common_helper.Helper) (uns.Unstru
 		"OLSConfig")
 }
 
-// BuildRAGConfigs builds the RAG configuration array with priorities.
-// OpenStack RAG is always included with priority 1.
-// OCP RAG is added with priority 2 if ocpVersion is provided.
-func BuildRAGConfigs(instance *apiv1beta1.OpenStackLightspeed, ocpVersion string) []map[string]interface{} {
-	rags := []map[string]interface{}{
-		// OpenStack RAG - Priority 1
-		{
+// BuildRAGConfigs builds the RAG configuration array.
+// OpenStack RAG is always included first.
+// OCP RAG is added if ocpVersion is provided.
+func BuildRAGConfigs(instance *apiv1beta1.OpenStackLightspeed, ocpVersion string) []interface{} {
+	rags := []interface{}{
+		// OpenStack RAG
+		map[string]interface{}{
 			"image":     instance.Spec.RAGImage,
 			"indexPath": OpenStackLightspeedVectorDBPath,
-			"priority":  1,
 		},
 	}
 
@@ -156,7 +155,6 @@ func BuildRAGConfigs(instance *apiv1beta1.OpenStackLightspeed, ocpVersion string
 			"image":     instance.Spec.RAGImage,
 			"indexPath": GetOCPVectorDBPath(ocpVersion),
 			"indexID":   GetOCPIndexName(ocpVersion),
-			"priority":  2,
 		})
 	}
 
@@ -168,7 +166,6 @@ func PatchOLSConfig(
 	helper *common_helper.Helper,
 	instance *apiv1beta1.OpenStackLightspeed,
 	olsConfig *uns.Unstructured,
-	ocpVersion string,
 ) error {
 	// Patch the Providers section
 	providersPatch := []interface{}{
@@ -215,13 +212,9 @@ func PatchOLSConfig(
 
 	// Patch the RAG section
 	// Build RAG array with priorities using BuildRAGConfigs
-	ragConfigs := BuildRAGConfigs(instance, ocpVersion)
-	ragSlice := make([]interface{}, len(ragConfigs))
-	for i, rag := range ragConfigs {
-		ragSlice[i] = rag
-	}
+	ragConfigs := BuildRAGConfigs(instance, instance.Status.ActiveOCPRAGVersion)
 
-	if err := uns.SetNestedSlice(olsConfig.Object, ragSlice, "spec", "ols", "rag"); err != nil {
+	if err := uns.SetNestedSlice(olsConfig.Object, ragConfigs, "spec", "ols", "rag"); err != nil {
 		return err
 	}
 

internal/controller/ocp_version.go

@@ -66,10 +66,17 @@ func DetectOCPVersion(ctx context.Context, helper *common_helper.Helper) (string
 	}
 
 	// Extract version from status.desired.version
+	// NOTE: We intentionally use desired.version rather than history[0].version because:
+	// - During OCP upgrades, desired.version reflects the target version
+	// - Users troubleshooting upgrade issues need docs for the NEW version
+	// - This provides proactive access to relevant documentation
 	version, found, err := uns.NestedString(clusterVersion.Object, "status", "desired", "version")
-	if !found || err != nil {
+	if err != nil {
 		return "", fmt.Errorf("failed to extract version from ClusterVersion: %w", err)
 	}
+	if !found {
+		return "", fmt.Errorf("version field not found in ClusterVersion status.desired.version")
+	}
 
 	// Parse version to get major.minor (e.g., "4.15.0" -> "4.15")
 	majorMinor, err := ParseMajorMinorVersion(version)
@@ -139,12 +146,7 @@ func ResolveOCPVersion(detectedVersion, overrideVersion string, enableOCPRAG boo
 
 	// Use override if provided
 	if overrideVersion != "" {
-		// Check if override is a supported version
-		if IsSupportedOCPVersion(overrideVersion) {
-			return overrideVersion, false, nil
-		}
-		return "", false, fmt.Errorf("override version %s is not supported, must be one of: %v",
-			overrideVersion, SupportedOCPVersions)
+		return overrideVersion, false, nil
 	}
 
 	// Use detected version