Skip to content
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.

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

feat: disables features not needed for Istio deployments #96

Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
apiVersion: v1
kind: Secret
metadata:
name: odh-dashboard-cert
namespace: odh-notebook-controller-system
type: kubernetes.io/tls
data:
tls.crt: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURIVENDQWdXZ0F3SUJBZ0lVZlpOenNGV29JRkxPYXB3aTNwMkx4VDNIWC9Bd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0hqRWNNQm9HQTFVRUF3d1RZMkV1WVhCd2N5MWpjbU11ZEdWemRHbHVaekFlRncweU16QXlNRE14TkRReQpNVGxhRncweU5EQXlNRE14TkRReU1UbGFNQnN4R1RBWEJnTlZCQU1NRUdGd2NITXRZM0pqTG5SbGMzUnBibWN3CmdnRWlNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0SUJEd0F3Z2dFS0FvSUJBUURCYmJ6Ritxa3lFZ0pUak1xSENHZTgKdnkzcTgwaTZPa1ByMlZmWm04THdxT3BJcDJxOWduZFF4MCtqaDI0UjhqLzgwQmVxMEJtc1FFNCsyTURXcmUzdQpkY2pBTVlSZWN2aE5DOVduQmlSZy9USVZYNEQya0hnV05XSS8zMDI4elVEZmlhMm1NbnFwNWFaVWUwcitndm5SClRRaFdUR0UrMStETTYrNUZySzB0NUhMeTZLLzJvRTBmb09SUG9wNzZyeEEzU3J6Rnd3N1gyZ2p0ckM0TjRJQlUKcE02OGtZUE9JZkk3bW9tV055bnRPeVQ5OGpUSThYY0VGeGp1L2NjcEczUi9aVmRiM1pXRUljeDJhd2ljZmFweAovMWNWRittNWkzSnRVRjBING5ZSG8yaXdFTlpmTTJZdHJNR2ZYaFBqdzBqWis0NFlNU0h4djdmdUttOGgybWxOCkFnTUJBQUdqVmpCVU1Ba0dBMVVkRXdRQ01BQXdDd1lEVlIwUEJBUURBZ1hnTUIwR0ExVWRKUVFXTUJRR0NDc0cKQVFVRkJ3TUNCZ2dyQmdFRkJRY0RBVEFiQmdOVkhSRUVGREFTZ2hCaGNIQnpMV055WXk1MFpYTjBhVzVuTUEwRwpDU3FHU0liM0RRRUJDd1VBQTRJQkFRRERKVE1vTVdTczZweHk0eHJTK2pFTnZrVkxDbHpabWZQbWx6MEZFY2JzCm9ZVW4vcGdQRmNQVms2QTdKZ204d2E3dG5Pc280U1JRWG5ReXFUYXNkZXJmZ3V2MWhLNkdpaGRzeTI1UncxYXIKaW5OSk93b1RXRG1aVVpTMmp3NElUUWlib2wzZUR4RzNNVXR1UnRzZG1WZFFkL3RreTdDZWxROFc1VDRqLytyMQpFeXRzWTJVYnIrcVdkcUxWZ0tjbXJnL0NiVzFhWVlOVlljVXlYa3d4ZGFRa09SaXBRd2NjaVJid3oyWERiN3VuCk56K29oSXVUMm01elFJLzkxRWlPVkp3MjFiR3oxWGg1Z2VKdnJsUG0ydjZPbkcvTFJ3bW9iWGl5Yk8wdzJTQkIKRUZnOURTYkF2NFdGcjR2ZUJ3akIwSXhlaW55Z2FLTnNSVm05U2w1eS9oaDkKLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo=
tls.key: LS0tLS1CRUdJTiBSU0EgUFJJVkFURSBLRVktLS0tLQpNSUlFcEFJQkFBS0NBUUVBd1cyOHhmcXBNaElDVTR6S2h3aG52TDh0NnZOSXVqcEQ2OWxYMlp2QzhLanFTS2RxCnZZSjNVTWRQbzRkdUVmSS8vTkFYcXRBWnJFQk9QdGpBMXEzdDduWEl3REdFWG5MNFRRdlZwd1lrWVAweUZWK0EKOXBCNEZqVmlQOTlOdk0xQTM0bXRwako2cWVXbVZIdEsvb0w1MFUwSVZreGhQdGZnek92dVJheXRMZVJ5OHVpdgo5cUJOSDZEa1Q2S2UrcThRTjBxOHhjTU8xOW9JN2F3dURlQ0FWS1RPdkpHRHppSHlPNXFKbGpjcDdUc2svZkkwCnlQRjNCQmNZN3YzSEtSdDBmMlZYVzkyVmhDSE1kbXNJbkgycWNmOVhGUmZwdVl0eWJWQmRCK0oyQjZOb3NCRFcKWHpObUxhekJuMTRUNDhOSTJmdU9HREVoOGIrMzdpcHZJZHBwVFFJREFRQUJBb0lCQVFDejRhekRaUGVDSS9OYgo1YnZXMWY4N0xZT3pVdXBZbmFUYXFiWWtIZEd0WXpqMXRoUHpCMmlVaTdaSk9zSW5HR1ZmWTlvT3RSYWE5UGFQClJaNFlSNG5VMEY2UU5ieUc1VjU2c0QzUjVVbGhsVFhGWUpxYk1nRXJqaHUva0poSHM0M1lGTDZUcDdBaFhmdFAKNTVUM21iQmZiOGNJRW1JQlFsdkIxc3N3cW9RbS93V3UyT1ZjZnBFRlRvN2N3eVptVVZHYnEwbmpiR01rbUE4bwpoMkVvUEtPV1FDckhQK0hKanBzSHhqbUdJZ3ZTdGVHOEpPcnBvVWVKQW0wdzVRYUVxeW9HSUJ0QzdmZnhvYlZSCkNOU1pETUNGMFh3RjFXWDJ2NFBqam83bTBIVU9iVHFXZlBzRXdRYTNSNWZON05HZ2FNMnRDTWpVUlZhYlBZZHkKcFZmcWdPc2hBb0dCQU9aNEEvdG1PdDFNbktLMG81dmh1TXpvMkhWN3djbmE3U0ljRURqODB4cTJzeTlkL3VXSgpqZTVlRTNHUDZDcXRQOXhBdFZDSUJwRXg4eDJjRFJ3dnRxcEFKNmgrSHRTcnUyOC9Oa2VndElxT3poL0dSZXlzClFRSHRZYmlTMEZSekNMTGpEcGpVb3BjY1llYzBpSDlycVA0Q1NDQnQ1WUtYcHJOYUYzNDNvaW4xQW9HQkFOYmIKUi9Ob2lncDNOZnpxV292QUdyT1IzZkxNd2RReklFbytTSHdSY1o1WDY2VXhqd3c2UUlTRlp5SEk2TXhMMXlpNQpUOTFNdHpIV2hIcnZJVWJySDBlRm1jN2sxSWRQY0ppd2xXU3dkWHYveS9kK21RUDRKT1BBMzB2cGRBUmtnVE9FCjQrUDlrQzlySjJGMkZaLzVGdy9aVzZ6QUZ5WFRDdW9TeGx5YjN2TDVBb0dCQUtLci81T0pHdTlzemZxQ0tpRXkKOTYrYWduNmFOYlIybEg1STlLSmt3ZFRQTkRhd3orUFFiWi9jUXprYTdES0RTdG41eW9EbklrdUZ5Q1lVS2FURgpnTmMycFVkbWpmaHFwc2ZsQkRrV2s1aGhKOWlCcUlWZktCdG1KRjJWTXZzSW54RTA5dTZrMTRaMWdCMGpsVnpxCjdzTXJkU0Yrc0VxM0kvRGdIRWo0bDd1cEFvR0FjaUdIaGZBcEs4Z0pnTEVJcWlYQXlWU1ozc2tQeVdYaktDMFAKbWdBMko1T3lsRXpRSFFHd2xmUzdSUUlSVDd5VnJZZEt1bFp2RmVWSytIYWdhYWlxTS9idkxpejJERzZSZERxUgpFU3gvTEFCRVc5TCsrMUhNWHNOc21rbUhuSEc3QkIvNlluaW1KOW8yMEJuSEFQUnpZTExvZE1xUlFVRnJFYzRwCldyWmQ1eGtDZ1lCd0ZETFVjZWNhRjRUcUZ5QnNhSE5KeisraUROUm1GeWMyNytGL3czQUE1ejRVMFIxVEdIM1EKOHNjeVhpNDdQUzF2Qy9uWklnOFpyVzhqL2lJVEIyR0JTQVNORU9hNzFxRWRkUUJyaFZ6cXE5aGVUMC8yNjVTMgpmaEhjQlhyQlk4OHBiRGUxSFI4d3k4OXBic2FtQXJNRTE5T0p3dUdVd05CcllHd2tZaTZmRXc9PQotLS0tLUVORCBSU0EgUFJJVkFURSBLRVktLS0tLQo=
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
kind: Route
apiVersion: route.openshift.io/v1
metadata:
name: opendatahub-odh-gateway
namespace: istio-system
labels:
maistra.io/gateway-name: odh-gateway
maistra.io/gateway-namespace: odh-notebook-controller-system
spec:
host: opendatahub.apps-crc.testing
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is replaced on the fly when executing make setup-service-mesh target.

