diff --git a/workspaces/backend/internal/models/workspaces/funcs.go b/workspaces/backend/internal/models/workspaces/funcs.go index 7329057b3..c4cf3caa6 100644 --- a/workspaces/backend/internal/models/workspaces/funcs.go +++ b/workspaces/backend/internal/models/workspaces/funcs.go @@ -359,7 +359,7 @@ func buildServices(ws *kubefloworgv1beta1.Workspace, wskPodTemplatePorts map[kub case kubefloworgv1beta1.ImagePortProtocolHTTP: services[i].HttpService = &HttpService{ DisplayName: ptr.Deref(port.DisplayName, wskPort.DefaultDisplayName), - HttpPath: fmt.Sprintf("/workspace/%s/%s/%s/", ws.Namespace, ws.Name, port.Id), + HttpPath: fmt.Sprintf("/workspace/connect/%s/%s/%s/", ws.Namespace, ws.Name, port.Id), } } } diff --git a/workspaces/controller/Makefile b/workspaces/controller/Makefile index 47985aedc..10c2478c3 100644 --- a/workspaces/controller/Makefile +++ b/workspaces/controller/Makefile @@ -138,12 +138,12 @@ uninstall: manifests kustomize ## Uninstall CRDs from the K8s cluster specified .PHONY: deploy deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in ~/.kube/config. - cd config/manager && $(KUSTOMIZE) edit set image controller=${IMG} - $(KUSTOMIZE) build config/default | $(KUBECTL) apply -f - + cd config/manager && $(KUSTOMIZE) edit set image workspaces-controller=${IMG} + $(KUSTOMIZE) build config/overlays/istio | $(KUBECTL) apply -f - .PHONY: undeploy undeploy: kustomize ## Undeploy controller from the K8s cluster specified in ~/.kube/config. Call with ignore-not-found=true to ignore resource not found errors during deletion. - $(KUSTOMIZE) build config/default | $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f - + $(KUSTOMIZE) build config/overlays/istio | $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f - ##@ Dependencies diff --git a/workspaces/controller/api/v1beta1/workspacekind_types.go b/workspaces/controller/api/v1beta1/workspacekind_types.go index 88fccee93..9bdd44da4 100644 --- a/workspaces/controller/api/v1beta1/workspacekind_types.go +++ b/workspaces/controller/api/v1beta1/workspacekind_types.go @@ -279,7 +279,7 @@ type WorkspaceKindVolumeMounts struct { type HTTPProxy struct { // if the path prefix is stripped from incoming HTTP requests - // - if true, the '/workspace/{profile_name}/{workspace_name}/' path prefix + // - if true, the '/workspace/connect/{profile_name}/{workspace_name}/' path prefix // is stripped from incoming requests, the application sees the request // as if it was made to '/...' // - this only works if the application serves RELATIVE URLs for its assets @@ -291,7 +291,7 @@ type HTTPProxy struct { // - sets the `spec.http[].headers.request` of the Istio VirtualService // https://istio.io/latest/docs/reference/config/networking/virtual-service/#Headers-HeaderOperations // - the following string templates are available: - // - `.PathPrefix`: the path prefix of the Workspace (e.g. '/workspace/{profile_name}/{workspace_name}/') + // - `.PathPrefix`: the path prefix of the Workspace (e.g. '/workspace/connect/{profile_name}/{workspace_name}/') // +kubebuilder:validation:Optional RequestHeaders *IstioHeaderOperations `json:"requestHeaders,omitempty"` } diff --git a/workspaces/controller/cmd/main.go b/workspaces/controller/cmd/main.go index 296580456..c65346f4b 100644 --- a/workspaces/controller/cmd/main.go +++ b/workspaces/controller/cmd/main.go @@ -20,11 +20,13 @@ import ( "crypto/tls" "flag" "os" + "strconv" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. _ "k8s.io/client-go/plugin/pkg/client/auth" + istiov1 "istio.io/client-go/pkg/apis/networking/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -36,6 +38,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" kubefloworgv1beta1 "github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1" + "github.com/kubeflow/notebooks/workspaces/controller/internal/config" controllerInternal "github.com/kubeflow/notebooks/workspaces/controller/internal/controller" "github.com/kubeflow/notebooks/workspaces/controller/internal/helper" webhookInternal "github.com/kubeflow/notebooks/workspaces/controller/internal/webhook" @@ -50,6 +53,8 @@ var ( func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(istiov1.AddToScheme(scheme)) + utilruntime.Must(kubefloworgv1beta1.AddToScheme(scheme)) // +kubebuilder:scaffold:scheme } @@ -60,6 +65,10 @@ func main() { var probeAddr string var secureMetrics bool var enableHTTP2 bool + + // Define command line flags + cfg := &config.EnvConfig{} + flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, @@ -69,6 +78,15 @@ func main() { "If set the metrics endpoint is served securely") flag.BoolVar(&enableHTTP2, "enable-http2", false, "If set, HTTP/2 will be enabled for the metrics and webhook servers") + flag.StringVar(&cfg.IstioGateway, "istio-gateway", getEnvAsStr("ISTIO_GATEWAY", ""), + "The name of the Istio gateway to use") + flag.StringVar(&cfg.IstioHosts, "istio-hosts", getEnvAsStr("ISTIO_HOSTS", ""), + "The hosts to use for the Istio VirtualService") + flag.StringVar(&cfg.ClusterDomain, "cluster-domain", getEnvAsStr("CLUSTER_DOMAIN", ""), + "The domain to use for the Istio VirtualService") + flag.BoolVar(&cfg.UseIstio, "use-istio", getEnvAsBool("USE_ISTIO", false), + "If set, Istio will be used") + opts := zap.Options{ Development: true, } @@ -129,7 +147,7 @@ func main() { // setup field indexers on the manager cache. we use these indexes to efficiently // query the cache for things like which Workspaces are using a particular WorkspaceKind - if err := helper.SetupManagerFieldIndexers(mgr); err != nil { + if err := helper.SetupManagerFieldIndexers(mgr, cfg); err != nil { setupLog.Error(err, "unable to setup field indexers") os.Exit(1) } @@ -137,6 +155,7 @@ func main() { if err = (&controllerInternal.WorkspaceReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), + Config: cfg, }).SetupWithManager(mgr, controller.Options{ RateLimiter: helper.BuildRateLimiter(), }); err != nil { @@ -188,3 +207,19 @@ func main() { os.Exit(1) } } + +func getEnvAsStr(name string, defaultVal string) string { + if value, exists := os.LookupEnv(name); exists { + return value + } + return defaultVal +} + +func getEnvAsBool(name string, defaultVal bool) bool { + if value, exists := os.LookupEnv(name); exists { + if boolValue, err := strconv.ParseBool(value); err == nil { + return boolValue + } + } + return defaultVal +} diff --git a/workspaces/controller/config/components/common/kustomization.yaml b/workspaces/controller/config/components/common/kustomization.yaml new file mode 100644 index 000000000..c49e81cab --- /dev/null +++ b/workspaces/controller/config/components/common/kustomization.yaml @@ -0,0 +1,9 @@ +apiVersion: kustomize.config.k8s.io/v1alpha1 +kind: Component + +labels: +- includeSelectors: true + pairs: + app.kubernetes.io/managed-by: kustomize + app.kubernetes.io/name: workspaces-controller + app.kubernetes.io/part-of: kubeflow-workspaces \ No newline at end of file diff --git a/workspaces/controller/config/components/istio/kustomization.yaml b/workspaces/controller/config/components/istio/kustomization.yaml new file mode 100644 index 000000000..0a3433b1b --- /dev/null +++ b/workspaces/controller/config/components/istio/kustomization.yaml @@ -0,0 +1,16 @@ +apiVersion: kustomize.config.k8s.io/v1alpha1 +kind: Component + +configMapGenerator: +- envs: + - params.env + name: kubeflow-workspaces-istio-config +generatorOptions: + disableNameSuffixHash: true + +labels: +- pairs: + app.kubernetes.io/component: controller-manager + +patches: +- path: manager_istio_patch.yaml diff --git a/workspaces/controller/config/components/istio/manager_istio_patch.yaml b/workspaces/controller/config/components/istio/manager_istio_patch.yaml new file mode 100644 index 000000000..38fe201a8 --- /dev/null +++ b/workspaces/controller/config/components/istio/manager_istio_patch.yaml @@ -0,0 +1,30 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: workspaces-controller +spec: + template: + spec: + containers: + - name: manager + env: + - name: USE_ISTIO + valueFrom: + configMapKeyRef: + name: kubeflow-workspaces-istio-config + key: USE_ISTIO + - name: ISTIO_GATEWAY + valueFrom: + configMapKeyRef: + name: kubeflow-workspaces-istio-config + key: ISTIO_GATEWAY + - name: ISTIO_HOST + valueFrom: + configMapKeyRef: + name: kubeflow-workspaces-istio-config + key: ISTIO_HOST + - name: CLUSTER_DOMAIN + valueFrom: + configMapKeyRef: + name: kubeflow-workspaces-istio-config + key: CLUSTER_DOMAIN \ No newline at end of file diff --git a/workspaces/controller/config/components/istio/params.env b/workspaces/controller/config/components/istio/params.env new file mode 100644 index 000000000..cf6f8d1e3 --- /dev/null +++ b/workspaces/controller/config/components/istio/params.env @@ -0,0 +1,4 @@ +USE_ISTIO=true +ISTIO_GATEWAY=kubeflow/kubeflow-gateway +ISTIO_HOST=* +CLUSTER_DOMAIN=cluster.local diff --git a/workspaces/controller/config/crd/bases/kubeflow.org_workspacekinds.yaml b/workspaces/controller/config/crd/bases/kubeflow.org_workspacekinds.yaml index 80c174a2e..1f0848945 100644 --- a/workspaces/controller/config/crd/bases/kubeflow.org_workspacekinds.yaml +++ b/workspaces/controller/config/crd/bases/kubeflow.org_workspacekinds.yaml @@ -3699,7 +3699,7 @@ spec: default: false description: |- if the path prefix is stripped from incoming HTTP requests - - if true, the '/workspace/{profile_name}/{workspace_name}/' path prefix + - if true, the '/workspace/connect/{profile_name}/{workspace_name}/' path prefix is stripped from incoming requests, the application sees the request as if it was made to '/...' - this only works if the application serves RELATIVE URLs for its assets @@ -3710,7 +3710,7 @@ spec: - sets the `spec.http[].headers.request` of the Istio VirtualService https://istio.io/latest/docs/reference/config/networking/virtual-service/#Headers-HeaderOperations - the following string templates are available: - - `.PathPrefix`: the path prefix of the Workspace (e.g. '/workspace/{profile_name}/{workspace_name}/') + - `.PathPrefix`: the path prefix of the Workspace (e.g. '/workspace/connect/{profile_name}/{workspace_name}/') properties: add: additionalProperties: diff --git a/workspaces/controller/config/overlays/istio/kustomization.yaml b/workspaces/controller/config/overlays/istio/kustomization.yaml new file mode 100644 index 000000000..e751551b6 --- /dev/null +++ b/workspaces/controller/config/overlays/istio/kustomization.yaml @@ -0,0 +1,11 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +namespace: kubeflow-workspaces + +resources: +- ../../default + +components: +- ../../components/istio +- ../../components/common diff --git a/workspaces/controller/config/samples/jupyterlab_v1beta1_workspacekind.yaml b/workspaces/controller/config/samples/jupyterlab_v1beta1_workspacekind.yaml index 6dfa28649..55fa5701b 100644 --- a/workspaces/controller/config/samples/jupyterlab_v1beta1_workspacekind.yaml +++ b/workspaces/controller/config/samples/jupyterlab_v1beta1_workspacekind.yaml @@ -153,7 +153,7 @@ spec: httpProxy: ## if the path prefix is stripped from incoming HTTP requests - ## - if true, the '/workspace/{profile_name}/{workspace_name}/' path prefix + ## - if true, the '/workspace/connect/{profile_name}/{workspace_name}/' path prefix ## is stripped from incoming requests, the application sees the request ## as if it was made to '/...' ## - this only works if the application serves RELATIVE URLs for its assets @@ -164,7 +164,7 @@ spec: ## - sets the `spec.http[].headers.request` of the Istio VirtualService ## https://istio.io/latest/docs/reference/config/networking/virtual-service/#Headers-HeaderOperations ## - the following string templates are available: - ## - `.PathPrefix`: the path prefix of the Workspace (e.g. '/workspace/{profile_name}/{workspace_name}/') + ## - `.PathPrefix`: the path prefix of the Workspace (e.g. '/workspace/connect/{profile_name}/{workspace_name}/') ## requestHeaders: {} #set: { "X-RStudio-Root-Path": "{{ .PathPrefix }}" } # for RStudio diff --git a/workspaces/controller/go.mod b/workspaces/controller/go.mod index b5c848044..f4bd2b7d9 100644 --- a/workspaces/controller/go.mod +++ b/workspaces/controller/go.mod @@ -7,6 +7,8 @@ require ( github.com/onsi/ginkgo/v2 v2.19.0 github.com/onsi/gomega v1.33.1 golang.org/x/time v0.3.0 + istio.io/api v1.22.8 + istio.io/client-go v1.22.8 k8s.io/api v0.31.0 k8s.io/apimachinery v0.31.0 k8s.io/client-go v0.31.0 @@ -59,6 +61,7 @@ require ( golang.org/x/text v0.16.0 // indirect golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157 // indirect google.golang.org/protobuf v1.34.2 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect diff --git a/workspaces/controller/go.sum b/workspaces/controller/go.sum index 8496f957c..03e28f87b 100644 --- a/workspaces/controller/go.sum +++ b/workspaces/controller/go.sum @@ -9,8 +9,8 @@ github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g= github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= -github.com/evanphx/json-patch v0.5.2 h1:xVCHIVMUu1wtM/VkR9jVZ45N3FhZfYMMYGorLCR8P3k= -github.com/evanphx/json-patch v0.5.2/go.mod h1:ZWS5hhDbVDyob71nXKNL0+PWn6ToqBHMikGIFbs31qQ= +github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCvpL6mnFh5mB2/l16U= +github.com/evanphx/json-patch v5.6.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg= github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ= github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA= @@ -155,6 +155,8 @@ golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gomodules.xyz/jsonpatch/v2 v2.4.0 h1:Ci3iUJyx9UeRx7CeFN8ARgGbkESwJK+KB9lLcWxY/Zw= gomodules.xyz/jsonpatch/v2 v2.4.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY= +google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157 h1:7whR9kGa5LUwFtpLm2ArCEejtnxlGeLbAyjFY8sGNFw= +google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157/go.mod h1:99sLkeliLXfdj2J75X3Ho+rrVCaJze0uwN7zDDkjPVU= google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg= google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= @@ -170,6 +172,10 @@ gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +istio.io/api v1.22.8 h1:mhkaeFJ13WZ2d6pvL9+exNeQ9UB6HX7e6m+XwO9XoYY= +istio.io/api v1.22.8/go.mod h1:S3l8LWqNYS9yT+d4bH+jqzH2lMencPkW7SKM1Cu9EyM= +istio.io/client-go v1.22.8 h1:wojmt220jSbfhpRDsPiflj2nSFTBuYtZNiW9hqKeaWE= +istio.io/client-go v1.22.8/go.mod h1:noO8SoyMxLwni3w+yGK67aydi2klExjmiqnXyeRS/00= k8s.io/api v0.31.0 h1:b9LiSjR2ym/SzTOlfMHm1tr7/21aD7fSkqgD/CVJBCo= k8s.io/api v0.31.0/go.mod h1:0YiFF+JfFxMM6+1hQei8FY8M7s1Mth+z/q7eF1aJkTE= k8s.io/apiextensions-apiserver v0.31.0 h1:fZgCVhGwsclj3qCw1buVXCV6khjRzKC5eCFt24kyLSk= diff --git a/workspaces/controller/internal/config/environment.go b/workspaces/controller/internal/config/environment.go new file mode 100644 index 000000000..5250697f1 --- /dev/null +++ b/workspaces/controller/internal/config/environment.go @@ -0,0 +1,24 @@ +/* +Copyright 2024. + +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. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +type EnvConfig struct { + IstioGateway string + IstioHosts string + ClusterDomain string + UseIstio bool +} diff --git a/workspaces/controller/internal/controller/suite_test.go b/workspaces/controller/internal/controller/suite_test.go index c606cf8b7..60a6d3ba5 100644 --- a/workspaces/controller/internal/controller/suite_test.go +++ b/workspaces/controller/internal/controller/suite_test.go @@ -42,6 +42,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" kubefloworgv1beta1 "github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1" + + istiov1 "istio.io/client-go/pkg/apis/networking/v1" + + "github.com/kubeflow/notebooks/workspaces/controller/internal/config" "github.com/kubeflow/notebooks/workspaces/controller/internal/helper" // +kubebuilder:scaffold:imports ) @@ -88,6 +92,8 @@ var _ = BeforeSuite(func() { By("setting up the scheme") err = kubefloworgv1beta1.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) + err = istiov1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) // +kubebuilder:scaffold:scheme @@ -106,13 +112,16 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) By("setting up the field indexers for the controller manager") - err = helper.SetupManagerFieldIndexers(k8sManager) + // Use test config with UseIstio disabled to avoid VirtualService CRD requirements in tests + testCfg := &config.EnvConfig{UseIstio: false} + err = helper.SetupManagerFieldIndexers(k8sManager, testCfg) Expect(err).NotTo(HaveOccurred()) By("setting up the Workspace controller") err = (&WorkspaceReconciler{ Client: k8sManager.GetClient(), Scheme: k8sManager.GetScheme(), + Config: testCfg, }).SetupWithManager(k8sManager, controller.Options{ RateLimiter: helper.BuildRateLimiter(), }) diff --git a/workspaces/controller/internal/controller/workspace_controller.go b/workspaces/controller/internal/controller/workspace_controller.go index b88e27290..e2b6c1f8b 100644 --- a/workspaces/controller/internal/controller/workspace_controller.go +++ b/workspaces/controller/internal/controller/workspace_controller.go @@ -22,6 +22,8 @@ import ( "strings" "github.com/go-logr/logr" + networkingv1 "istio.io/api/networking/v1" + istiov1 "istio.io/client-go/pkg/apis/networking/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" @@ -44,6 +46,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" kubefloworgv1beta1 "github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1" + "github.com/kubeflow/notebooks/workspaces/controller/internal/config" "github.com/kubeflow/notebooks/workspaces/controller/internal/helper" ) @@ -56,9 +59,13 @@ const ( workspacePodTemplateContainerName = "main" // lengths for resource names - generateNameSuffixLength = 6 - maxServiceNameLength = 63 - maxStatefulSetNameLength = 52 // https://github.com/kubernetes/kubernetes/issues/64023 + generateNameSuffixLength = 6 + maxServiceNameLength = 63 + maxVirtualServiceNameLength = 63 + maxStatefulSetNameLength = 52 // https://github.com/kubernetes/kubernetes/issues/64023 + + // workspace connection path template + workspaceConnectPathTemplate = "/workspace/connect/%s/%s/%s/" // state message formats for Workspace status stateMsgErrorUnknownWorkspaceKind = "Workspace references unknown WorkspaceKind: %s" @@ -68,6 +75,7 @@ const ( stateMsgErrorGenFailureService = "Workspace failed to generate Service with error: %s" stateMsgErrorMultipleStatefulSets = "Workspace owns multiple StatefulSets: %s" stateMsgErrorMultipleServices = "Workspace owns multiple Services: %s" + stateMsgErrorMultipleVirtualServices = "Workspace owns multiple VirtualServices: %s" stateMsgErrorStatefulSetWarningEvent = "Workspace StatefulSet has warning event: %s" stateMsgErrorPodUnschedulable = "Workspace Pod is unschedulable: %s" stateMsgErrorPodSchedulingGate = "Workspace Pod is waiting for scheduling gate: %s" @@ -85,6 +93,7 @@ const ( type WorkspaceReconciler struct { client.Client Scheme *runtime.Scheme + Config *config.EnvConfig } // +kubebuilder:rbac:groups=kubeflow.org,resources=workspaces,verbs=create;delete;get;list;patch;update;watch @@ -354,10 +363,71 @@ 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 - // + if r.Config.UseIstio { + log.V(2).Info("reconciling VirtualService for Workspace") + // generateVirtualService + virtualsvc, err := generateVirtualService(workspace, workspaceKind, serviceName, currentImageConfig.Spec, r.Config) + if err != nil { + log.V(0).Info("failed to generate VirtualService for Workspace", "error", err.Error()) + return r.updateWorkspaceState(ctx, log, workspace, + kubefloworgv1beta1.WorkspaceStateError, + fmt.Sprintf("failed to generate VirtualService for Workspace: %s", err.Error()), + ) + } + if err := ctrl.SetControllerReference(workspace, virtualsvc, r.Scheme); err != nil { + log.Error(err, "unable to set controller reference on VirtualService") + return ctrl.Result{}, err + } + + // fetch VirtualServices + // NOTE: we filter by VirtualServices that are owned by the Workspace, not by name + // this allows us to generate a random name for the VirtualService with `metadata.generateName` + var VirtualServiceName string + ownedVirtualServices := &istiov1.VirtualServiceList{} + listOpts = &client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector(helper.IndexWorkspaceOwnerField, workspace.Name), + Namespace: req.Namespace, + } + if err := r.List(ctx, ownedVirtualServices, listOpts); err != nil { + log.Error(err, "unable to list VirtualServices") + return ctrl.Result{}, err + } + + switch numVirtualServices := len(ownedVirtualServices.Items); { + case numVirtualServices > 1: + virtualServiceList := make([]string, len(ownedVirtualServices.Items)) + for i, vs := range ownedVirtualServices.Items { + virtualServiceList[i] = vs.Name + } + virtualServiceListString := strings.Join(virtualServiceList, ", ") + log.Error(nil, "Workspace owns multiple VirtualServices", "virtualServices", virtualServiceListString) + return r.updateWorkspaceState(ctx, log, workspace, + kubefloworgv1beta1.WorkspaceStateError, + fmt.Sprintf(stateMsgErrorMultipleVirtualServices, virtualServiceListString), + ) + case numVirtualServices == 0: + if err := r.Create(ctx, virtualsvc); err != nil { + log.Error(err, "unable to create VirtualService") + return ctrl.Result{}, err + } + VirtualServiceName = virtualsvc.ObjectMeta.Name + log.V(2).Info("VirtualService created", "virtualService", VirtualServiceName) + default: + foundVirtualService := ownedVirtualServices.Items[0] + VirtualServiceName = foundVirtualService.ObjectMeta.Name + if helper.CopyVirtualServiceFields(virtualsvc, foundVirtualService) { + if err := r.Update(ctx, foundVirtualService); err != nil { + if apierrors.IsConflict(err) { + log.V(2).Info("update conflict while updating VirtualService, will requeue") + return ctrl.Result{Requeue: true}, nil + } + log.Error(err, "unable to update VirtualService") + return ctrl.Result{}, err + } + log.V(2).Info("VirtualService updated", "virtualService", VirtualServiceName) + } + } + } // fetch Pod // NOTE: the first StatefulSet Pod is always called "{statefulSetName}-0" @@ -418,11 +488,19 @@ func (r *WorkspaceReconciler) SetupWithManager(mgr ctrl.Manager, opts controller return labelExists }) - return ctrl.NewControllerManagedBy(mgr). + // Build the controller with core resources + controllerBuilder := ctrl.NewControllerManagedBy(mgr). WithOptions(opts). For(&kubefloworgv1beta1.Workspace{}). Owns(&appsv1.StatefulSet{}). - Owns(&corev1.Service{}). + Owns(&corev1.Service{}) + + if r.Config.UseIstio { + + controllerBuilder = controllerBuilder.Owns(&istiov1.VirtualService{}) + } + + return controllerBuilder. Watches( &kubefloworgv1beta1.WorkspaceKind{}, handler.EnqueueRequestsFromMapFunc(r.mapWorkspaceKindToRequest), @@ -570,6 +648,11 @@ func getPodConfig(workspace *kubefloworgv1beta1.Workspace, workspaceKind *kubefl } } +// getWorkspaceConnectPath generates the HTTP path for connecting to a workspace port +func getWorkspaceConnectPath(namespace, workspaceName string, portId kubefloworgv1beta1.PortId) string { + return fmt.Sprintf(workspaceConnectPathTemplate, namespace, workspaceName, portId) +} + // 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 { @@ -629,7 +712,7 @@ func generateStatefulSet(workspace *kubefloworgv1beta1.Workspace, workspaceKind httpPathPrefixFunc := func(portId kubefloworgv1beta1.PortId) string { port, ok := containerPortsIdMap[portId] if ok { - return fmt.Sprintf("/workspace/%s/%s/%s/", workspace.Namespace, workspace.Name, port.Id) + return getWorkspaceConnectPath(workspace.Namespace, workspace.Name, port.Id) } else { return "" } @@ -881,6 +964,96 @@ func generateService(workspace *kubefloworgv1beta1.Workspace, imageConfigSpec ku return service, nil } +// generateVirtualService generates a VirtualService for a Workspace +func generateVirtualService(workspace *kubefloworgv1beta1.Workspace, workspaceKind *kubefloworgv1beta1.WorkspaceKind, serviceName string, imageConfigSpec kubefloworgv1beta1.ImageConfigSpec, cfg *config.EnvConfig) (*istiov1.VirtualService, error) { + // NOTE: the name prefix is used to generate a unique name for the VirtualService + namePrefix := generateNamePrefix(workspace.Name, maxVirtualServiceNameLength) + + currentPodTemplatePortsMap := make(map[kubefloworgv1beta1.PortId]kubefloworgv1beta1.WorkspaceKindPort) + for _, port := range workspaceKind.Spec.PodTemplate.Ports { + currentPodTemplatePortsMap[port.Id] = port + } + + clusterDomain := "cluster.local" + if cfg.ClusterDomain != "" { + clusterDomain = cfg.ClusterDomain + } + serviceHost := fmt.Sprintf("%s.%s.svc.%s", serviceName, workspace.Namespace, clusterDomain) + + httpRoutes := []*networkingv1.HTTPRoute{} + for _, port := range imageConfigSpec.Ports { + if _, exists := currentPodTemplatePortsMap[port.Id]; exists { + podTemplatePort := currentPodTemplatePortsMap[port.Id] + matchUriPrefix := getWorkspaceConnectPath(workspace.Namespace, workspace.Name, port.Id) + + // Additional Cases would be added for SSH, etc. + switch podTemplatePort.Protocol { //nolint:gocritic + case kubefloworgv1beta1.ImagePortProtocolHTTP: + virtualServiceSpecHttp := &networkingv1.HTTPRoute{ + Headers: &networkingv1.Headers{ + Request: &networkingv1.Headers_HeaderOperations{}, + }, + Rewrite: &networkingv1.HTTPRewrite{}, + Match: []*networkingv1.HTTPMatchRequest{ + { + Uri: &networkingv1.StringMatch{ + MatchType: &networkingv1.StringMatch_Prefix{ + Prefix: matchUriPrefix, + }, + }, + }, + }, + Route: []*networkingv1.HTTPRouteDestination{ + { + Destination: &networkingv1.Destination{ + Host: serviceHost, + Port: &networkingv1.PortSelector{ + Number: uint32(port.Port), //nolint:gosec + }, + }, + }, + }, + } + if !ptr.Deref(podTemplatePort.HTTPProxy.RemovePathPrefix, false) { + virtualServiceSpecHttp.Rewrite.Uri = matchUriPrefix + } + if podTemplatePort.HTTPProxy.RequestHeaders != nil { + virtualServiceSpecHttp.Headers.Request.Set = podTemplatePort.HTTPProxy.RequestHeaders.Set + virtualServiceSpecHttp.Headers.Request.Add = podTemplatePort.HTTPProxy.RequestHeaders.Add + virtualServiceSpecHttp.Headers.Request.Remove = podTemplatePort.HTTPProxy.RequestHeaders.Remove + } + httpRoutes = append(httpRoutes, virtualServiceSpecHttp) + } + } + } + + if cfg.IstioGateway == "" { + return nil, fmt.Errorf("ISTIO_GATEWAY environment variable is not set") + } + + istioHosts := "*" + if cfg.IstioHosts != "" { + istioHosts = cfg.IstioHosts + } + + virtualService := &istiov1.VirtualService{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: namePrefix, + Namespace: workspace.Namespace, + Labels: map[string]string{ + workspaceNameLabel: workspace.Name, + }, + }, + Spec: networkingv1.VirtualService{ + Gateways: []string{cfg.IstioGateway}, + Hosts: []string{istioHosts}, + Http: httpRoutes, + }, + } + + return virtualService, nil +} + // generateWorkspaceStatus generates a WorkspaceStatus for a Workspace func (r *WorkspaceReconciler) generateWorkspaceStatus(ctx context.Context, log logr.Logger, workspace *kubefloworgv1beta1.Workspace, pod *corev1.Pod, statefulSet *appsv1.StatefulSet) (kubefloworgv1beta1.WorkspaceStatus, error) { // NOTE: some fields are populated before this function is called, diff --git a/workspaces/controller/internal/helper/helper.go b/workspaces/controller/internal/helper/helper.go index d4d6bdef2..3a5403200 100644 --- a/workspaces/controller/internal/helper/helper.go +++ b/workspaces/controller/internal/helper/helper.go @@ -19,6 +19,7 @@ package helper import ( "reflect" + istiov1 "istio.io/client-go/pkg/apis/networking/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -118,6 +119,44 @@ func CopyServiceFields(desired *corev1.Service, target *corev1.Service) bool { return requireUpdate } +// CopyVirtualServiceFields updates a target VirtualService with the fields from a desired VirtualService, returning true if an update is required. +func CopyVirtualServiceFields(desired *istiov1.VirtualService, target *istiov1.VirtualService) bool { + requireUpdate := false + + // Using the Spec definition https://pkg.go.dev/istio.io/api/networking/v1alpha3alpha3#VirtualService + if !reflect.DeepEqual(target.Spec.Gateways, desired.Spec.Gateways) { + target.Spec.Gateways = desired.Spec.Gateways + requireUpdate = true + } + + if !reflect.DeepEqual(target.Spec.Hosts, desired.Spec.Hosts) { + target.Spec.Hosts = desired.Spec.Hosts + requireUpdate = true + } + + if !reflect.DeepEqual(target.Spec.Http, desired.Spec.Http) { + target.Spec.Http = desired.Spec.Http + requireUpdate = true + } + + if !reflect.DeepEqual(target.Spec.Tls, desired.Spec.Tls) { + target.Spec.Tls = desired.Spec.Tls + requireUpdate = true + } + + if !reflect.DeepEqual(target.Spec.Tcp, desired.Spec.Tcp) { + target.Spec.Tcp = desired.Spec.Tcp + requireUpdate = true + } + + if !reflect.DeepEqual(target.Spec.ExportTo, desired.Spec.ExportTo) { + target.Spec.ExportTo = desired.Spec.ExportTo + requireUpdate = true + } + + return requireUpdate +} + // NormalizePodConfigSpec normalizes a PodConfigSpec so that it can be compared with reflect.DeepEqual func NormalizePodConfigSpec(spec kubefloworgv1beta1.PodConfigSpec) error { diff --git a/workspaces/controller/internal/helper/index.go b/workspaces/controller/internal/helper/index.go index 7c3d06163..26bd1060a 100644 --- a/workspaces/controller/internal/helper/index.go +++ b/workspaces/controller/internal/helper/index.go @@ -19,6 +19,7 @@ package helper import ( "context" + istiov1 "istio.io/client-go/pkg/apis/networking/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -26,16 +27,18 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" kubefloworgv1beta1 "github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1" + "github.com/kubeflow/notebooks/workspaces/controller/internal/config" ) const ( IndexEventInvolvedObjectUidField = ".involvedObject.uid" IndexWorkspaceOwnerField = ".metadata.controller" IndexWorkspaceKindField = ".spec.kind" + OwnerKindWorkspace = "Workspace" ) // SetupManagerFieldIndexers sets up field indexes on a controller-runtime manager -func SetupManagerFieldIndexers(mgr ctrl.Manager) error { +func SetupManagerFieldIndexers(mgr ctrl.Manager, cfg *config.EnvConfig) error { // Index Event by `involvedObject.uid` if err := mgr.GetFieldIndexer().IndexField(context.Background(), &corev1.Event{}, IndexEventInvolvedObjectUidField, func(rawObj client.Object) []string { @@ -55,7 +58,7 @@ func SetupManagerFieldIndexers(mgr ctrl.Manager) error { if owner == nil { return nil } - if owner.APIVersion != kubefloworgv1beta1.GroupVersion.String() || owner.Kind != "Workspace" { + if owner.APIVersion != kubefloworgv1beta1.GroupVersion.String() || owner.Kind != OwnerKindWorkspace { return nil } return []string{owner.Name} @@ -70,7 +73,7 @@ func SetupManagerFieldIndexers(mgr ctrl.Manager) error { if owner == nil { return nil } - if owner.APIVersion != kubefloworgv1beta1.GroupVersion.String() || owner.Kind != "Workspace" { + if owner.APIVersion != kubefloworgv1beta1.GroupVersion.String() || owner.Kind != OwnerKindWorkspace { return nil } return []string{owner.Name} @@ -78,6 +81,23 @@ func SetupManagerFieldIndexers(mgr ctrl.Manager) error { return err } + // Index VirtualService by its owner Workspace (only when Istio is enabled) + if cfg.UseIstio { + if err := mgr.GetFieldIndexer().IndexField(context.Background(), &istiov1.VirtualService{}, IndexWorkspaceOwnerField, func(rawObj client.Object) []string { + virtualService := rawObj.(*istiov1.VirtualService) + owner := metav1.GetControllerOf(virtualService) + if owner == nil { + return nil + } + if owner.APIVersion != kubefloworgv1beta1.GroupVersion.String() || owner.Kind != OwnerKindWorkspace { + return nil + } + return []string{owner.Name} + }); err != nil { + return err + } + } + // Index Workspace by WorkspaceKind if err := mgr.GetFieldIndexer().IndexField(context.Background(), &kubefloworgv1beta1.Workspace{}, IndexWorkspaceKindField, func(rawObj client.Object) []string { ws := rawObj.(*kubefloworgv1beta1.Workspace) diff --git a/workspaces/controller/internal/webhook/suite_test.go b/workspaces/controller/internal/webhook/suite_test.go index f187c350d..f5f74635a 100644 --- a/workspaces/controller/internal/webhook/suite_test.go +++ b/workspaces/controller/internal/webhook/suite_test.go @@ -45,7 +45,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" kubefloworgv1beta1 "github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1" + "github.com/kubeflow/notebooks/workspaces/controller/internal/config" "github.com/kubeflow/notebooks/workspaces/controller/internal/helper" + + istiov1 "istio.io/client-go/pkg/apis/networking/v1" // +kubebuilder:scaffold:imports ) @@ -95,6 +98,8 @@ var _ = BeforeSuite(func() { By("setting up the scheme") err = kubefloworgv1beta1.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) + err = istiov1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) // +kubebuilder:scaffold:scheme @@ -120,7 +125,9 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) By("setting up the field indexers for the controller manager") - err = helper.SetupManagerFieldIndexers(k8sManager) + // Use test config with UseIstio disabled to avoid VirtualService CRD requirements in tests + testCfg := &config.EnvConfig{UseIstio: false} + err = helper.SetupManagerFieldIndexers(k8sManager, testCfg) Expect(err).NotTo(HaveOccurred()) By("setting up the Workspace webhook") diff --git a/workspaces/controller/test/e2e/e2e_test.go b/workspaces/controller/test/e2e/e2e_test.go index 207621fd1..674267097 100644 --- a/workspaces/controller/test/e2e/e2e_test.go +++ b/workspaces/controller/test/e2e/e2e_test.go @@ -274,7 +274,7 @@ var _ = Describe("controller", Ordered, func() { Eventually(getServiceName, timeout, interval).Should(Succeed()) By("validating that the workspace service endpoint is reachable") - serviceEndpoint := fmt.Sprintf("http://%s:%d/workspace/%s/%s/%s/lab", + serviceEndpoint := fmt.Sprintf("http://%s:%d/workspace/connect/%s/%s/%s/lab", workspaceSvcName, workspacePortInt, workspaceNamespace, workspaceName, workspacePortId, ) curlService := func() error { diff --git a/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspace.mock.ts b/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspace.mock.ts index a5e33aec2..16ab31854 100644 --- a/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspace.mock.ts +++ b/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspace.mock.ts @@ -80,7 +80,7 @@ const generateMockWorkspace = ( { httpService: { displayName: 'Jupyter-lab', - httpPath: `/workspace/${namespace}/${name}/Jupyter-lab/`, + httpPath: `/workspace/connect/${namespace}/${name}/Jupyter-lab/`, }, }, ],