Skip to content

Commit

Permalink
Add call to reconcile VirtualService
Browse files Browse the repository at this point in the history
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
  • Loading branch information
mcruzdev committed Sep 15, 2024
1 parent bc4e445 commit 6c0bb84
Show file tree
Hide file tree
Showing 11 changed files with 16,907 additions and 41 deletions.
2 changes: 1 addition & 1 deletion workspaces/controller/config/manager/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ resources:
images:
- name: controller
newName: ghcr.io/kubeflow/notebooks/workspace-controller
newTag: latest
newTag: latest
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion workspaces/controller/internal/controller/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
53 changes: 19 additions & 34 deletions workspaces/controller/internal/controller/workspace_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package controller
import (
"context"
"fmt"
"github.com/kubeflow/notebooks/workspaces/controller/internal/istio"
"reflect"
"strings"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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))
Expand All @@ -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)
}
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand Down
60 changes: 60 additions & 0 deletions workspaces/controller/internal/helper/helper.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package helper

import (
"fmt"
"os"
"reflect"

kubefloworgv1beta1 "github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1"
Expand All @@ -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
Expand Down Expand Up @@ -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
}
40 changes: 37 additions & 3 deletions workspaces/controller/internal/helper/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 ""
}
}
}
35 changes: 35 additions & 0 deletions workspaces/controller/internal/helper/template_test.go
Original file line number Diff line number Diff line change
@@ -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/"))
})
})
Loading

0 comments on commit 6c0bb84

Please sign in to comment.