to:
kind: Service
name: istio-ingressgateway
weight: 100
port:
targetPort: https
tls:
termination: passthrough
wildcardPolicy: None
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
apiVersion: networking.istio.io/v1beta1
kind: Gateway
metadata:
name: odh-gateway
namespace: odh-notebook-controller-system
spec:
selector:
istio: ingressgateway
servers:
- port:
number: 443
name: https
protocol: HTTPS
tls:
mode: SIMPLE
credentialName: odh-dashboard-cert
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the overlays here are used mainly for testing purposes I can adjust tests to be using http if desired.

hosts:
- "*"
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- ../openshift
- gateway.yaml
- gateway-route.yaml
- cert-secret.yaml
- smm.yaml
namespace: odh-notebook-controller-system

configMapGenerator:
- name: config
behavior: merge
literals:
- USE_ISTIO=true
- ISTIO_GATEWAY=odh-notebook-controller-system/odh-gateway

patchesJson6902:
- patch: |-
- op: replace
path: /metadata/namespace
value: istio-system
target:
group: route.openshift.io
kind: Route
version: v1
namespace: odh-notebook-controller-system
name: opendatahub-odh-gateway
- patch: |-
- op: replace
path: /metadata/namespace
value: istio-system
target:
kind: Secret
version: v1
namespace: odh-notebook-controller-system
name: odh-dashboard-cert
- patch: |-
- op: replace
path: /spec/controlPlaneRef/namespace
value: istio-system
target:
group: maistra.io
version: v1
kind: ServiceMeshMember
name: default
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: maistra.io/v1
kind: ServiceMeshMember
metadata:
name: default
spec:
controlPlaneRef:
namespace: odh-notebook-controller-system
name: custom
49 changes: 45 additions & 4 deletions components/odh-notebook-controller/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ CONTAINER_ENGINE ?= podman
ENVTEST_K8S_VERSION = 1.23

