feat: implement VirtualService reconciliation

workspaces/controller/config/manager/kustomization.yaml

@@ -5,4 +5,4 @@ resources:
 images:
 - name: controller
   newName: ghcr.io/kubeflow/notebooks/workspace-controller
-  newTag: latest
+  newTag: latest

workspaces/controller/config/samples/jupyterlab_v1beta1_workspacekind.yaml

@@ -156,7 +156,7 @@ spec:
       requestHeaders: {}
         #set: { "X-RStudio-Root-Path": "{{ .PathPrefix }}" } # for RStudio
         #add: {}
-      #remove: []
+        #remove: []
 
     ## environment variables for Workspace Pods (MUTABLE)
     ##  - spec for EnvVar:

workspaces/controller/internal/controller/suite_test.go

@@ -70,7 +70,7 @@ var _ = BeforeSuite(func() {
 
 	By("bootstrapping test environment")
 	testEnv = &envtest.Environment{
-		CRDDirectoryPaths:     []string{filepath.Join("..", "..", "config", "crd", "bases")},
+		CRDDirectoryPaths:     []string{filepath.Join("..", "..", "config", "crd", "bases"), filepath.Join("..", "..", "test", "crd")},
 		ErrorIfCRDPathMissing: true,
 
 		// The BinaryAssetsDirectory is only required if you want to run the tests directly without call the makefile target test.

workspaces/controller/internal/controller/workspace_controller.go

@@ -19,6 +19,7 @@ package controller
 import (
 	"context"
 	"fmt"
+	"github.com/kubeflow/notebooks/workspaces/controller/internal/istio"
 	"reflect"
 	"strings"
 
@@ -53,8 +54,6 @@ const (
 	workspaceSelectorLabel = "statefulset"
 
 	// lengths for resource names
-	generateNameSuffixLength = 6
-	maxServiceNameLength     = 63
 	maxStatefulSetNameLength = 52 // https://github.com/kubernetes/kubernetes/issues/64023
 
 	// state message formats for Workspace status
@@ -342,10 +341,20 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		}
 	}
 
-	//
-	// TODO: reconcile the Istio VirtualService to expose the Workspace
-	//       and implement the `spec.podTemplate.httpProxy` options
-	//
+	// VirtualService reconciliation
+	virtualService, err := istio.GenerateIstioVirtualService(workspace, workspaceKind, currentImageConfig, serviceName, log)
+	if err != nil {
+		log.Error(err, "unable to generate Istio Virtual Service")
+	}
+	log.Info(fmt.Sprintf("VirtualService %s", virtualService))
+
+	if err := ctrl.SetControllerReference(workspace, virtualService, r.Scheme); err != nil {
+		return ctrl.Result{}, err
+	}
+
+	if err := istio.ReconcileVirtualService(ctx, r.Client, virtualService.GetName(), virtualService.GetNamespace(), virtualService, log); err != nil {
+		return ctrl.Result{}, err
+	}
 
 	// fetch Pod
 	// NOTE: the first StatefulSet Pod is always called "{statefulSetName}-0"
@@ -555,25 +564,10 @@ func getPodConfig(workspace *kubefloworgv1beta1.Workspace, workspaceKind *kubefl
 	}
 }
 
-// generateNamePrefix generates a name prefix for a Workspace
-// the format is "ws-{WORKSPACE_NAME}-" the workspace name is truncated to fit within the max length
-func generateNamePrefix(workspaceName string, maxLength int) string {
-	namePrefix := fmt.Sprintf("ws-%s", workspaceName)
-	maxLength = maxLength - generateNameSuffixLength // subtract 6 for the `metadata.generateName` suffix
-	maxLength = maxLength - 1                        // subtract 1 for the trailing "-"
-	if len(namePrefix) > maxLength {
-		namePrefix = namePrefix[:min(len(namePrefix), maxLength)]
-	}
-	if namePrefix[len(namePrefix)-1] != '-' {
-		namePrefix = namePrefix + "-"
-	}
-	return namePrefix
-}
-
 // generateStatefulSet generates a StatefulSet for a Workspace
 func generateStatefulSet(workspace *kubefloworgv1beta1.Workspace, workspaceKind *kubefloworgv1beta1.WorkspaceKind, imageConfigSpec kubefloworgv1beta1.ImageConfigSpec, podConfigSpec kubefloworgv1beta1.PodConfigSpec) (*appsv1.StatefulSet, error) {
 	// generate name prefix
-	namePrefix := generateNamePrefix(workspace.Name, maxStatefulSetNameLength)
+	namePrefix := helper.GenerateNamePrefix(workspace.Name, maxStatefulSetNameLength)
 
 	// generate replica count
 	replicas := int32(1)
@@ -591,17 +585,7 @@ func generateStatefulSet(workspace *kubefloworgv1beta1.Workspace, workspaceKind
 		imagePullPolicy = *imageConfigSpec.ImagePullPolicy
 	}
 
-	// define go string template functions
-	// NOTE: these are used in places like the `extraEnv` values
 	containerPortsIdMap := make(map[string]kubefloworgv1beta1.ImagePort)
-	httpPathPrefixFunc := func(portId string) string {
-		port, ok := containerPortsIdMap[portId]
-		if ok {
-			return fmt.Sprintf("/workspace/%s/%s/%s/", workspace.Namespace, workspace.Name, port.Id)
-		} else {
-			return ""
-		}
-	}
 
 	// generate container ports
 	containerPorts := make([]corev1.ContainerPort, len(imageConfigSpec.Ports))
@@ -620,14 +604,15 @@ func generateStatefulSet(workspace *kubefloworgv1beta1.Workspace, workspaceKind
 		// NOTE: we construct this map for use in the go string templates
 		containerPortsIdMap[port.Id] = port
 	}
+	httpPathPrefixFunc := helper.GenerateHttpPathPrefixFunc(workspace, containerPortsIdMap)
 
 	// generate container env
 	containerEnv := make([]corev1.EnvVar, len(workspaceKind.Spec.PodTemplate.ExtraEnv))
 	for i, env := range workspaceKind.Spec.PodTemplate.ExtraEnv {
 		env := env.DeepCopy() // copy to avoid modifying the original
 		if env.Value != "" {
 			rawValue := env.Value
-			outValue, err := helper.RenderExtraEnvValueTemplate(rawValue, httpPathPrefixFunc)
+			outValue, err := helper.RenderWithHttpPathPrefixFunc(rawValue, httpPathPrefixFunc)
 			if err != nil {
 				return nil, fmt.Errorf("failed to render extraEnv %q: %w", env.Name, err)
 			}
@@ -806,7 +791,7 @@ func generateStatefulSet(workspace *kubefloworgv1beta1.Workspace, workspaceKind
 // generateService generates a Service for a Workspace
 func generateService(workspace *kubefloworgv1beta1.Workspace, imageConfigSpec kubefloworgv1beta1.ImageConfigSpec) (*corev1.Service, error) {
 	// generate name prefix
-	namePrefix := generateNamePrefix(workspace.Name, maxServiceNameLength)
+	namePrefix := helper.GenerateNamePrefix(workspace.Name, helper.MaxServiceNameLength)
 
 	// generate service ports
 	servicePorts := make([]corev1.ServicePort, len(imageConfigSpec.Ports))

workspaces/controller/internal/controller/workspace_controller_test.go

@@ -18,6 +18,9 @@ package controller
 
 import (
 	"fmt"
+	"github.com/kubeflow/notebooks/workspaces/controller/internal/istio"
+	"github.com/onsi/gomega/format"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"time"
 
 	"k8s.io/utils/ptr"
@@ -36,6 +39,9 @@ import (
 
 var _ = Describe("Workspace Controller", func() {
 
+	// https://onsi.github.io/gomega/#adjusting-output
+	format.MaxLength = 10000
+
 	// Define utility constants for object names and testing timeouts/durations and intervals.
 	const (
 		namespaceName = "default"
@@ -189,6 +195,17 @@ var _ = Describe("Workspace Controller", func() {
 
 			// TODO: use this to get the Service
 			//service := serviceList.Items[0]
+			By("creating a VirtualService")
+			virtualServiceList := &unstructured.UnstructuredList{}
+			virtualServiceList.SetAPIVersion(istio.ApiVersionIstio)
+			virtualServiceList.SetKind(istio.VirtualServiceKind)
+			Eventually(func() ([]unstructured.Unstructured, error) {
+				err := k8sClient.List(ctx, virtualServiceList, client.InNamespace(namespaceName))
+				if err != nil {
+					return nil, err
+				}
+				return virtualServiceList.Items, nil
+			}, timeout, interval).Should(HaveLen(1))
 
 			//
 			// TODO: populate these tests

workspaces/controller/internal/helper/helper.go

@@ -1,6 +1,8 @@
 package helper
 
 import (
+	"fmt"
+	"os"
 	"reflect"
 
 	kubefloworgv1beta1 "github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1"
@@ -10,6 +12,11 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
+const (
+	GenerateNameSuffixLength = 6
+	MaxServiceNameLength     = 63
+)
+
 // CopyStatefulSetFields updates a target StatefulSet with the fields from a desired StatefulSet, returning true if an update is required.
 func CopyStatefulSetFields(desired *appsv1.StatefulSet, target *appsv1.StatefulSet) bool {
 	requireUpdate := false
@@ -166,3 +173,56 @@ func NormalizePodConfigSpec(spec kubefloworgv1beta1.PodConfigSpec) error {
 
 	return nil
 }
+
+// GenerateNamePrefix generates a name prefix for a Workspace
+// the format is "ws-{WORKSPACE_NAME}-" the workspace name is truncated to fit within the max length
+func GenerateNamePrefix(workspaceName string, maxLength int) string {
+	namePrefix := fmt.Sprintf("ws-%s", workspaceName)
+	maxLength = maxLength - GenerateNameSuffixLength // subtract 6 for the `metadata.generateName` suffix
+	maxLength = maxLength - 1                        // subtract 1 for the trailing "-"
+	if len(namePrefix) > maxLength {
+		namePrefix = namePrefix[:min(len(namePrefix), maxLength)]
+	}
+	if namePrefix[len(namePrefix)-1] != '-' {
+		namePrefix = namePrefix + "-"
+	}
+	return namePrefix
+}
+
+// RemoveTrailingDash removes trailing dash from string.
+func RemoveTrailingDash(s string) string {
+	if len(s) > 0 && s[len(s)-1] == '-' {
+		return s[:len(s)-1]
+	}
+	return s
+}
+
+// GetEnvOrDefault is a utility function for getting environment variable value, otherwise uses the defaultValue.
+func GetEnvOrDefault(name, defaultValue string) string {
+	if lookupEnv, exists := os.LookupEnv(name); exists {
+		return lookupEnv
+	} else {
+		return defaultValue
+	}
+}
+
+// GenerateContainerPortsIdMap generates a map[string]kubefloworgv1beta1.ImagePort having as key the kubefloworgv1beta1.ImagePort.Id.
+func GenerateContainerPortsIdMap(imageConfig *kubefloworgv1beta1.ImageConfigValue) (map[string]kubefloworgv1beta1.ImagePort, error) {
+	containerPortsIdMap := make(map[string]kubefloworgv1beta1.ImagePort)
+
+	containerPorts := make([]corev1.ContainerPort, len(imageConfig.Spec.Ports))
+	seenPorts := make(map[int32]bool)
+	for i, port := range imageConfig.Spec.Ports {
+		if seenPorts[port.Port] {
+			return nil, fmt.Errorf("duplicate port number %d in imageConfig", port.Port)
+		}
+		containerPorts[i] = corev1.ContainerPort{
+			Name:          fmt.Sprintf("http-%d", port.Port),
+			ContainerPort: port.Port,
+			Protocol:      corev1.ProtocolTCP,
+		}
+		seenPorts[port.Port] = true
+		containerPortsIdMap[port.Id] = port
+	}
+	return containerPortsIdMap, nil
+}

workspaces/controller/internal/helper/template.go

@@ -3,15 +3,16 @@ package helper
 import (
 	"bytes"
 	"fmt"
+	kubefloworgv1beta1 "github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1"
 	"text/template"
 )
 
-// RenderExtraEnvValueTemplate renders a single WorkspaceKind `spec.podTemplate.extraEnv[].value` string template
-func RenderExtraEnvValueTemplate(rawValue string, httpPathPrefixFunc func(string) string) (string, error) {
+// RenderWithHttpPathPrefixFunc renders a string template using templateFunc (Go template function).
+func RenderWithHttpPathPrefixFunc(rawValue string, templateFunc func(portId string) string) (string, error) {
 
 	// Parse the raw value as a template
 	tmpl, err := template.New("value").
-		Funcs(template.FuncMap{"httpPathPrefix": httpPathPrefixFunc}).
+		Funcs(template.FuncMap{"httpPathPrefix": templateFunc}).
 		Parse(rawValue)
 	if err != nil {
 		err = fmt.Errorf("failed to parse template %q: %w", rawValue, err)
@@ -28,3 +29,36 @@ func RenderExtraEnvValueTemplate(rawValue string, httpPathPrefixFunc func(string
 
 	return buf.String(), nil
 }
+
+// RenderHeadersWithHttpPathPrefix renders a map[string]string values using httpPathPrefixFunc Go template function.
+func RenderHeadersWithHttpPathPrefix(requestHeaders map[string]string, templateFunc func(v string) string) map[string]string {
+
+	if len(requestHeaders) == 0 {
+		return make(map[string]string, 0)
+	}
+
+	headers := make(map[string]string, len(requestHeaders))
+	for key, value := range requestHeaders {
+		if value != "" {
+			out, err := RenderWithHttpPathPrefixFunc(value, templateFunc)
+			if err != nil {
+				return make(map[string]string)
+			}
+			value = out
+		}
+		headers[key] = value
+	}
+	return headers
+}
+
+// GenerateHttpPathPrefixFunc generates the httpPathPrefix Go template function.
+func GenerateHttpPathPrefixFunc(workspace *kubefloworgv1beta1.Workspace, containerPortsIdMap map[string]kubefloworgv1beta1.ImagePort) func(portId string) string {
+	return func(portId string) string {
+		port, ok := containerPortsIdMap[portId]
+		if ok {
+			return fmt.Sprintf("/workspace/%s/%s/%s/", workspace.Namespace, workspace.Name, port.Id)
+		} else {
+			return ""
+		}
+	}
+}

workspaces/controller/internal/helper/template_test.go

@@ -0,0 +1,35 @@
+package helper
+
+import (
+	kubefloworgv1beta1 "github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1"
+	"github.com/onsi/ginkgo/v2"
+	"github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+var _ = ginkgo.Describe("helper", func() {
+
+	ginkgo.It("should render request headers correctly", func() {
+
+		containerPortsIdMap := make(map[string]kubefloworgv1beta1.ImagePort)
+		containerPortsIdMap["rstudio"] = kubefloworgv1beta1.ImagePort{
+			Port: 8080,
+			Id:   "rstudio",
+		}
+
+		headers := map[string]string{"X-RStudio-Root-Path": `{{ httpPathPrefix "rstudio" }}`}
+
+		ws := &kubefloworgv1beta1.Workspace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "simple",
+				Namespace: "default",
+			},
+		}
+
+		function := GenerateHttpPathPrefixFunc(ws, containerPortsIdMap)
+
+		out := RenderHeadersWithHttpPathPrefix(headers, function)
+
+		gomega.Expect(out["X-RStudio-Root-Path"]).To(gomega.Equal("/workspace/default/simple/rstudio/"))
+	})
+})