Merge branch 'main' into fix-flaky-test

controllers/routingctrl/reconcile_resources.go

@@ -15,7 +15,6 @@ import (
 	"github.com/opendatahub-io/odh-platform/pkg/unstruct"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
@@ -69,22 +68,13 @@ func (r *Controller) createRoutingResources(ctx context.Context, target *unstruc
 func (r *Controller) exportService(ctx context.Context, target *unstructured.Unstructured, exportedSvc *corev1.Service, domain string) error {
 	exportModes := r.extractExportModes(target)
 
-	templateData := spi.RoutingTemplateData{
-		PlatformRoutingConfiguration: r.config,
-		PublicServiceName:            exportedSvc.GetName() + "-" + exportedSvc.GetNamespace(),
-		ServiceName:                  exportedSvc.GetName(),
-		ServiceNamespace:             exportedSvc.GetNamespace(),
-		ServiceTargetPort:            exportedSvc.Spec.Ports[0].TargetPort.String(),
-		Domain:                       domain,
-	}
+	templateData := spi.NewRoutingData(r.config, exportedSvc, domain)
 
 	// To establish ownership for watched component
 	ownershipLabels := append(labels.AsOwner(target), labels.AppManagedBy("odh-routing-controller"))
 
-	targetKey := client.ObjectKeyFromObject(target)
-
 	for _, exportMode := range exportModes {
-		resources, err := r.templateLoader.Load(ctx, exportMode, targetKey, templateData)
+		resources, err := r.templateLoader.Load(templateData, exportMode)
 		if err != nil {
 			return fmt.Errorf("could not load templates for type %s: %w", exportMode, err)
 		}
@@ -98,7 +88,7 @@ func (r *Controller) exportService(ctx context.Context, target *unstructured.Uns
 	return r.propagateHostsToWatchedCR(target, templateData)
 }
 
-func (r *Controller) propagateHostsToWatchedCR(target *unstructured.Unstructured, data spi.RoutingTemplateData) error {
+func (r *Controller) propagateHostsToWatchedCR(target *unstructured.Unstructured, data *spi.RoutingData) error {
 	exportModes := r.extractExportModes(target)
 
 	// Remove all existing routing addresses

pkg/metadata/labels/funcs.go

@@ -38,7 +38,12 @@ func StandardLabelsFrom(source metav1.Object) []metadata.Option {
 // AppendStandardLabelsFrom appends standard labels found in source object but only
 // when they are not already present in the target object.
 func AppendStandardLabelsFrom(source metav1.Object) *LabelAppender {
-	return &LabelAppender{labels: standardLabelsFrom(source)}
+	return AppendLabels(standardLabelsFrom(source)...)
+}
+
+// AppendLabels appends provided labels to the target object but only when they are not already present.
+func AppendLabels(labels ...Label) *LabelAppender {
+	return &LabelAppender{labels: labels}
 }
 
 // MatchingLabels returns a client.MatchingLabels selector for the provided labels.

pkg/routing/routing.go

@@ -2,7 +2,6 @@ package routing
 
 import (
 	"bytes"
-	"context"
 	_ "embed" // needed for go:embed directive
 	"fmt"
 	"strings"
@@ -11,7 +10,6 @@ import (
 	"github.com/opendatahub-io/odh-platform/pkg/schema"
 	"github.com/opendatahub-io/odh-platform/pkg/spi"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/types"
 )
 
 //go:embed template/routing_public.yaml
@@ -27,7 +25,7 @@ func NewStaticTemplateLoader() spi.RoutingTemplateLoader {
 	return &staticTemplateLoader{}
 }
 
-func (s *staticTemplateLoader) Load(_ context.Context, routeType spi.RouteType, key types.NamespacedName, data spi.RoutingTemplateData) ([]*unstructured.Unstructured, error) {
+func (s *staticTemplateLoader) Load(data *spi.RoutingData, routeType spi.RouteType) ([]*unstructured.Unstructured, error) {
 	var resources []*unstructured.Unstructured
 
 	var templateContent []byte
@@ -60,7 +58,7 @@ func (s *staticTemplateLoader) Load(_ context.Context, routeType spi.RouteType,
 	return resources, nil
 }
 
-func (s *staticTemplateLoader) resolveTemplate(tmpl []byte, data spi.RoutingTemplateData) ([]byte, error) {
+func (s *staticTemplateLoader) resolveTemplate(tmpl []byte, data *spi.RoutingData) ([]byte, error) {
 	engine, err := template.New("routing").Parse(string(tmpl))
 	if err != nil {
 		return []byte{}, fmt.Errorf("could not create template engine: %w", err)

pkg/routing/routing_test.go

@@ -1,40 +1,53 @@
 package routing_test
 
 import (
-	"context"
-
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/opendatahub-io/odh-platform/pkg/routing"
 	"github.com/opendatahub-io/odh-platform/pkg/spi"
 	"github.com/opendatahub-io/odh-platform/test"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/ptr"
 )
 
 var _ = Describe("Resource functions", test.Unit(), func() {
 
 	Context("Template Loader", func() {
 
-		data := spi.RoutingTemplateData{
-			PlatformRoutingConfiguration: spi.PlatformRoutingConfiguration{
-				GatewayNamespace:     "opendatahub",
-				IngressSelectorLabel: "istio",
-				IngressSelectorValue: "rhoai-gateway",
-				IngressService:       "rhoai-router-ingress",
-			},
-			PublicServiceName: "registry-office",
-			ServiceName:       "registry",
-			ServiceNamespace:  "office",
-			Domain:            "app-crc.testing",
+		config := spi.PlatformRoutingConfiguration{
+			GatewayNamespace:     "opendatahub",
+			IngressSelectorLabel: "istio",
+			IngressSelectorValue: "rhoai-gateway",
+			IngressService:       "rhoai-router-ingress",
 		}
 
+		data := spi.NewRoutingData(config,
+			&corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "registry",
+					Namespace: "office",
+				},
+				Spec: corev1.ServiceSpec{
+					Ports: []corev1.ServicePort{
+						{
+							Name:        "http-api",
+							Port:        80,
+							AppProtocol: ptr.To("http"),
+						},
+					},
+				},
+			},
+			"app-crc.testing",
+		)
+
 		It("should load public resources", func() {
 			// given
 			// data^
 
 			// when
-			res, err := routing.NewStaticTemplateLoader().Load(context.Background(), spi.PublicRoute, types.NamespacedName{}, data)
+			res, err := routing.NewStaticTemplateLoader().Load(data, spi.PublicRoute)
 
 			// then
 			Expect(err).ShouldNot(HaveOccurred())
@@ -46,7 +59,7 @@ var _ = Describe("Resource functions", test.Unit(), func() {
 			// data^
 
 			// when
-			res, err := routing.NewStaticTemplateLoader().Load(context.Background(), spi.ExternalRoute, types.NamespacedName{}, data)
+			res, err := routing.NewStaticTemplateLoader().Load(data, spi.ExternalRoute)
 
 			// then
 			Expect(err).ShouldNot(HaveOccurred())

pkg/spi/types.go

@@ -9,6 +9,7 @@ import (
 
 	authorinov1beta2 "github.com/kuadrant/authorino/api/v1beta2"
 	"github.com/opendatahub-io/odh-platform/pkg/platform"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 )
@@ -130,8 +131,7 @@ type PlatformRoutingConfiguration struct {
 	GatewayNamespace string
 }
 
-// TODO(mvp) revise the stuct name - is it only for templates?
-type RoutingTemplateData struct {
+type RoutingData struct {
 	PlatformRoutingConfiguration
 
 	PublicServiceName string // [service-name]-[service-namespace]
@@ -143,10 +143,18 @@ type RoutingTemplateData struct {
 	Domain string
 }
 
-// RoutingTemplateLoader provides a way to differentiate the Route template used based on
-//   - RouteType
-//   - Namespace / Resource name
-//   - Loader source
+func NewRoutingData(config PlatformRoutingConfiguration, svc *corev1.Service, domain string) *RoutingData {
+	return &RoutingData{
+		PlatformRoutingConfiguration: config,
+		PublicServiceName:            svc.GetName() + "-" + svc.GetNamespace(),
+		ServiceName:                  svc.GetName(),
+		ServiceNamespace:             svc.GetNamespace(),
+		ServiceTargetPort:            svc.Spec.Ports[0].TargetPort.String(),
+		Domain:                       domain,
+	}
+}
+
+// RoutingTemplateLoader provides a way to differentiate the Route resource templates used based on its types.
 type RoutingTemplateLoader interface {
-	Load(ctx context.Context, routeType RouteType, key types.NamespacedName, data RoutingTemplateData) ([]*unstructured.Unstructured, error)
+	Load(data *RoutingData, routeType RouteType) ([]*unstructured.Unstructured, error)
 }