# Kubernetes configuration
K8S_NAMESPACE = odh-notebook-controller-system
K8S_NAMESPACE ?= odh-notebook-controller-system

# Webhook configuration
WEBHOOK_PORT = 8443
Expand Down Expand Up @@ -136,14 +136,45 @@ setup: manifests kustomize ## Replace Kustomize manifests with your environment
sed -i'' -e 's,newName: .*,newName: '"${IMG}"',' ./config/base/kustomization.yaml
sed -i'' -e 's,newTag: .*,newTag: '"${TAG}"',' ./config/base/kustomization.yaml

.PHONE: deploy-kf
.PHONY: setup-service-mesh
setup-service-mesh: kustomize ## Replace Kustomize manifests with your environment configuration.
sed -i'' -e 's,namespace: .*,namespace: '"${K8S_NAMESPACE}"',' \
../notebook-controller/config/overlays/service-mesh/smm.yaml
sed -i'' -e 's,namespace: .*,namespace: '"${K8S_NAMESPACE}"',' \
../notebook-controller/config/overlays/service-mesh/kustomization.yaml
sed -i'' -e 's,newName: .*,newName: '"${KF_IMG}"',' \
../notebook-controller/config/overlays/service-mesh/kustomization.yaml
sed -i'' -e 's,newTag: .*,newTag: '"${KF_TAG}"',' \
../notebook-controller/config/overlays/service-mesh/kustomization.yaml
sed -i'' -e 's,host: .*,host: opendatahub.'"$(shell kubectl get ingress.config.openshift.io cluster -o 'jsonpath={.spec.domain}')"',' \
../notebook-controller/config/overlays/service-mesh/gateway-route.yaml

