Skip to content

[OTA-1975] pass products-data cincinnati endpoint to plcc #1429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ankitathomas wants to merge 1 commit into
base: main
Choose a base branch
from
+183 −10
Open

[OTA-1975] pass products-data cincinnati endpoint to plcc #1429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 67 additions & 7 deletions pkg/agenticrun/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"k8s.io/klog/v2"

configv1 "github.com/openshift/api/config/v1"
routev1 "github.com/openshift/api/route/v1"
agenticrunv1alpha1 "github.com/openshift/lightspeed-agentic-operator/api/v1alpha1"

i "github.com/openshift/cluster-version-operator/pkg/internal"
Expand Down Expand Up @@ -258,7 +259,7 @@ func (c *Controller) Sync(ctx context.Context, key string) error {
return kutilerrors.NewAggregate(errs)
}

agenticRuns, err := getAgenticRuns(ctx, c.dynamicClient, updates, conditionalUpdates, c.config.Namespace, currentVersion, cv.Spec.Channel, prompt, c.config.SkillsImage)
agenticRuns, err := getAgenticRuns(ctx, c.dynamicClient, c.client, updates, conditionalUpdates, c.config.Namespace, currentVersion, cv.Spec.Channel, prompt, c.config.SkillsImage)
if err != nil {
klog.V(i.Normal).Infof("Getting agentic runs hit an error: %v", err)
return kutilerrors.NewAggregate(append(errs, err))
Expand Down Expand Up @@ -383,6 +384,7 @@ func deleteAgenticRun(ctx context.Context, client ctrlruntimeclient.Client, agen
func getAgenticRuns(
ctx context.Context,
dynamicClient dynamic.Interface,
client ctrlruntimeclient.Client,
availableUpdates []configv1.Release,
conditionalUpdates []configv1.ConditionalUpdate,
namespace string,
Expand All @@ -399,7 +401,7 @@ func getAgenticRuns(
for _, au := range availableUpdates {
targetVersion := au.Version
readinessJSON := runReadinessJSON(ctx, dynamicClient, currentVersion, targetVersion)
if agenticRun, err := getAgenticRun(namespace, currentVersion, targetVersion, channel, updateKindRecommended, systemPrompt, readinessJSON, availableUpdates, skillsImage); err != nil {
if agenticRun, err := getAgenticRun(ctx, client, namespace, currentVersion, targetVersion, channel, updateKindRecommended, systemPrompt, readinessJSON, availableUpdates, skillsImage); err != nil {
errs = append(errs, err)
continue
} else {
Expand All @@ -410,7 +412,7 @@ func getAgenticRuns(
for _, cu := range conditionalUpdates {
targetVersion := cu.Release.Version
readinessJSON := runReadinessJSON(ctx, dynamicClient, currentVersion, targetVersion)
if agenticRun, err := getAgenticRun(namespace, currentVersion, targetVersion, channel, updateKindConditional, systemPrompt, readinessJSON, availableUpdates, skillsImage); err != nil {
if agenticRun, err := getAgenticRun(ctx, client, namespace, currentVersion, targetVersion, channel, updateKindConditional, systemPrompt, readinessJSON, availableUpdates, skillsImage); err != nil {
errs = append(errs, err)
continue
} else {
Expand All @@ -421,7 +423,7 @@ func getAgenticRuns(
return agenticRuns, kutilerrors.NewAggregate(errs)
}

func getAgenticRun(namespace, currentVersion, targetVersion, channel, updateKind, systemPrompt, readinessJSON string, availableUpdates []configv1.Release, skillsImage string) (*agenticrunv1alpha1.AgenticRun, error) {
func getAgenticRun(ctx context.Context, client ctrlruntimeclient.Client, namespace, currentVersion, targetVersion, channel, updateKind, systemPrompt, readinessJSON string, availableUpdates []configv1.Release, skillsImage string) (*agenticrunv1alpha1.AgenticRun, error) {

var errs []error
for _, v := range []string{currentVersion, targetVersion} {
Expand All @@ -433,9 +435,18 @@ func getAgenticRun(namespace, currentVersion, targetVersion, channel, updateKind
return nil, kutilerrors.NewAggregate(errs)
}

// Discover Cincinnati URL
cincinnatiURL, err := discoverCincinnatiURL(ctx, client)
if err != nil {
klog.V(i.Normal).Infof("Could not discover Cincinnati URL: %v (skills will use public API only)", err)
cincinnatiURL = ""
} else {
klog.V(i.Normal).Infof("Discovered Cincinnati URL: %s", cincinnatiURL)
}

name := agenticRunName(currentVersion, targetVersion)
updateType := classifyUpdate(currentVersion, targetVersion)
request := buildRequest(systemPrompt, currentVersion, targetVersion, channel, updateType, updateKind, availableUpdates, readinessJSON)
request := buildRequest(systemPrompt, currentVersion, targetVersion, channel, updateType, updateKind, availableUpdates, readinessJSON, cincinnatiURL)
Comment thread
ankitathomas marked this conversation as resolved.
return &agenticrunv1alpha1.AgenticRun{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Expand Down Expand Up @@ -545,9 +556,52 @@ func runReadinessJSON(ctx context.Context, dynamicClient dynamic.Interface, curr
return string(data)
}

// discoverCincinnatiURL discovers the Cincinnati service URL via Kubernetes APIs
func discoverCincinnatiURL(ctx context.Context, client ctrlruntimeclient.Client) (string, error) {
// List services in openshift-update-service namespace
services := &corev1.ServiceList{}
err := client.List(ctx, services,
ctrlruntimeclient.InNamespace("openshift-update-service"))
if err != nil {
return "", fmt.Errorf("failed to list services in openshift-update-service: %w", err)
}

// Find Cincinnati service (there's only one per cluster)
var cincinnatiService *corev1.Service
for i := range services.Items {
svc := &services.Items[i]
if svc.Labels["app"] == "updateservice" {
cincinnatiService = svc
break
}
}

if cincinnatiService == nil {
return "", fmt.Errorf("Cincinnati service not found (no service with label app=updateservice)")
}

// Find corresponding route
routes := &routev1.RouteList{}
err = client.List(ctx, routes,
ctrlruntimeclient.InNamespace("openshift-update-service"))
if err != nil {
return "", fmt.Errorf("failed to list routes in openshift-update-service: %w", err)
}

for i := range routes.Items {
route := &routes.Items[i]
if route.Spec.To.Name == cincinnatiService.Name {
// Found the route, construct URL and return
return fmt.Sprintf("https://%s", route.Spec.Host), nil
}
}

return "", fmt.Errorf("Cincinnati route not found for service %s", cincinnatiService.Name)
}

// buildRequest constructs the agentic run request with system prompt, metadata, and readiness data.
func buildRequest(systemPrompt, current, target, channel, updateType, targetType string,
updates []configv1.Release, readinessJSON string) string {
updates []configv1.Release, readinessJSON, cincinnatiURL string) string {

var b strings.Builder

Expand All @@ -560,7 +614,13 @@ func buildRequest(systemPrompt, current, target, channel, updateType, targetType
_, _ = 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, "Update path: %s\n", targetType)

if cincinnatiURL != "" {
_, _ = fmt.Fprintf(&b, "Cincinnati URL: %s\n", cincinnatiURL)
}

b.WriteString("\n")

if targetType == updateKindConditional {
b.WriteString("WARNING: This target version is available as a CONDITIONAL update.\n")
Expand Down
119 changes: 116 additions & 3 deletions pkg/agenticrun/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
dynamicfake "k8s.io/client-go/dynamic/fake"

configv1 "github.com/openshift/api/config/v1"
routev1 "github.com/openshift/api/route/v1"
agenticrunv1alpha1 "github.com/openshift/lightspeed-agentic-operator/api/v1alpha1"

"github.com/openshift/cluster-version-operator/pkg/internal"
Expand Down Expand Up @@ -230,7 +231,7 @@ func TestBuildRequest(t *testing.T) {
}

t.Run("recommended target", func(t *testing.T) {
request := buildRequest("", "4.15.3", "4.16.0", "stable-4.16", "minor", "recommended", updates, "")
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")
}
Expand All @@ -255,7 +256,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")
}
Expand All @@ -265,7 +266,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")
}
Expand Down Expand Up @@ -1078,6 +1079,7 @@ Other recommended versions available:
agenticRuns, err := getAgenticRuns(
context.Background(),
nil,
nil,
tt.availableUpdates,
tt.conditionalUpdates,
tt.namespace,
Expand Down Expand Up @@ -1297,6 +1299,7 @@ func TestGetAgenticRuns_WithReadinessData(t *testing.T) {
agenticRuns, err := getAgenticRuns(
context.Background(),
dc,
nil,
[]configv1.Release{{Version: "4.21.8"}},
nil,
"openshift-lightspeed",
Expand Down Expand Up @@ -1407,3 +1410,113 @@ func TestGetAgenticRuns_WithReadinessData(t *testing.T) {
t.Errorf("node_capacity total_nodes = %v, want 2", nc["total_nodes"])
}
}

func TestDiscoverCincinnatiURL(t *testing.T) {
tests := []struct {
name string
objects []runtime.Object
expectedURL string
expectError bool
errorContains string
}{
{
name: "successfully discovers Cincinnati URL",
objects: []runtime.Object{
&corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "cincinnati",
Namespace: "openshift-update-service",
Labels: map[string]string{
"app": "updateservice",
},
},
},
&routev1.Route{
ObjectMeta: metav1.ObjectMeta{
Name: "cincinnati",
Namespace: "openshift-update-service",
},
Spec: routev1.RouteSpec{
Host: "cincinnati.example.com",
To: routev1.RouteTargetReference{
Name: "cincinnati",
},
},
},
},
expectedURL: "https://cincinnati.example.com",
},
{
name: "service not found",
objects: []runtime.Object{},
expectError: true,
errorContains: "Cincinnati service not found",
},
{
name: "route not found",
objects: []runtime.Object{
&corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "cincinnati",
Namespace: "openshift-update-service",
Labels: map[string]string{
"app": "updateservice",
},
},
},
},
expectError: true,
errorContains: "Cincinnati route not found",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
scheme := runtime.NewScheme()
_ = routev1.Install(scheme)
_ = corev1.AddToScheme(scheme)
client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(tt.objects...).Build()

url, err := discoverCincinnatiURL(context.Background(), client)

if tt.expectError {
if err == nil {
t.Errorf("expected error containing %q, got nil", tt.errorContains)
} else if !strings.Contains(err.Error(), tt.errorContains) {
t.Errorf("expected error containing %q, got %q", tt.errorContains, err.Error())
}
} else {
if err != nil {
t.Errorf("unexpected error: %v", err)
}
if url != tt.expectedURL {
t.Errorf("discoverCincinnatiURL() = %q, want %q", url, tt.expectedURL)
}
}
})
}
}

func TestBuildRequest_WithCincinnatiURL(t *testing.T) {
updates := []configv1.Release{
{Version: "4.16.0"},
}

t.Run("includes Cincinnati URL when provided", func(t *testing.T) {
request := buildRequest("", "4.15.3", "4.16.0", "stable-4.16", "minor",
"Recommended", updates, "{}", "https://cincinnati.example.com")

if !strings.Contains(request, "Cincinnati URL: https://cincinnati.example.com") {
t.Error("request should contain Cincinnati URL")
}
})

t.Run("omits Cincinnati URL when empty", func(t *testing.T) {
request := buildRequest("", "4.15.3", "4.16.0", "stable-4.16", "minor",
"Recommended", updates, "{}", "")

if strings.Contains(request, "Cincinnati URL:") {
t.Error("request should not contain Cincinnati URL line when empty")
}
})
}