.PHONY: deploy-service-mesh
deploy-service-mesh: ## Deploy Service Mesh to the Openshift cluster.
./e2e/scripts/install-ossm-release.sh install-operators
./e2e/scripts/install-ossm-release.sh install-smcp

.PHONY: undeploy-service-mesh
undeploy-service-mesh: ## Undeploy Service Mesh and related operators
./e2e/scripts/install-ossm-release.sh delete-smcp
./e2e/scripts/install-ossm-release.sh delete-operators

.PHONY: deploy-kf
deploy-kf: setup-kf ## Deploy kubeflow controller to the Openshift cluster.
$(KUSTOMIZE) build ../notebook-controller/config/overlays/openshift | oc apply -f -

.PHONY: deploy
deploy: setup deploy-kf ## Deploy controller to the Openshift cluster.
$(KUSTOMIZE) build config/base | oc apply -f -

.PHONY: deploy-with-mesh
deploy-with-mesh: setup-service-mesh deploy
$(KUSTOMIZE) build ../notebook-controller/config/overlays/service-mesh | oc apply -f -

.PHONY: undeploy-with-mesh
undeploy-with-mesh: setup-service-mesh undeploy
$(KUSTOMIZE) build ../notebook-controller/config/overlays/service-mesh | oc delete --ignore-not-found=true -f -

.PHONY: deploy-dev
deploy-dev: setup deploy-kf ## Deploy controller to the Openshift cluster.
$(KUSTOMIZE) build config/development | oc apply -f -
Expand All @@ -168,14 +199,24 @@ undeploy: setup undeploy-kf ## Undeploy controller from the Openshift cluster.
undeploy-dev: setup undeploy-kf ## Undeploy controller from the Openshift cluster.
$(KUSTOMIZE) build config/development | oc delete --ignore-not-found=true -f -

e2e-test-%:
$(eval deploymentMode:=$(subst e2e-test-,,$@))
go test ./e2e/ -run ^TestE2ENotebookController -v \
--nb-namespace=$(K8S_NAMESPACE) \
--deploymentMode=$(deploymentMode) \
${E2E_TEST_FLAGS}

.PHONY: e2e-test
e2e-test: ## Run e2e tests for the controller
go test ./e2e/ -run ^TestE2ENotebookController -v --nb-namespace=${K8S_NAMESPACE} ${E2E_TEST_FLAGS}
e2e-test: e2e-test-oauth ## Run e2e tests for the controller with oauth proxy
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to keep existing target behavior - to run only tests scenarios where oauth-proxy approach is defined. Happy to adjust based on your needs for CI. There's already a script in place to run the service mesh scenario https://github.com/opendatahub-io/kubeflow/pull/96/files#diff-13d60fc682972fcc720de0f06e1826afc91ba40af5d5c8bd987b370d44d0a332


.PHONY: run-ci-e2e-tests
run-ci-e2e-tests:
bash ./run-e2e-test.sh

.PHONY: run-ci-e2e-tests-service-mesh
run-ci-e2e-service-mesh-tests:
bash ./run-e2e-test-service-mesh.sh

##@ Build Dependencies

## Location to install dependencies to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (

const (
AnnotationInjectOAuth = "notebooks.opendatahub.io/inject-oauth"
AnnotationServiceMesh = "opendatahub.io/service-mesh"
AnnotationValueReconciliationLock = "odh-notebook-controller-lock"
AnnotationLogoutUrl = "notebooks.opendatahub.io/oauth-logout-url"
)
Expand Down Expand Up @@ -79,6 +80,17 @@ func OAuthInjectionIsEnabled(meta metav1.ObjectMeta) bool {
}
}

// ServiceMeshIsEnabled returns true if the notebook should be part of
// the service mesh.
func ServiceMeshIsEnabled(meta metav1.ObjectMeta) bool {
if meta.Annotations[AnnotationServiceMesh] != "" {
result, _ := strconv.ParseBool(meta.Annotations[AnnotationServiceMesh])
return result
} else {
return false
}
}

// ReconciliationLockIsEnabled returns true if the reconciliation lock
// annotation is present in the notebook.
func ReconciliationLockIsEnabled(meta metav1.ObjectMeta) bool {
Expand Down Expand Up @@ -151,37 +163,38 @@ func (r *OpenshiftNotebookReconciler) Reconcile(ctx context.Context, req ctrl.Re
return ctrl.Result{}, err
}

// Create the objects required by the OAuth proxy sidecar (see
// notebook_oauth.go file)
if OAuthInjectionIsEnabled(notebook.ObjectMeta) {
// Call the OAuth Service Account reconciler
err = r.ReconcileOAuthServiceAccount(notebook, ctx)
if err != nil {
return ctrl.Result{}, err
}
if !ServiceMeshIsEnabled(notebook.ObjectMeta) {
// Create the objects required by the OAuth proxy sidecar (see notebook_oauth.go file)
if OAuthInjectionIsEnabled(notebook.ObjectMeta) {

// Call the OAuth Service reconciler
err = r.ReconcileOAuthService(notebook, ctx)
if err != nil {
return ctrl.Result{}, err
}
err = r.ReconcileOAuthServiceAccount(notebook, ctx)
if err != nil {
return ctrl.Result{}, err
}

// Call the OAuth Secret reconciler
err = r.ReconcileOAuthSecret(notebook, ctx)
if err != nil {
return ctrl.Result{}, err
}
// Call the OAuth Service reconciler
err = r.ReconcileOAuthService(notebook, ctx)
if err != nil {
return ctrl.Result{}, err
}

// Call the OAuth Route reconciler
err = r.ReconcileOAuthRoute(notebook, ctx)
if err != nil {
return ctrl.Result{}, err
}
} else {
// Call the route reconciler (see notebook_route.go file)
err = r.ReconcileRoute(notebook, ctx)
if err != nil {
return ctrl.Result{}, err
// Call the OAuth Secret reconciler
err = r.ReconcileOAuthSecret(notebook, ctx)
if err != nil {
return ctrl.Result{}, err
}

// Call the OAuth Route reconciler
err = r.ReconcileOAuthRoute(notebook, ctx)
if err != nil {
return ctrl.Result{}, err
}
} else {
// Call the route reconciler (see notebook_route.go file)
err = r.ReconcileRoute(notebook, ctx)
if err != nil {
return ctrl.Result{}, err
}
}
}

Expand Down
Loading