From e997b15eb0114f083eee1b635b14023f7092c107 Mon Sep 17 00:00:00 2001 From: Diego Lovison Date: Thu, 18 Jul 2024 18:55:25 -0300 Subject: [PATCH 01/18] Move embedded binary and bash calls from YAML to a single script --- .github/scripts/tests/kind-integration.sh | 95 ++++++++++++++++ .github/workflows/kind-integration.yml | 127 +--------------------- 2 files changed, 101 insertions(+), 121 deletions(-) create mode 100755 .github/scripts/tests/kind-integration.sh diff --git a/.github/scripts/tests/kind-integration.sh b/.github/scripts/tests/kind-integration.sh new file mode 100755 index 000000000..e11b795c4 --- /dev/null +++ b/.github/scripts/tests/kind-integration.sh @@ -0,0 +1,95 @@ +#!/bin/bash +set -e + +if [ "$GIT_WORKSPACE" = "" ]; then + echo "GIT_WORKSPACE variable not definied. Should be the root of the source code. Example GIT_WORKSPACE=/home/dev/git/data-science-pipelines-operator" && exit +fi + +if [ "$REGISTRY_ADDRESS" = "" ]; then + echo "REGISTRY_ADDRESS variable not definied." && exit +fi + +# Env vars +IMAGE_REPO_DSPO="data-science-pipelines-operator" +DSPA_NAMESPACE="test-dspa" +DSPA_EXTERNAL_NAMESPACE="dspa-ext" +MINIO_NAMESPACE="test-minio" +MARIADB_NAMESPACE="test-mariadb" +PYPISERVER_NAMESPACE="test-pypiserver" +DSPA_NAME="test-dspa" +DSPA_EXTERNAL_NAME="dspa-ext" +DSPA_DEPLOY_WAIT_TIMEOUT="300" +INTEGRATION_TESTS_DIR="${GIT_WORKSPACE}/tests" +DSPA_PATH="${GIT_WORKSPACE}/tests/resources/dspa-lite.yaml" +DSPA_EXTERNAL_PATH="${GIT_WORKSPACE}/tests/resources/dspa-external-lite.yaml" +CONFIG_DIR="${GIT_WORKSPACE}/config" +RESOURCES_DIR_CRD="${GIT_WORKSPACE}/.github/resources" +DSPO_IMAGE="${REGISTRY_ADDRESS}/data-science-pipelines-operator" +OPENDATAHUB_NAMESPACE="opendatahub" +RESOURCES_DIR_PYPI="${GIT_WORKSPACE}/.github/resources/pypiserver/base" + +# TODO: Consolidate testing CRDS (2 locations) +# Apply OCP CRDs +kubectl apply -f ${RESOURCES_DIR_CRD}/crds +kubectl apply -f "${CONFIG_DIR}/crd/external/route.openshift.io_routes.yaml" + +# Build image +( cd $GIT_WORKSPACE && make podman-build -e IMG="${DSPO_IMAGE}" ) + +# Create opendatahub namespace +kubectl create namespace $OPENDATAHUB_NAMESPACE + +# Deploy Argo Lite +( cd "${GIT_WORKSPACE}/.github/resources/argo-lite" && kustomize build . | kubectl -n $OPENDATAHUB_NAMESPACE apply -f - ) + +# Deploy DSPO +( cd $GIT_WORKSPACE && make podman-push -e IMG="${DSPO_IMAGE}" ) +( cd $GIT_WORKSPACE && make deploy-kind -e IMG="${DSPO_IMAGE}" ) + +# Create Minio Namespace +kubectl create namespace $MINIO_NAMESPACE + +# Deploy Minio +( cd "${GIT_WORKSPACE}/.github/resources/minio" && kustomize build . | kubectl -n $MINIO_NAMESPACE apply -f - ) + +# Create MariaDB Namespace +kubectl create namespace $MARIADB_NAMESPACE + +# Deploy MariaDB +( cd "${GIT_WORKSPACE}/.github/resources/mariadb" && kustomize build . | kubectl -n $MARIADB_NAMESPACE apply -f - ) + +# Create Pypiserver Namespace +kubectl create namespace $PYPISERVER_NAMESPACE + +# Deploy pypi-server +( cd "${GIT_WORKSPACE}/.github/resources/pypiserver/base" && kustomize build . | kubectl -n $PYPISERVER_NAMESPACE apply -f - ) + +# Wait for Dependencies (DSPO, Minio, Mariadb, Pypi server) +kubectl wait -n $OPENDATAHUB_NAMESPACE --timeout=60s --for=condition=Available=true deployment data-science-pipelines-operator-controller-manager +kubectl wait -n $MARIADB_NAMESPACE --timeout=60s --for=condition=Available=true deployment mariadb +kubectl wait -n $MINIO_NAMESPACE --timeout=60s --for=condition=Available=true deployment minio +kubectl wait -n $PYPISERVER_NAMESPACE --timeout=60s --for=condition=Available=true deployment pypi-server + +# Upload Python Packages to pypi-server +( cd "${GIT_WORKSPACE}/.github/scripts/python_package_upload" && sh package_upload.sh ) + +# Create DSPA Namespace +kubectl create namespace $DSPA_NAMESPACE + +# Create Namespace for DSPA with External connections +kubectl create namespace $DSPA_EXTERNAL_NAMESPACE + +# Apply MariaDB and Minio Secrets and Configmaps in the External Namespace +( cd "${GIT_WORKSPACE}/.github/resources/external-pre-reqs" && kustomize build . | oc -n $DSPA_EXTERNAL_NAMESPACE apply -f - ) + +# Apply PIP Server ConfigMap +( cd "${GIT_WORKSPACE}/.github/resources/pypiserver/base" && kubectl apply -f $RESOURCES_DIR_PYPI/nginx-tls-config.yaml -n $DSPA_NAMESPACE ) + +# Run tests +( cd $GIT_WORKSPACE && make integrationtest K8SAPISERVERHOST=$(oc whoami --show-server) DSPANAMESPACE=${DSPA_NAMESPACE} DSPAPATH=${DSPA_PATH} ) + +# Run tests for DSPA with External Connections +( cd $GIT_WORKSPACE && make integrationtest K8SAPISERVERHOST=$(oc whoami --show-server) DSPANAMESPACE=${DSPA_EXTERNAL_NAMESPACE} DSPAPATH=${DSPA_EXTERNAL_PATH} ) + +# Clean up +( cd $GIT_WORKSPACE && make undeploy-kind ) diff --git a/.github/workflows/kind-integration.yml b/.github/workflows/kind-integration.yml index b3ec4200e..72d5940d6 100644 --- a/.github/workflows/kind-integration.yml +++ b/.github/workflows/kind-integration.yml @@ -11,6 +11,7 @@ on: - config/** - tests/** - .github/resources/** + - '.github/workflows/kind-integration.yml' types: - opened - reopened @@ -22,18 +23,8 @@ concurrency: cancel-in-progress: true env: - IMAGE_REPO_DSPO: data-science-pipelines-operator - DSPA_NAMESPACE: test-dspa - DSPA_EXTERNAL_NAMESPACE: dspa-ext - MINIO_NAMESPACE: test-minio - MARIADB_NAMESPACE: test-mariadb - PYPISERVER_NAMESPACE: test-pypiserver - DSPA_NAME: test-dspa - DSPA_EXTERNAL_NAME: dspa-ext - DSPA_DEPLOY_WAIT_TIMEOUT: 300 - INTEGRATION_TESTS_DIR: ${{ github.workspace }}/tests - DSPA_PATH: ${{ github.workspace }}/tests/resources/dspa-lite.yaml - DSPA_EXTERNAL_PATH: ${{ github.workspace }}/tests/resources/dspa-external-lite.yaml + GIT_WORKSPACE: ${{ github.workspace }} + jobs: dspo-tests: runs-on: ubuntu-20.04 @@ -53,113 +44,7 @@ jobs: - name: Setup and start KinD cluster uses: ./.github/actions/kind - # TODO: Consolidate testing CRDS (2 locations) - - name: Apply OCP CRDs - env: - RESOURCES_DIR: ${{ github.workspace }}/.github/resources - CONFIG_DIR: ${{ github.workspace }}/config - run: | - kubectl apply -f ${{ env.RESOURCES_DIR }}/crds - kubectl apply -f ${{ env.CONFIG_DIR }}/crd/external/route.openshift.io_routes.yaml - - - name: Build image - env: - DSPO_IMAGE: ${{env.REGISTRY_ADDRESS}}/data-science-pipelines-operator - run: | - make podman-build -e IMG="${DSPO_IMAGE}" - - - name: Create opendatahub namespace - run: | - kubectl create namespace opendatahub - - - name: Deploy Argo Lite - working-directory: ${{ github.workspace }}/.github/resources/argo-lite - run: | - kustomize build . | kubectl apply -f - - - - name: Deploy DSPO - env: - DSPO_IMAGE: ${{env.REGISTRY_ADDRESS}}/data-science-pipelines-operator - run: | - make podman-push -e IMG="${DSPO_IMAGE}" - make deploy-kind -e IMG="${DSPO_IMAGE}" - - - name: Create Minio Namespace - run: | - kubectl create namespace ${{ env.MINIO_NAMESPACE }} - - - name: Deploy Minio - working-directory: ${{ github.workspace }}/.github/resources/minio - run: | - kustomize build . | oc -n ${{ env.MINIO_NAMESPACE }} apply -f - - - - name: Create MariaDB Namespace - run: | - kubectl create namespace ${{ env.MARIADB_NAMESPACE }} - - - name: Deploy MariaDB - working-directory: ${{ github.workspace }}/.github/resources/mariadb - run: | - kustomize build . | oc -n ${{ env.MARIADB_NAMESPACE }} apply -f - - - - name: Create Pypiserver Namespace - run: | - kubectl create namespace ${{ env.PYPISERVER_NAMESPACE }} - - - name: Deploy pypi-server - working-directory: ${{ github.workspace }}/.github/resources/pypiserver/base - run: | - kustomize build . | oc -n ${{ env.PYPISERVER_NAMESPACE }} apply -f - - - - name: Wait for Dependencies (DSPO, Minio, Mariadb, Pypi server) - run: | - kubectl wait -n opendatahub --timeout=60s --for=condition=Available=true deployment data-science-pipelines-operator-controller-manager - kubectl wait -n ${{ env.MARIADB_NAMESPACE }} --timeout=60s --for=condition=Available=true deployment mariadb - kubectl wait -n ${{ env.MINIO_NAMESPACE }} --timeout=60s --for=condition=Available=true deployment minio - kubectl wait -n ${{ env.PYPISERVER_NAMESPACE }} --timeout=60s --for=condition=Available=true deployment pypi-server - - - name: Upload Python Packages to pypi-server - working-directory: ${{ github.workspace }}/.github/scripts/python_package_upload - run: | - sh package_upload.sh - - - name: Create DSPA Namespace - run: | - kubectl create namespace ${{ env.DSPA_NAMESPACE }} - - - name: Create Namespace for DSPA with External connections - run: | - kubectl create namespace ${{ env.DSPA_EXTERNAL_NAMESPACE }} - - - name: Apply MariaDB and Minio Secrets and Configmaps in the External Namespace - working-directory: ${{ github.workspace }}/.github/resources/external-pre-reqs - run: | - kustomize build . | oc -n ${{ env.DSPA_EXTERNAL_NAMESPACE }} apply -f - - - - name: Apply PIP Server ConfigMap - env: - RESOURCES_DIR: ${{ github.workspace }}/.github/resources/pypiserver/base - run: | - kubectl apply -f $RESOURCES_DIR/nginx-tls-config.yaml -n ${{ env.DSPA_NAMESPACE }} - - - name: Run tests - working-directory: ${{ github.workspace }} - env: - NAMESPACE: ${{ env.DSPA_NAMESPACE }} - DSPA_NAME: ${{ env.DSPA_NAME }} - DSPA_PATH: ${{ env.DSPA_PATH }} - run: | - make integrationtest K8SAPISERVERHOST=$(oc whoami --show-server) DSPANAMESPACE=${NAMESPACE} DSPAPATH=${DSPA_PATH} - - - name: Run tests for DSPA with External Connections - working-directory: ${{ github.workspace }} - env: - NAMESPACE: ${{ env.DSPA_EXTERNAL_NAMESPACE }} - DSPA_NAME: ${{ env.DSPA_EXTERNAL_NAME }} - DSPA_EXTERNAL_PATH: ${{ env.DSPA_EXTERNAL_PATH }} - run: | - make integrationtest K8SAPISERVERHOST=$(oc whoami --show-server) DSPANAMESPACE=${NAMESPACE} DSPAPATH=${DSPA_EXTERNAL_PATH} - - - name: Clean up + - name: Run test + working-directory: ${{ github.workspace }}/.github/scripts/tests run: | - make undeploy-kind + sh kind-integration.sh From ac77225c743221cc36cb5fd3b83f4cab2f28272d Mon Sep 17 00:00:00 2001 From: Diego Lovison Date: Fri, 19 Jul 2024 11:44:11 -0300 Subject: [PATCH 02/18] Move comment to echo --- .github/scripts/tests/kind-integration.sh | 80 +++++++++++++++++------ 1 file changed, 60 insertions(+), 20 deletions(-) diff --git a/.github/scripts/tests/kind-integration.sh b/.github/scripts/tests/kind-integration.sh index e11b795c4..bccd536c8 100755 --- a/.github/scripts/tests/kind-integration.sh +++ b/.github/scripts/tests/kind-integration.sh @@ -29,67 +29,107 @@ OPENDATAHUB_NAMESPACE="opendatahub" RESOURCES_DIR_PYPI="${GIT_WORKSPACE}/.github/resources/pypiserver/base" # TODO: Consolidate testing CRDS (2 locations) -# Apply OCP CRDs +echo "---------------------------------" +echo "# Apply OCP CRDs" +echo "---------------------------------" kubectl apply -f ${RESOURCES_DIR_CRD}/crds kubectl apply -f "${CONFIG_DIR}/crd/external/route.openshift.io_routes.yaml" -# Build image +echo "---------------------------------" +echo "Build image" +echo "---------------------------------" ( cd $GIT_WORKSPACE && make podman-build -e IMG="${DSPO_IMAGE}" ) -# Create opendatahub namespace +echo "---------------------------------" +echo "Create opendatahub namespace" +echo "---------------------------------" kubectl create namespace $OPENDATAHUB_NAMESPACE -# Deploy Argo Lite +echo "---------------------------------" +echo "Deploy Argo Lite" +echo "---------------------------------" ( cd "${GIT_WORKSPACE}/.github/resources/argo-lite" && kustomize build . | kubectl -n $OPENDATAHUB_NAMESPACE apply -f - ) -# Deploy DSPO +echo "---------------------------------" +echo "Deploy DSPO" +echo "---------------------------------" ( cd $GIT_WORKSPACE && make podman-push -e IMG="${DSPO_IMAGE}" ) ( cd $GIT_WORKSPACE && make deploy-kind -e IMG="${DSPO_IMAGE}" ) -# Create Minio Namespace +echo "---------------------------------" +echo "Create Minio Namespace" +echo "---------------------------------" kubectl create namespace $MINIO_NAMESPACE -# Deploy Minio +echo "---------------------------------" +echo "Deploy Minio" +echo "---------------------------------" ( cd "${GIT_WORKSPACE}/.github/resources/minio" && kustomize build . | kubectl -n $MINIO_NAMESPACE apply -f - ) -# Create MariaDB Namespace +echo "---------------------------------" +echo "Create MariaDB Namespace" +echo "---------------------------------" kubectl create namespace $MARIADB_NAMESPACE -# Deploy MariaDB +echo "---------------------------------" +echo "Deploy MariaDB" +echo "---------------------------------" ( cd "${GIT_WORKSPACE}/.github/resources/mariadb" && kustomize build . | kubectl -n $MARIADB_NAMESPACE apply -f - ) -# Create Pypiserver Namespace +echo "---------------------------------" +echo "Create Pypiserver Namespace" +echo "---------------------------------" kubectl create namespace $PYPISERVER_NAMESPACE -# Deploy pypi-server +echo "---------------------------------" +echo "Deploy pypi-server" +echo "---------------------------------" ( cd "${GIT_WORKSPACE}/.github/resources/pypiserver/base" && kustomize build . | kubectl -n $PYPISERVER_NAMESPACE apply -f - ) -# Wait for Dependencies (DSPO, Minio, Mariadb, Pypi server) +echo "---------------------------------" +echo "Wait for Dependencies (DSPO, Minio, Mariadb, Pypi server)" +echo "---------------------------------" kubectl wait -n $OPENDATAHUB_NAMESPACE --timeout=60s --for=condition=Available=true deployment data-science-pipelines-operator-controller-manager kubectl wait -n $MARIADB_NAMESPACE --timeout=60s --for=condition=Available=true deployment mariadb kubectl wait -n $MINIO_NAMESPACE --timeout=60s --for=condition=Available=true deployment minio kubectl wait -n $PYPISERVER_NAMESPACE --timeout=60s --for=condition=Available=true deployment pypi-server -# Upload Python Packages to pypi-server +echo "---------------------------------" +echo "Upload Python Packages to pypi-server" +echo "---------------------------------" ( cd "${GIT_WORKSPACE}/.github/scripts/python_package_upload" && sh package_upload.sh ) -# Create DSPA Namespace +echo "---------------------------------" +echo "Create DSPA Namespace" +echo "---------------------------------" kubectl create namespace $DSPA_NAMESPACE -# Create Namespace for DSPA with External connections +echo "---------------------------------" +echo "Create Namespace for DSPA with External connections" +echo "---------------------------------" kubectl create namespace $DSPA_EXTERNAL_NAMESPACE -# Apply MariaDB and Minio Secrets and Configmaps in the External Namespace +echo "---------------------------------" +echo "Apply MariaDB and Minio Secrets and Configmaps in the External Namespace" +echo "---------------------------------" ( cd "${GIT_WORKSPACE}/.github/resources/external-pre-reqs" && kustomize build . | oc -n $DSPA_EXTERNAL_NAMESPACE apply -f - ) -# Apply PIP Server ConfigMap +echo "---------------------------------" +echo "Apply PIP Server ConfigMap" +echo "---------------------------------" ( cd "${GIT_WORKSPACE}/.github/resources/pypiserver/base" && kubectl apply -f $RESOURCES_DIR_PYPI/nginx-tls-config.yaml -n $DSPA_NAMESPACE ) -# Run tests +echo "---------------------------------" +echo "Run tests" +echo "---------------------------------" ( cd $GIT_WORKSPACE && make integrationtest K8SAPISERVERHOST=$(oc whoami --show-server) DSPANAMESPACE=${DSPA_NAMESPACE} DSPAPATH=${DSPA_PATH} ) -# Run tests for DSPA with External Connections +echo "---------------------------------" +echo "Run tests for DSPA with External Connections" +echo "---------------------------------" ( cd $GIT_WORKSPACE && make integrationtest K8SAPISERVERHOST=$(oc whoami --show-server) DSPANAMESPACE=${DSPA_EXTERNAL_NAMESPACE} DSPAPATH=${DSPA_EXTERNAL_PATH} ) -# Clean up +echo "---------------------------------" +echo "Clean up" +echo "---------------------------------" ( cd $GIT_WORKSPACE && make undeploy-kind ) From dbf9bd9c531b3318a2378e2075db135316cc61ae Mon Sep 17 00:00:00 2001 From: Diego Lovison Date: Fri, 19 Jul 2024 11:44:45 -0300 Subject: [PATCH 03/18] fix type --- .github/scripts/tests/kind-integration.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/scripts/tests/kind-integration.sh b/.github/scripts/tests/kind-integration.sh index bccd536c8..ab0050180 100755 --- a/.github/scripts/tests/kind-integration.sh +++ b/.github/scripts/tests/kind-integration.sh @@ -2,11 +2,11 @@ set -e if [ "$GIT_WORKSPACE" = "" ]; then - echo "GIT_WORKSPACE variable not definied. Should be the root of the source code. Example GIT_WORKSPACE=/home/dev/git/data-science-pipelines-operator" && exit + echo "GIT_WORKSPACE variable not defined. Should be the root of the source code. Example GIT_WORKSPACE=/home/dev/git/data-science-pipelines-operator" && exit fi if [ "$REGISTRY_ADDRESS" = "" ]; then - echo "REGISTRY_ADDRESS variable not definied." && exit + echo "REGISTRY_ADDRESS variable not defined." && exit fi # Env vars From 9fa0d43709d0f76b7bb94548d59870471fb8dde9 Mon Sep 17 00:00:00 2001 From: Humair Khan Date: Wed, 24 Jul 2024 17:00:56 -0400 Subject: [PATCH 04/18] increase default expiry for signed url Signed-off-by: Humair Khan --- api/v1alpha1/dspipeline_types.go | 4 ++-- ...tions.opendatahub.io_datasciencepipelinesapplications.yaml | 4 ++-- controllers/config/defaults.go | 2 +- .../case_0/expected/created/apiserver_deployment.yaml | 2 +- .../case_2/expected/created/apiserver_deployment.yaml | 2 +- .../case_3/expected/created/apiserver_deployment.yaml | 2 +- .../case_4/expected/created/apiserver_deployment.yaml | 2 +- .../case_5/expected/created/apiserver_deployment.yaml | 2 +- .../case_7/expected/created/apiserver_deployment.yaml | 2 +- .../case_8/expected/created/apiserver_deployment.yaml | 2 +- .../case_9/expected/created/apiserver_deployment.yaml | 2 +- 11 files changed, 13 insertions(+), 13 deletions(-) diff --git a/api/v1alpha1/dspipeline_types.go b/api/v1alpha1/dspipeline_types.go index 72af4a3f9..1dcb30a9c 100644 --- a/api/v1alpha1/dspipeline_types.go +++ b/api/v1alpha1/dspipeline_types.go @@ -150,8 +150,8 @@ type APIServer struct { // The expiry time (seconds) for artifact download links when // querying the dsp server via /apis/v2beta1/artifacts/{id}?share_url=true - // Default: 15 - // +kubebuilder:default:=15 + // Default: 60 + // +kubebuilder:default:=60 // +kubebuilder:validation:Optional ArtifactSignedURLExpirySeconds *int `json:"artifactSignedURLExpirySeconds"` } diff --git a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml index d28b446b9..a8af2afb7 100644 --- a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml +++ b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml @@ -68,10 +68,10 @@ spec: type: string type: object artifactSignedURLExpirySeconds: - default: 15 + default: 60 description: 'The expiry time (seconds) for artifact download links when querying the dsp server via /apis/v2beta1/artifacts/{id}?share_url=true - Default: 15' + Default: 60' type: integer autoUpdatePipelineDefaultVersion: default: true diff --git a/controllers/config/defaults.go b/controllers/config/defaults.go index 8dcb620fb..412254d96 100644 --- a/controllers/config/defaults.go +++ b/controllers/config/defaults.go @@ -60,7 +60,7 @@ const ( DefaultDBSecretKey = "password" GeneratedDBPasswordLength = 12 - DefaultSignedUrlExpiryTimeSeconds = 15 + DefaultSignedUrlExpiryTimeSeconds = 60 MariaDBName = "mlpipeline" MariaDBHostPrefix = "mariadb" diff --git a/controllers/testdata/declarative/case_0/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_0/expected/created/apiserver_deployment.yaml index 35eec3d89..8b92f211f 100644 --- a/controllers/testdata/declarative/case_0/expected/created/apiserver_deployment.yaml +++ b/controllers/testdata/declarative/case_0/expected/created/apiserver_deployment.yaml @@ -80,7 +80,7 @@ spec: - name: ML_PIPELINE_SERVICE_PORT_GRPC value: "8887" - name: SIGNED_URL_EXPIRY_TIME_SECONDS - value: "15" + value: "60" - name: EXECUTIONTYPE value: PipelineRun - name: CACHE_IMAGE diff --git a/controllers/testdata/declarative/case_2/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_2/expected/created/apiserver_deployment.yaml index d656ff5b7..810ddfc1c 100644 --- a/controllers/testdata/declarative/case_2/expected/created/apiserver_deployment.yaml +++ b/controllers/testdata/declarative/case_2/expected/created/apiserver_deployment.yaml @@ -80,7 +80,7 @@ spec: - name: ML_PIPELINE_SERVICE_PORT_GRPC value: "8887" - name: SIGNED_URL_EXPIRY_TIME_SECONDS - value: "15" + value: "60" - name: EXECUTIONTYPE value: PipelineRun - name: CACHE_IMAGE diff --git a/controllers/testdata/declarative/case_3/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_3/expected/created/apiserver_deployment.yaml index 3f7ccd43f..c79819661 100644 --- a/controllers/testdata/declarative/case_3/expected/created/apiserver_deployment.yaml +++ b/controllers/testdata/declarative/case_3/expected/created/apiserver_deployment.yaml @@ -80,7 +80,7 @@ spec: - name: ML_PIPELINE_SERVICE_PORT_GRPC value: "8887" - name: SIGNED_URL_EXPIRY_TIME_SECONDS - value: "15" + value: "60" - name: EXECUTIONTYPE value: PipelineRun - name: CACHE_IMAGE diff --git a/controllers/testdata/declarative/case_4/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_4/expected/created/apiserver_deployment.yaml index 6d163e59a..16131faad 100644 --- a/controllers/testdata/declarative/case_4/expected/created/apiserver_deployment.yaml +++ b/controllers/testdata/declarative/case_4/expected/created/apiserver_deployment.yaml @@ -80,7 +80,7 @@ spec: - name: ML_PIPELINE_SERVICE_PORT_GRPC value: "8887" - name: SIGNED_URL_EXPIRY_TIME_SECONDS - value: "15" + value: "60" - name: EXECUTIONTYPE value: PipelineRun - name: CACHE_IMAGE diff --git a/controllers/testdata/declarative/case_5/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_5/expected/created/apiserver_deployment.yaml index c5be15a7b..8f1937885 100644 --- a/controllers/testdata/declarative/case_5/expected/created/apiserver_deployment.yaml +++ b/controllers/testdata/declarative/case_5/expected/created/apiserver_deployment.yaml @@ -84,7 +84,7 @@ spec: - name: ML_PIPELINE_SERVICE_PORT_GRPC value: "8887" - name: SIGNED_URL_EXPIRY_TIME_SECONDS - value: "15" + value: "60" - name: EXECUTIONTYPE value: PipelineRun - name: CACHE_IMAGE diff --git a/controllers/testdata/declarative/case_7/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_7/expected/created/apiserver_deployment.yaml index c2e090ecc..b9603e2df 100644 --- a/controllers/testdata/declarative/case_7/expected/created/apiserver_deployment.yaml +++ b/controllers/testdata/declarative/case_7/expected/created/apiserver_deployment.yaml @@ -84,7 +84,7 @@ spec: - name: ML_PIPELINE_SERVICE_PORT_GRPC value: "8887" - name: SIGNED_URL_EXPIRY_TIME_SECONDS - value: "15" + value: "60" - name: EXECUTIONTYPE value: Workflow - name: DB_DRIVER_NAME diff --git a/controllers/testdata/declarative/case_8/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_8/expected/created/apiserver_deployment.yaml index 363525244..ae01ded70 100644 --- a/controllers/testdata/declarative/case_8/expected/created/apiserver_deployment.yaml +++ b/controllers/testdata/declarative/case_8/expected/created/apiserver_deployment.yaml @@ -92,7 +92,7 @@ spec: - name: ML_PIPELINE_SERVICE_PORT_GRPC value: "8887" - name: SIGNED_URL_EXPIRY_TIME_SECONDS - value: "15" + value: "60" - name: ML_PIPELINE_TLS_ENABLED value: "true" - name: EXECUTIONTYPE diff --git a/controllers/testdata/declarative/case_9/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_9/expected/created/apiserver_deployment.yaml index 788b2faaf..be53d39ba 100644 --- a/controllers/testdata/declarative/case_9/expected/created/apiserver_deployment.yaml +++ b/controllers/testdata/declarative/case_9/expected/created/apiserver_deployment.yaml @@ -84,7 +84,7 @@ spec: - name: ML_PIPELINE_SERVICE_PORT_GRPC value: "8887" - name: SIGNED_URL_EXPIRY_TIME_SECONDS - value: "15" + value: "60" - name: EXECUTIONTYPE value: Workflow - name: DB_DRIVER_NAME From f33d8b5c45982e88d75f57aeb23e89dd0bc1db0c Mon Sep 17 00:00:00 2001 From: Diego Lovison Date: Thu, 1 Aug 2024 16:19:28 -0300 Subject: [PATCH 05/18] Consolidate kustomize binaries in DSPO Makefile --- .github/workflows/kind-integration.yml | 1 + Makefile | 20 ++++++++------------ 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/.github/workflows/kind-integration.yml b/.github/workflows/kind-integration.yml index 72d5940d6..7d3571013 100644 --- a/.github/workflows/kind-integration.yml +++ b/.github/workflows/kind-integration.yml @@ -12,6 +12,7 @@ on: - tests/** - .github/resources/** - '.github/workflows/kind-integration.yml' + - Makefile types: - opened - reopened diff --git a/Makefile b/Makefile index fd9fd70f2..7120c56e6 100644 --- a/Makefile +++ b/Makefile @@ -177,17 +177,17 @@ undeploy: ## Undeploy controller from the K8s cluster specified in ~/.kube/confi $(KUSTOMIZE) build config/overlays/make-deploy | kubectl delete --ignore-not-found=$(ignore-not-found) -f - .PHONY: deploy-kind -deploy-kind: +deploy-kind: manifests kustomize cd config/overlays/kind-tests \ - && kustomize edit set image controller=${IMG} \ - && kustomize edit set namespace ${OPERATOR_NS} - kustomize build config/overlays/kind-tests | kubectl apply -f - + && $(KUSTOMIZE) edit set image controller=${IMG} \ + && $(KUSTOMIZE) edit set namespace ${OPERATOR_NS} + $(KUSTOMIZE) build config/overlays/kind-tests | kubectl apply -f - .PHONY: undeploy-kind -undeploy-kind: ## Undeploy controller from the K8s cluster specified in ~/.kube/config. Call with ignore-not-found=true to ignore resource not found errors during deletion. +undeploy-kind: manifests 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. cd config/overlays/kind-tests \ - && kustomize edit set namespace ${OPERATOR_NS} - kustomize build config/overlays/kind-tests | kubectl delete --ignore-not-found=$(ignore-not-found) -f - + && $(KUSTOMIZE) edit set namespace ${OPERATOR_NS} + $(KUSTOMIZE) build config/overlays/kind-tests | kubectl delete --ignore-not-found=$(ignore-not-found) -f - ##@ Build Dependencies @@ -204,11 +204,7 @@ ENVTEST ?= $(LOCALBIN)/setup-envtest ## Tool Versions arch:= $(shell uname -m) -ifeq ($(arch), ppc64le) -KUSTOMIZE_VERSION ?= v5.1.0 -else -KUSTOMIZE_VERSION ?= v3.8.7 -endif +KUSTOMIZE_VERSION ?= v5.2.1 CONTROLLER_TOOLS_VERSION ?= v0.10.0 KUSTOMIZE_INSTALL_SCRIPT ?= "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh" From d42a1302c38eb0f72e2eeb05dd9214ea32fd7e2c Mon Sep 17 00:00:00 2001 From: Greg Sheremeta Date: Mon, 5 Aug 2024 15:29:40 -0400 Subject: [PATCH 06/18] add ability to customize `kfp-launcher` ConfigMap data contents Allow the user to fully replace the `data` contents of the kfp-launcher ConfigMap. The `kfp-launcher` component requires a ConfigMap to exist in the namespace where it runs (i.e. the namespace where pipelines run). This ConfigMap contains object storage configuration, as well as pipeline root (object store root path where artifacts will be uploaded) configuration. Currently this ConfigMap *must* be named "kfp-launcher". We currently deploy a default copy of the kfp-launcher ConfigMap via DSPO, but a user may want to provide their own ConfigMap configuration, so that they can specify multiple object storage sources and paths. Add a `CustomKfpLauncherConfigMap` parameter to DSPA.ApiServer. When specified, the `data` contents of the `kfp-launcher` ConfigMap that DSPO writes will be fully replaced with the `data` contents of the ConfigMap specified here. Fixes: https://issues.redhat.com/browse/RHOAIENG-4528 Co-authored-by: Achyut Madhusudan --- api/v1alpha1/dspipeline_types.go | 13 +++++++++ ...b.io_datasciencepipelinesapplications.yaml | 15 ++++++++++ .../default/kfp_launcher_config.yaml.tmpl | 4 +++ .../v2/dspa-all-fields/dspa_all_fields.yaml | 1 + controllers/dspipeline_params.go | 24 ++++++++++++++++ controllers/dspipeline_params_test.go | 28 ++++++++++++++++++- controllers/testutil/util.go | 14 ++++++++++ 7 files changed, 98 insertions(+), 1 deletion(-) diff --git a/api/v1alpha1/dspipeline_types.go b/api/v1alpha1/dspipeline_types.go index 1dcb30a9c..8cbf80bf3 100644 --- a/api/v1alpha1/dspipeline_types.go +++ b/api/v1alpha1/dspipeline_types.go @@ -86,6 +86,19 @@ type APIServer struct { // for the api server to use instead. CustomServerConfig *ScriptConfigMap `json:"customServerConfigMap,omitempty"` + // When specified, the `data` contents of the `kfp-launcher` ConfigMap that DSPO writes + // will be fully replaced with the `data` contents of the ConfigMap specified here. + // This allows the user to fully replace the `data` contents of the kfp-launcher ConfigMap. + // The `kfp-launcher` component requires a ConfigMap to exist in the namespace + // where it runs (i.e. the namespace where pipelines run). This ConfigMap contains + // object storage configuration, as well as pipeline root (object store root path + // where artifacts will be uploaded) configuration. Currently this ConfigMap *must* + // be named "kfp-launcher". We currently deploy a default copy of the kfp-launcher + // ConfigMap via DSPO, but a user may want to provide their own ConfigMap configuration, + // so that they can specify multiple object storage sources and paths. + // +kubebuilder:validation:Optional + CustomKfpLauncherConfigMap string `json:"customKfpLauncherConfigMap,omitempty"` + // Default: true // Deprecated: DSP V1 only, will be removed in the future. // +kubebuilder:default:=true diff --git a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml index a8af2afb7..cf0807800 100644 --- a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml +++ b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml @@ -112,6 +112,21 @@ spec: description: 'Default: true Deprecated: DSP V1 only, will be removed in the future.' type: boolean + customKfpLauncherConfigMap: + description: When specified, the `data` contents of the `kfp-launcher` + ConfigMap that DSPO writes will be fully replaced with the `data` + contents of the ConfigMap specified here. This allows the user + to fully replace the `data` contents of the kfp-launcher ConfigMap. + The `kfp-launcher` component requires a ConfigMap to exist in + the namespace where it runs (i.e. the namespace where pipelines + run). This ConfigMap contains object storage configuration, + as well as pipeline root (object store root path where artifacts + will be uploaded) configuration. Currently this ConfigMap *must* + be named "kfp-launcher". We currently deploy a default copy + of the kfp-launcher ConfigMap via DSPO, but a user may want + to provide their own ConfigMap configuration, so that they can + specify multiple object storage sources and paths. + type: string customServerConfigMap: description: CustomServerConfig is a custom config file that you can provide for the api server to use instead. diff --git a/config/internal/apiserver/default/kfp_launcher_config.yaml.tmpl b/config/internal/apiserver/default/kfp_launcher_config.yaml.tmpl index 1d497b584..8d85bf00b 100644 --- a/config/internal/apiserver/default/kfp_launcher_config.yaml.tmpl +++ b/config/internal/apiserver/default/kfp_launcher_config.yaml.tmpl @@ -1,5 +1,8 @@ apiVersion: v1 data: + {{ if .APIServer.CustomKfpLauncherConfigMap }} + {{.CustomKfpLauncherConfigMapData}} + {{ else }} {{ if .ObjectStorageConnection.BasePath }} defaultPipelineRoot: s3://{{.ObjectStorageConnection.Bucket}}/{{.ObjectStorageConnection.BasePath}} {{ else }} @@ -13,6 +16,7 @@ data: secretName: {{.ObjectStorageConnection.CredentialsSecret.SecretName}} accessKeyKey: {{.ObjectStorageConnection.CredentialsSecret.AccessKey}} secretKeyKey: {{.ObjectStorageConnection.CredentialsSecret.SecretKey}} + {{ end }} kind: ConfigMap metadata: name: kfp-launcher diff --git a/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml b/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml index ab66c9474..8697a4611 100644 --- a/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml +++ b/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml @@ -11,6 +11,7 @@ metadata: spec: dspVersion: v2 apiServer: + customKfpLauncherConfigMap: configmapname deploy: true enableSamplePipeline: true image: quay.io/opendatahub/ds-pipelines-api-server:latest diff --git a/controllers/dspipeline_params.go b/controllers/dspipeline_params.go index 3c17c9bd4..aa5364ef4 100644 --- a/controllers/dspipeline_params.go +++ b/controllers/dspipeline_params.go @@ -66,6 +66,7 @@ type DSPAParams struct { Minio *dspa.Minio MLMD *dspa.MLMD WorkflowController *dspa.WorkflowController + CustomKfpLauncherConfigMapData string DBConnection ObjectStorageConnection @@ -646,6 +647,29 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip } } + if p.APIServer.CustomKfpLauncherConfigMap != "" { + cm, err := util.GetConfigMap(ctx, p.APIServer.CustomKfpLauncherConfigMap, p.Namespace, client) + if err != nil { + if apierrs.IsNotFound(err) { + log.Info(fmt.Sprintf("ConfigMap referenced by CustomKfpLauncherConfig not found: [%s], Error: %v", p.APIServer.CustomKfpLauncherConfigMap, err)) + return err + } else { + log.Info(fmt.Sprintf("Error fetching ConfigMap referenced by CustomKfpLauncherConfig: [%s], Error: %v", p.APIServer.CustomKfpLauncherConfigMap, err)) + return err + } + + } else { + // when setting a map into the `data` field of a ConfigMap, text/template works well with a json object + jsonData, err := json.Marshal(cm.Data) + if err != nil { + log.Info(fmt.Sprintf("Error reading data of ConfigMap referenced by CustomKfpLauncherConfig: [%s], Error: %v", p.APIServer.CustomKfpLauncherConfigMap, err)) + return err + } else { + p.CustomKfpLauncherConfigMapData = string(jsonData) + } + } + } + // Track whether the "ca-bundle.crt" configmap key from odh-trusted-ca bundle // was found, this will be used to decide whether we need to account for this // ourselves later or not. diff --git a/controllers/dspipeline_params_test.go b/controllers/dspipeline_params_test.go index 65e5c63c3..55609d897 100644 --- a/controllers/dspipeline_params_test.go +++ b/controllers/dspipeline_params_test.go @@ -18,13 +18,16 @@ limitations under the License. package controllers import ( + "encoding/json" + "testing" + dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" "github.com/opendatahub-io/data-science-pipelines-operator/controllers/testutil" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" - "testing" ) type Client struct { @@ -258,3 +261,26 @@ func TestExtractParams_CABundle(t *testing.T) { func strPtr(v string) *string { return &v } + +func TestExtractParams_WithCustomKfpLauncherConfigMap(t *testing.T) { + ctx, params, client := CreateNewTestObjects() + cmDataExpected := map[string]string{ + "this-is-the-only-thing": "that-should-be-in-kfp-launcher-now", + } + cm := v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-custom-kfp-launcher", + Namespace: "testnamespace", + }, + Data: cmDataExpected, + } + err := client.Create(ctx, &cm) + require.Nil(t, err) + + dspa := testutil.CreateDSPAWithCustomKfpLauncherConfigMap("my-custom-kfp-launcher") + err = params.ExtractParams(ctx, dspa, client.Client, client.Log) + require.Nil(t, err) + + cmDataExpectedJson, err := json.Marshal(cmDataExpected) + require.Equal(t, string(cmDataExpectedJson), params.CustomKfpLauncherConfigMapData) +} diff --git a/controllers/testutil/util.go b/controllers/testutil/util.go index 2bee136f7..d6a2c7f43 100644 --- a/controllers/testutil/util.go +++ b/controllers/testutil/util.go @@ -256,3 +256,17 @@ func CreateDSPAWithAPIServerPodtoPodTlsEnabled() *dspav1alpha1.DataSciencePipeli func boolPtr(b bool) *bool { return &b } + +func CreateDSPAWithCustomKfpLauncherConfigMap(configMapName string) *dspav1alpha1.DataSciencePipelinesApplication { + dspa := CreateEmptyDSPA() + dspa.Spec.DSPVersion = "v2" + // required, or we get an error because OCP certs aren't found + dspa.Spec.PodToPodTLS = boolPtr(false) + // required, or we get an error because this is required in v2 + dspa.Spec.MLMD.Deploy = true + dspa.Spec.APIServer = &dspav1alpha1.APIServer{ + Deploy: true, + CustomKfpLauncherConfigMap: configMapName, + } + return dspa +} From 8f7fabc640d52318a70e1b49a6e9e72a5533e81c Mon Sep 17 00:00:00 2001 From: Achyut Madhusudan Date: Tue, 30 Jul 2024 19:21:25 +0530 Subject: [PATCH 07/18] Clarify the "DSPA not found" log message that happens when a DSPA is deleted Co-authored-by: Greg Sheremeta --- controllers/dspipeline_controller.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/controllers/dspipeline_controller.go b/controllers/dspipeline_controller.go index a8b0a1a82..859a0ffae 100644 --- a/controllers/dspipeline_controller.go +++ b/controllers/dspipeline_controller.go @@ -177,7 +177,8 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. dspa := &dspav1alpha1.DataSciencePipelinesApplication{} err := r.Get(ctx, req.NamespacedName, dspa) if err != nil && apierrs.IsNotFound(err) { - log.Info("DSPA resource was not found") + // TODO change this to a Debug message when doing https://issues.redhat.com/browse/RHOAIENG-1650 + log.Info("DSPA resource was not found, assuming it was recently deleted, nothing to do here") return ctrl.Result{}, nil } else if err != nil { log.Error(err, "Encountered error when fetching DSPA") From 63d7afd74671ddd6e534fd125f1629fbd6018db1 Mon Sep 17 00:00:00 2001 From: "Ricardo M. Oliveira" Date: Wed, 10 Jul 2024 18:24:57 -0300 Subject: [PATCH 08/18] Expose API Server and Envoy endpoints and add a new Envoy condition Signed-off-by: Ricardo M. Oliveira --- api/v1alpha1/dspipeline_types.go | 14 ++++ api/v1alpha1/zz_generated.deepcopy.go | 33 ++++++++ ...b.io_datasciencepipelinesapplications.yaml | 17 +++++ .../v2/dspa-all-fields/dspa_all_fields.yaml | 13 ++++ controllers/apiserver.go | 45 +++++++++++ controllers/apiserver_test.go | 56 +++++++++++++- controllers/config/defaults.go | 1 + controllers/dspastatus/dspa_status.go | 16 ++++ controllers/dspipeline_controller.go | 46 ++++++++++++ controllers/metrics.go | 11 +++ controllers/mlmd.go | 50 +++++++++++++ controllers/mlmd_test.go | 75 ++++++++++++++++++- 12 files changed, 375 insertions(+), 2 deletions(-) diff --git a/api/v1alpha1/dspipeline_types.go b/api/v1alpha1/dspipeline_types.go index 8cbf80bf3..eb98fd4b5 100644 --- a/api/v1alpha1/dspipeline_types.go +++ b/api/v1alpha1/dspipeline_types.go @@ -407,9 +407,23 @@ type SecretKeyValue struct { } type DSPAStatus struct { + // +kubebuilder:validation:Optional + Components ComponentStatus `json:"components,omitempty"` Conditions []metav1.Condition `json:"conditions,omitempty"` } +type ComponentStatus struct { + // +kubebuilder:validation:Optional + Envoy ComponentDetailStatus `json:"envoy,omitempty"` + APIServer ComponentDetailStatus `json:"apiServer,omitempty"` +} + +type ComponentDetailStatus struct { + // +kubebuilder:validation:Optional + Url string `json:"url,omitempty"` + ExternalUrl string `json:"externalUrl,omitempty"` +} + //+kubebuilder:object:root=true //+kubebuilder:subresource:status //+kubebuilder:resource:shortName=dspa diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 29464a334..6deeb3b07 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -81,6 +81,38 @@ func (in *CABundle) DeepCopy() *CABundle { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ComponentDetailStatus) DeepCopyInto(out *ComponentDetailStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ComponentDetailStatus. +func (in *ComponentDetailStatus) DeepCopy() *ComponentDetailStatus { + if in == nil { + return nil + } + out := new(ComponentDetailStatus) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ComponentStatus) DeepCopyInto(out *ComponentStatus) { + *out = *in + out.Envoy = in.Envoy + out.APIServer = in.APIServer +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ComponentStatus. +func (in *ComponentStatus) DeepCopy() *ComponentStatus { + if in == nil { + return nil + } + out := new(ComponentStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DSPASpec) DeepCopyInto(out *DSPASpec) { *out = *in @@ -144,6 +176,7 @@ func (in *DSPASpec) DeepCopy() *DSPASpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DSPAStatus) DeepCopyInto(out *DSPAStatus) { *out = *in + out.Components = in.Components if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]v1.Condition, len(*in)) diff --git a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml index cf0807800..2100f40c4 100644 --- a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml +++ b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml @@ -886,6 +886,23 @@ spec: type: object status: properties: + components: + properties: + apiServer: + properties: + externalUrl: + type: string + url: + type: string + type: object + envoy: + properties: + externalUrl: + type: string + url: + type: string + type: object + type: object conditions: items: description: "Condition contains details for one aspect of the current diff --git a/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml b/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml index 8697a4611..b411bb975 100644 --- a/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml +++ b/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml @@ -184,6 +184,13 @@ spec: secretKey: somekey # example status fields status: + components: + envoy: + url: http://envoy.svc.cluster.local + externalUrl: https://envoy-dspa.example.com + apiServer: + url: http://envoy.svc.cluster.local + externalUrl: https://envoy-dspa.example.com conditions: - lastTransitionTime: '2024-03-14T22:04:25Z' message: Database connectivity successfully verified @@ -215,6 +222,12 @@ status: reason: MinimumReplicasAvailable status: 'True' type: ScheduledWorkflowReady + - lastTransitionTime: '2024-03-14T22:04:30Z' + message: 'Component [ds-pipeline-metadata-envoy] is minimally available.' + observedGeneration: 3 + reason: MinimumReplicasAvailable + status: 'True' + type: EnvoyReady - lastTransitionTime: '2024-03-14T22:06:37Z' message: All components are ready. observedGeneration: 3 diff --git a/controllers/apiserver.go b/controllers/apiserver.go index 33196d9ed..7c5f38f68 100644 --- a/controllers/apiserver.go +++ b/controllers/apiserver.go @@ -17,6 +17,7 @@ package controllers import ( "context" + "fmt" dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" v1 "github.com/openshift/api/route/v1" @@ -86,3 +87,47 @@ func (r *DSPAReconciler) ReconcileAPIServer(ctx context.Context, dsp *dspav1alph log.Info("Finished applying APIServer Resources") return nil } + +func (r *DSPAReconciler) GetAPIServerServiceHostname(ctx context.Context, dsp *dspav1alpha1.DataSciencePipelinesApplication) (string, error) { + service := &corev1.Service{} + namespacedNamed := types.NamespacedName{Name: "ds-pipeline-" + dsp.Name, Namespace: dsp.Namespace} + err := r.Get(ctx, namespacedNamed, service) + if err != nil { + return "", err + } + + // Loop over all Service ports, if a secured port is found + // set port and scheme to its secured ones and skip the loop + serviceScheme := "" + servicePort := "" + for i := 0; i < len(service.Spec.Ports); i++ { + servicePort = fmt.Sprintf("%d", service.Spec.Ports[i].Port) + if servicePort == "8443" || servicePort == "443" { + // If a secured port is found, just set scheme to 'https://' and skip the loop + serviceScheme = "https://" + break + } else { + serviceScheme = "http://" + } + } + + return serviceScheme + service.Name + "." + service.Namespace + ".svc.cluster.local:" + servicePort, nil +} + +func (r *DSPAReconciler) GetAPIServerRouteHostname(ctx context.Context, dsp *dspav1alpha1.DataSciencePipelinesApplication) (string, error) { + route := &v1.Route{} + namespacedNamed := types.NamespacedName{Name: "ds-pipeline-" + dsp.Name, Namespace: dsp.Namespace} + err := r.Get(ctx, namespacedNamed, route) + if err != nil { + return "", err + } + + serviceScheme := "" + if route.Spec.TLS != nil { + serviceScheme = "https://" + } else { + serviceScheme = "http://" + } + + return serviceScheme + route.Spec.Host, nil +} diff --git a/controllers/apiserver_test.go b/controllers/apiserver_test.go index 89f24273c..8d11138b3 100644 --- a/controllers/apiserver_test.go +++ b/controllers/apiserver_test.go @@ -18,9 +18,10 @@ limitations under the License. package controllers import ( - "github.com/opendatahub-io/data-science-pipelines-operator/controllers/config" "testing" + "github.com/opendatahub-io/data-science-pipelines-operator/controllers/config" + dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" @@ -120,3 +121,56 @@ func TestDontDeployAPIServer(t *testing.T) { assert.False(t, created) assert.Nil(t, err) } + +func TestApiServerEndpoints(t *testing.T) { + testNamespace := "testnamespace" + testDSPAName := "testdspa" + expectedAPIServerName := apiServerDefaultResourceNamePrefix + testDSPAName + + // Construct DSPASpec with deployed APIServer + dspa := &dspav1alpha1.DataSciencePipelinesApplication{ + Spec: dspav1alpha1.DSPASpec{ + APIServer: &dspav1alpha1.APIServer{ + Deploy: true, + }, + MLMD: &dspav1alpha1.MLMD{}, + Database: &dspav1alpha1.Database{ + DisableHealthCheck: false, + MariaDB: &dspav1alpha1.MariaDB{ + Deploy: true, + }, + }, + ObjectStorage: &dspav1alpha1.ObjectStorage{ + DisableHealthCheck: false, + Minio: &dspav1alpha1.Minio{ + Deploy: false, + Image: "someimage", + }, + }, + }, + } + + // Enrich DSPA with name+namespace + dspa.Name = testDSPAName + dspa.Namespace = testNamespace + + // Create Context, Fake Controller and Params + ctx, params, reconciler := CreateNewTestObjects() + err := params.ExtractParams(ctx, dspa, reconciler.Client, reconciler.Log) + assert.Nil(t, err) + + // Assert APIServer Deployment doesn't yet exist + deployment := &appsv1.Deployment{} + created, err := reconciler.IsResourceCreated(ctx, deployment, expectedAPIServerName, testNamespace) + assert.False(t, created) + assert.Nil(t, err) + + // Run test reconciliation + err = reconciler.ReconcileAPIServer(ctx, dspa, params) + assert.Nil(t, err) + + dspa_created := &dspav1alpha1.DataSciencePipelinesApplication{} + created, err = reconciler.IsResourceCreated(ctx, dspa, testDSPAName, testNamespace) + assert.NotNil(t, dspa_created.Status.Components.APIServer.Url) + assert.NotNil(t, dspa_created.Status.Components.APIServer.ExternalUrl) +} diff --git a/controllers/config/defaults.go b/controllers/config/defaults.go index 412254d96..22fa53954 100644 --- a/controllers/config/defaults.go +++ b/controllers/config/defaults.go @@ -139,6 +139,7 @@ const ( APIServerReady = "APIServerReady" PersistenceAgentReady = "PersistenceAgentReady" ScheduledWorkflowReady = "ScheduledWorkflowReady" + EnvoyReady = "EnvoyReady" CrReady = "Ready" ) diff --git a/controllers/dspastatus/dspa_status.go b/controllers/dspastatus/dspa_status.go index f8e2b0d7f..04cc74b68 100644 --- a/controllers/dspastatus/dspa_status.go +++ b/controllers/dspastatus/dspa_status.go @@ -2,6 +2,7 @@ package dspastatus import ( "fmt" + dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" "github.com/opendatahub-io/data-science-pipelines-operator/controllers/config" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -20,6 +21,8 @@ type DSPAStatus interface { SetScheduledWorkflowStatus(scheduledWorkflowReady metav1.Condition) + SetEnvoyStatus(envoyReady metav1.Condition) + GetConditions() []metav1.Condition } @@ -29,6 +32,7 @@ func NewDSPAStatus(dspa *dspav1alpha1.DataSciencePipelinesApplication) DSPAStatu apiServerCondition := BuildUnknownCondition(config.APIServerReady) persistenceAgentCondition := BuildUnknownCondition(config.PersistenceAgentReady) scheduledWorkflowReadyCondition := BuildUnknownCondition(config.ScheduledWorkflowReady) + envoyReadyCondition := BuildUnknownCondition(config.EnvoyReady) return &dspaStatus{ dspa: dspa, @@ -37,6 +41,7 @@ func NewDSPAStatus(dspa *dspav1alpha1.DataSciencePipelinesApplication) DSPAStatu apiServerReady: &apiServerCondition, persistenceAgentReady: &persistenceAgentCondition, scheduledWorkflowReady: &scheduledWorkflowReadyCondition, + envoyReady: &envoyReadyCondition, } } @@ -47,6 +52,7 @@ type dspaStatus struct { apiServerReady *metav1.Condition persistenceAgentReady *metav1.Condition scheduledWorkflowReady *metav1.Condition + envoyReady *metav1.Condition } func (s *dspaStatus) SetDatabaseNotReady(err error, reason string) { @@ -90,6 +96,10 @@ func (s *dspaStatus) SetScheduledWorkflowStatus(scheduledWorkflowReady metav1.Co s.scheduledWorkflowReady = &scheduledWorkflowReady } +func (s *dspaStatus) SetEnvoyStatus(envoyReady metav1.Condition) { + s.envoyReady = &envoyReady +} + func (s *dspaStatus) GetConditions() []metav1.Condition { componentConditions := []metav1.Condition{ *s.getDatabaseAvailableCondition(), @@ -97,6 +107,7 @@ func (s *dspaStatus) GetConditions() []metav1.Condition { *s.getApiServerReadyCondition(), *s.getPersistenceAgentReadyCondition(), *s.getScheduledWorkflowReadyCondition(), + *s.getEnvoyReadyCondition(), } allReady := true @@ -134,6 +145,7 @@ func (s *dspaStatus) GetConditions() []metav1.Condition { *s.apiServerReady, *s.persistenceAgentReady, *s.scheduledWorkflowReady, + *s.envoyReady, crReady, } @@ -167,6 +179,10 @@ func (s *dspaStatus) getScheduledWorkflowReadyCondition() *metav1.Condition { return s.scheduledWorkflowReady } +func (s *dspaStatus) getEnvoyReadyCondition() *metav1.Condition { + return s.envoyReady +} + func BuildTrueCondition(conditionType string, message string) metav1.Condition { condition := metav1.Condition{} condition.Type = conditionType diff --git a/controllers/dspipeline_controller.go b/controllers/dspipeline_controller.go index 859a0ffae..528b43295 100644 --- a/controllers/dspipeline_controller.go +++ b/controllers/dspipeline_controller.go @@ -19,6 +19,7 @@ package controllers import ( "context" "fmt" + "github.com/opendatahub-io/data-science-pipelines-operator/controllers/dspastatus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/controller" @@ -305,7 +306,10 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. err = r.ReconcileMLMD(dspa, params) if err != nil { + r.setStatusAsNotReady(config.EnvoyReady, err, dspaStatus.SetEnvoyStatus) return ctrl.Result{}, err + } else { + r.setStatus(ctx, "ds-pipeline-metadata-envoy-"+dspa.Name, config.EnvoyReady, dspa, dspaStatus.SetEnvoyStatus, log) } err = r.ReconcileWorkflowController(dspa, params) @@ -325,6 +329,7 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. util.GetConditionByType(config.APIServerReady, conditions): APIServerReadyMetric, util.GetConditionByType(config.PersistenceAgentReady, conditions): PersistenceAgentReadyMetric, util.GetConditionByType(config.ScheduledWorkflowReady, conditions): ScheduledWorkflowReadyMetric, + util.GetConditionByType(config.EnvoyReady, conditions): EnvoyReadyMetric, util.GetConditionByType(config.CrReady, conditions): CrReadyMetric, } r.PublishMetrics(dspa, metricsMap) @@ -356,6 +361,7 @@ func (r *DSPAReconciler) setStatus(ctx context.Context, resourceName string, con func (r *DSPAReconciler) updateStatus(ctx context.Context, dspa *dspav1alpha1.DataSciencePipelinesApplication, dspaStatus dspastatus.DSPAStatus, log logr.Logger, req ctrl.Request) { r.refreshDspa(ctx, dspa, req, log) + dspa.Status.Components = r.GetComponents(ctx, dspa) dspa.Status.Conditions = dspaStatus.GetConditions() err := r.Status().Update(ctx, dspa) if err != nil { @@ -502,6 +508,46 @@ func (r *DSPAReconciler) PublishMetrics(dspa *dspav1alpha1.DataSciencePipelinesA } } +func (r *DSPAReconciler) GetComponents(ctx context.Context, dspa *dspav1alpha1.DataSciencePipelinesApplication) dspav1alpha1.ComponentStatus { + log := r.Log.WithValues("namespace", dspa.Namespace).WithValues("dspa_name", dspa.Name) + log.Info("Updating components endpoints") + + envoyUrl, err := r.GetEnvoyServiceHostname(ctx, dspa) + if err != nil { + log.Info(err.Error()) + } + + envoyExternalUrl, err := r.GetEnvoyRouteHostname(ctx, dspa) + if err != nil { + log.Info(err.Error()) + } + + envoyComponent := &dspav1alpha1.ComponentDetailStatus{ + Url: envoyUrl, + ExternalUrl: envoyExternalUrl, + } + + apiServerUrl, err := r.GetAPIServerServiceHostname(ctx, dspa) + if err != nil { + log.Info(err.Error()) + } + + apiServerExternalUrl, err := r.GetAPIServerRouteHostname(ctx, dspa) + if err != nil { + log.Info(err.Error()) + } + + apiServerComponent := &dspav1alpha1.ComponentDetailStatus{ + Url: apiServerUrl, + ExternalUrl: apiServerExternalUrl, + } + + return dspav1alpha1.ComponentStatus{ + Envoy: *envoyComponent, + APIServer: *apiServerComponent, + } +} + // SetupWithManager sets up the controller with the Manager. func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/controllers/metrics.go b/controllers/metrics.go index 22a45e99b..d4b25465e 100644 --- a/controllers/metrics.go +++ b/controllers/metrics.go @@ -73,6 +73,16 @@ var ( "dspa_namespace", }, ) + EnvoyReadyMetric = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Name: "data_science_pipelines_application_envoy_ready", + Help: "Data Science Pipelines Application - Envoy Ready Status", + }, + []string{ + "dspa_name", + "dspa_namespace", + }, + ) CrReadyMetric = prometheus.NewGaugeVec( prometheus.GaugeOpts{ Name: "data_science_pipelines_application_ready", @@ -92,5 +102,6 @@ func InitMetrics() { APIServerReadyMetric, PersistenceAgentReadyMetric, ScheduledWorkflowReadyMetric, + EnvoyReadyMetric, CrReadyMetric) } diff --git a/controllers/mlmd.go b/controllers/mlmd.go index c9f0f3bc6..bc348a8e5 100644 --- a/controllers/mlmd.go +++ b/controllers/mlmd.go @@ -16,7 +16,13 @@ limitations under the License. package controllers import ( + "context" + "fmt" + dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" + v1 "github.com/openshift/api/route/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" ) const ( @@ -72,3 +78,47 @@ func (r *DSPAReconciler) ReconcileMLMD(dsp *dspav1alpha1.DataSciencePipelinesApp log.Info("Finished applying MLMD Resources") return nil } + +func (r *DSPAReconciler) GetEnvoyServiceHostname(ctx context.Context, dsp *dspav1alpha1.DataSciencePipelinesApplication) (string, error) { + service := &corev1.Service{} + namespacedNamed := types.NamespacedName{Name: "ds-pipeline-md-" + dsp.Name, Namespace: dsp.Namespace} + err := r.Get(ctx, namespacedNamed, service) + if err != nil { + return "", err + } + + // Loop over all Service ports, if a secured port is found + // set port and scheme to its secured ones and skip the loop + serviceScheme := "" + servicePort := "" + for i := 0; i < len(service.Spec.Ports); i++ { + servicePort = fmt.Sprintf("%d", service.Spec.Ports[i].Port) + if servicePort == "8443" || servicePort == "443" { + // If a secured port is found, just set scheme to 'https://' and skip the loop + serviceScheme = "https://" + break + } else { + serviceScheme = "http://" + } + } + + return serviceScheme + service.Name + "." + service.Namespace + ".svc.cluster.local:" + servicePort, nil +} + +func (r *DSPAReconciler) GetEnvoyRouteHostname(ctx context.Context, dsp *dspav1alpha1.DataSciencePipelinesApplication) (string, error) { + route := &v1.Route{} + namespacedNamed := types.NamespacedName{Name: "ds-pipeline-md-" + dsp.Name, Namespace: dsp.Namespace} + err := r.Get(ctx, namespacedNamed, route) + if err != nil { + return "", err + } + + serviceScheme := "" + if route.Spec.TLS != nil { + serviceScheme = "https://" + } else { + serviceScheme = "http://" + } + + return serviceScheme + route.Spec.Host, nil +} diff --git a/controllers/mlmd_test.go b/controllers/mlmd_test.go index 529256745..d993b714d 100644 --- a/controllers/mlmd_test.go +++ b/controllers/mlmd_test.go @@ -18,9 +18,10 @@ limitations under the License. package controllers import ( - v1 "github.com/openshift/api/route/v1" "testing" + v1 "github.com/openshift/api/route/v1" + dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" @@ -819,4 +820,76 @@ func TestDontDeployEnvoyRouteV2(t *testing.T) { func boolPtr(b bool) *bool { return &b + +func TestGetEndpointsMLMDV2(t *testing.T) { + testNamespace := "testnamespace" + testDSPAName := "testdspa" + expectedMLMDEnvoyName := "ds-pipeline-metadata-envoy-testdspa" + expectedMLMDEnvoyRouteName := "ds-pipeline-md-testdspa" + + // Construct DSPA Spec with MLMD Enabled + dspa := &dspav1alpha1.DataSciencePipelinesApplication{ + Spec: dspav1alpha1.DSPASpec{ + DSPVersion: "v2", + APIServer: &dspav1alpha1.APIServer{}, + MLMD: &dspav1alpha1.MLMD{ + Deploy: true, + }, + Database: &dspav1alpha1.Database{ + DisableHealthCheck: false, + MariaDB: &dspav1alpha1.MariaDB{ + Deploy: true, + }, + }, + ObjectStorage: &dspav1alpha1.ObjectStorage{ + DisableHealthCheck: false, + Minio: &dspav1alpha1.Minio{ + Deploy: false, + Image: "someimage", + }, + }, + }, + } + + // Enrich DSPA with name+namespace + dspa.Namespace = testNamespace + dspa.Name = testDSPAName + + // Create Context, Fake Controller and Params + ctx, params, reconciler := CreateNewTestObjects() + err := params.ExtractParams(ctx, dspa, reconciler.Client, reconciler.Log) + assert.Nil(t, err) + + // Ensure MLMD-Envoy resources doesn't yet exist + deployment := &appsv1.Deployment{} + created, err := reconciler.IsResourceCreated(ctx, deployment, expectedMLMDEnvoyName, testNamespace) + assert.False(t, created) + assert.Nil(t, err) + + // Ensure MLMD-Envoy route doesn't yet exist + route := &v1.Route{} + created, err = reconciler.IsResourceCreated(ctx, route, expectedMLMDEnvoyRouteName, testNamespace) + assert.False(t, created) + assert.Nil(t, err) + + // Run test reconciliation + err = reconciler.ReconcileMLMD(dspa, params) + assert.Nil(t, err) + + // Ensure MLMD-Envoy resources now exists + deployment = &appsv1.Deployment{} + created, err = reconciler.IsResourceCreated(ctx, deployment, expectedMLMDEnvoyName, testNamespace) + assert.True(t, created) + assert.Nil(t, err) + + // Ensure MLMD-Envoy route now exists + route = &v1.Route{} + created, err = reconciler.IsResourceCreated(ctx, route, expectedMLMDEnvoyRouteName, testNamespace) + assert.True(t, created) + assert.Nil(t, err) + + dspa_created := &dspav1alpha1.DataSciencePipelinesApplication{} + created, err = reconciler.IsResourceCreated(ctx, dspa, testDSPAName, testNamespace) + assert.NotNil(t, dspa_created.Status.Components.Envoy.Url) + assert.NotNil(t, dspa_created.Status.Components.Envoy.ExternalUrl) } From 0cd495775f372cbf7c71eb717cfc6f73ef8b551a Mon Sep 17 00:00:00 2001 From: "Ricardo M. Oliveira" Date: Wed, 10 Jul 2024 18:49:34 -0300 Subject: [PATCH 09/18] Test debug DSP conditions Signed-off-by: Ricardo M. Oliveira --- tests/resources/dspa-lite.yaml | 1 + tests/suite_test.go | 23 +++++++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/resources/dspa-lite.yaml b/tests/resources/dspa-lite.yaml index 225eef9d3..48c512066 100644 --- a/tests/resources/dspa-lite.yaml +++ b/tests/resources/dspa-lite.yaml @@ -40,6 +40,7 @@ spec: mlmd: deploy: true envoy: + deployRoute: true resources: limits: cpu: 20m diff --git a/tests/suite_test.go b/tests/suite_test.go index 512fac361..248068438 100644 --- a/tests/suite_test.go +++ b/tests/suite_test.go @@ -26,15 +26,18 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/anthhub/forwarder" "github.com/go-logr/logr" mfc "github.com/manifestival/controller-runtime-client" mf "github.com/manifestival/manifestival" + "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" testUtil "github.com/opendatahub-io/data-science-pipelines-operator/tests/util" "github.com/stretchr/testify/suite" "go.uber.org/zap/zapcore" + "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -167,12 +170,12 @@ func (suite *IntegrationTestSuite) SetupSuite() { if !skipDeploy { loggr.Info("Deploying DSPA...") err = testUtil.DeployDSPA(suite.T(), ctx, clientmgr.k8sClient, DSPA, DSPANamespace, DeployTimeout, PollInterval) - assert.NoError(suite.T(), err) + require.NoError(suite.T(), err) loggr.Info("Waiting for DSPA pods to ready...") } err = testUtil.WaitForDSPAReady(suite.T(), ctx, clientmgr.k8sClient, DSPA.Name, DSPANamespace, DeployTimeout, PollInterval) - assert.NoError(suite.T(), err) + require.NoError(suite.T(), err, fmt.Sprintf("Error Deploying DSPA:\n%s", printConditions(ctx, DSPA, DSPANamespace, clientmgr.k8sClient))) loggr.Info("DSPA Deployed.") loggr.Info("Setting up Portforwarding service.") @@ -194,6 +197,22 @@ func (suite *IntegrationTestSuite) SetupSuite() { loggr.Info("Portforwarding service Successfully set up.") } +func printConditions(ctx context.Context, dspa *v1alpha1.DataSciencePipelinesApplication, namespace string, client client.Client) string { + nsn := types.NamespacedName{ + Name: dspa.Name, + Namespace: namespace, + } + err := client.Get(ctx, nsn, dspa) + if err != nil { + return "No conditions" + } + conditions := "" + for _, condition := range dspa.Status.Conditions { + conditions = conditions + fmt.Sprintf("Type: %s, Status: %s, Message: %s\n", condition.Type, condition.Status, condition.Message) + } + return conditions +} + func (suite *IntegrationTestSuite) TearDownSuite() { if !skipCleanup { err := testUtil.DeleteDSPA(suite.T(), ctx, clientmgr.k8sClient, DSPA.Name, DSPANamespace, DeployTimeout, PollInterval) From 5d45ff1243a8eff7e89fad24dcd63379e651aceb Mon Sep 17 00:00:00 2001 From: "Ricardo M. Oliveira" Date: Thu, 11 Jul 2024 15:59:26 -0300 Subject: [PATCH 10/18] Make components field from status optional --- api/v1alpha1/dspipeline_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1alpha1/dspipeline_types.go b/api/v1alpha1/dspipeline_types.go index eb98fd4b5..52af0a1c0 100644 --- a/api/v1alpha1/dspipeline_types.go +++ b/api/v1alpha1/dspipeline_types.go @@ -419,8 +419,8 @@ type ComponentStatus struct { } type ComponentDetailStatus struct { + Url string `json:"url,omitempty"` // +kubebuilder:validation:Optional - Url string `json:"url,omitempty"` ExternalUrl string `json:"externalUrl,omitempty"` } From bd5fce35194ddc5a219fcd0db3663ec3e617e45f Mon Sep 17 00:00:00 2001 From: "Ricardo M. Oliveira" Date: Thu, 11 Jul 2024 16:40:08 -0300 Subject: [PATCH 11/18] Add step to collect DSPO logs in case of test failures Signed-off-by: Ricardo M. Oliveira --- .github/workflows/kind-integration.yml | 125 ++++++++++++++++++++++++- 1 file changed, 124 insertions(+), 1 deletion(-) diff --git a/.github/workflows/kind-integration.yml b/.github/workflows/kind-integration.yml index 7d3571013..a1786b534 100644 --- a/.github/workflows/kind-integration.yml +++ b/.github/workflows/kind-integration.yml @@ -48,4 +48,127 @@ jobs: - name: Run test working-directory: ${{ github.workspace }}/.github/scripts/tests run: | - sh kind-integration.sh + kubectl apply -f ${{ env.RESOURCES_DIR }}/crds + kubectl apply -f ${{ env.CONFIG_DIR }}/crd/external/route.openshift.io_routes.yaml + + - name: Build image + env: + DSPO_IMAGE: ${{env.REGISTRY_ADDRESS}}/data-science-pipelines-operator + run: | + make podman-build -e IMG="${DSPO_IMAGE}" + + - name: Create opendatahub namespace + run: | + kubectl create namespace opendatahub + + - name: Deploy Argo Lite + working-directory: ${{ github.workspace }}/.github/resources/argo-lite + run: | + kustomize build . | kubectl apply -f - + + - name: Deploy DSPO + env: + DSPO_IMAGE: ${{env.REGISTRY_ADDRESS}}/data-science-pipelines-operator + run: | + make podman-push -e IMG="${DSPO_IMAGE}" + make deploy-kind -e IMG="${DSPO_IMAGE}" + + - name: Create Minio Namespace + run: | + kubectl create namespace ${{ env.MINIO_NAMESPACE }} + + - name: Deploy Minio + working-directory: ${{ github.workspace }}/.github/resources/minio + run: | + kustomize build . | oc -n ${{ env.MINIO_NAMESPACE }} apply -f - + + - name: Create MariaDB Namespace + run: | + kubectl create namespace ${{ env.MARIADB_NAMESPACE }} + + - name: Deploy MariaDB + working-directory: ${{ github.workspace }}/.github/resources/mariadb + run: | + kustomize build . | oc -n ${{ env.MARIADB_NAMESPACE }} apply -f - + + - name: Create Pypiserver Namespace + run: | + kubectl create namespace ${{ env.PYPISERVER_NAMESPACE }} + + - name: Deploy pypi-server + working-directory: ${{ github.workspace }}/.github/resources/pypiserver/base + run: | + kustomize build . | oc -n ${{ env.PYPISERVER_NAMESPACE }} apply -f - + + - name: Wait for Dependencies (DSPO, Minio, Mariadb, Pypi server) + run: | + kubectl wait -n opendatahub --timeout=60s --for=condition=Available=true deployment data-science-pipelines-operator-controller-manager + kubectl wait -n ${{ env.MARIADB_NAMESPACE }} --timeout=60s --for=condition=Available=true deployment mariadb + kubectl wait -n ${{ env.MINIO_NAMESPACE }} --timeout=60s --for=condition=Available=true deployment minio + kubectl wait -n ${{ env.PYPISERVER_NAMESPACE }} --timeout=60s --for=condition=Available=true deployment pypi-server + + - name: Upload Python Packages to pypi-server + working-directory: ${{ github.workspace }}/.github/scripts/python_package_upload + run: | + sh package_upload.sh + + - name: Create DSPA Namespace + run: | + kubectl create namespace ${{ env.DSPA_NAMESPACE }} + + - name: Create Namespace for DSPA with External connections + run: | + kubectl create namespace ${{ env.DSPA_EXTERNAL_NAMESPACE }} + + - name: Apply MariaDB and Minio Secrets and Configmaps in the External Namespace + working-directory: ${{ github.workspace }}/.github/resources/external-pre-reqs + run: | + kustomize build . | oc -n ${{ env.DSPA_EXTERNAL_NAMESPACE }} apply -f - + + - name: Apply PIP Server ConfigMap + env: + RESOURCES_DIR: ${{ github.workspace }}/.github/resources/pypiserver/base + run: | + kubectl apply -f $RESOURCES_DIR/nginx-tls-config.yaml -n ${{ env.DSPA_NAMESPACE }} + + - name: Run tests + working-directory: ${{ github.workspace }} + env: + NAMESPACE: ${{ env.DSPA_NAMESPACE }} + DSPA_NAME: ${{ env.DSPA_NAME }} + DSPA_PATH: ${{ env.DSPA_PATH }} + run: | + make integrationtest K8SAPISERVERHOST=$(oc whoami --show-server) DSPANAMESPACE=${NAMESPACE} DSPAPATH=${DSPA_PATH} + + - name: Run tests for DSPA with External Connections + working-directory: ${{ github.workspace }} + env: + NAMESPACE: ${{ env.DSPA_EXTERNAL_NAMESPACE }} + DSPA_NAME: ${{ env.DSPA_EXTERNAL_NAME }} + DSPA_EXTERNAL_PATH: ${{ env.DSPA_EXTERNAL_PATH }} + run: | + make integrationtest K8SAPISERVERHOST=$(oc whoami --show-server) DSPANAMESPACE=${NAMESPACE} DSPAPATH=${DSPA_EXTERNAL_PATH} + + - name: Collect DSPO Logs if any tests failed + if: failure() + run: | + tmp_dir=$(mktemp -d) + DSPO=$(kubectl get pod -n opendatahub --no-headers -o custom-columns=NAME:.metadata.name) + echo "DSPO Pod: $DSPO" + kubectl logs -n opendatahub $DSPO > $tmp_dir/$DSPO.log + kubectl get -n $DSPA_NAMESPACE events > $tmp_dir/events.log + kubectl get -n $DSPA_NAMESPACE all > $tmp_dir/resources.log + kubectl get -n $DSPA_NAMESPACE sa > $tmp_dir/serviceaccounts.log + kubectl get -n $DSPA_NAMESPACE secret > $tmp_dir/secrets.log + kubectl describe -n $DSPA_NAMESPACE deployment ds-pipeline-metadata-envoy-$DSPA_NAME > $tmp_dir/envoy.log + + - name: Collect test results + if: failure() + uses: actions/upload-artifact@v4 + with: + name: integration-test-artifacts + path: /tmp/tmp.*/* + + - name: Clean up + run: | + make undeploy-kind From 6f28be9771577deb0cc0cc7c12a4ca690635f900 Mon Sep 17 00:00:00 2001 From: "Ricardo M. Oliveira" Date: Thu, 11 Jul 2024 20:48:21 -0300 Subject: [PATCH 12/18] Fix issue with Envoy Deployment when optionally enabling route Signed-off-by: Ricardo M. Oliveira --- .github/workflows/kind-integration.yml | 125 +----------------- api/v1alpha1/dspipeline_types.go | 2 +- api/v1alpha1/zz_generated.deepcopy.go | 2 +- ...b.io_datasciencepipelinesapplications.yaml | 2 +- .../metadata-envoy.deployment.yaml.tmpl | 2 + .../v2/dspa-all-fields/dspa_all_fields.yaml | 12 +- controllers/apiserver.go | 45 ------- controllers/config/defaults.go | 2 +- controllers/dspastatus/dspa_status.go | 20 +-- controllers/dspipeline_controller.go | 56 +++++--- controllers/dspipeline_params.go | 2 + controllers/metrics.go | 8 +- controllers/mlmd.go | 55 +------- controllers/mlmd_test.go | 35 +++-- controllers/util/util.go | 80 ++++++++++- tests/resources/dspa-external-lite.yaml | 1 + tests/resources/dspa-lite.yaml | 2 +- tests/suite_test.go | 20 +-- tests/util/resources.go | 16 +++ 19 files changed, 185 insertions(+), 302 deletions(-) diff --git a/.github/workflows/kind-integration.yml b/.github/workflows/kind-integration.yml index a1786b534..7d3571013 100644 --- a/.github/workflows/kind-integration.yml +++ b/.github/workflows/kind-integration.yml @@ -48,127 +48,4 @@ jobs: - name: Run test working-directory: ${{ github.workspace }}/.github/scripts/tests run: | - kubectl apply -f ${{ env.RESOURCES_DIR }}/crds - kubectl apply -f ${{ env.CONFIG_DIR }}/crd/external/route.openshift.io_routes.yaml - - - name: Build image - env: - DSPO_IMAGE: ${{env.REGISTRY_ADDRESS}}/data-science-pipelines-operator - run: | - make podman-build -e IMG="${DSPO_IMAGE}" - - - name: Create opendatahub namespace - run: | - kubectl create namespace opendatahub - - - name: Deploy Argo Lite - working-directory: ${{ github.workspace }}/.github/resources/argo-lite - run: | - kustomize build . | kubectl apply -f - - - - name: Deploy DSPO - env: - DSPO_IMAGE: ${{env.REGISTRY_ADDRESS}}/data-science-pipelines-operator - run: | - make podman-push -e IMG="${DSPO_IMAGE}" - make deploy-kind -e IMG="${DSPO_IMAGE}" - - - name: Create Minio Namespace - run: | - kubectl create namespace ${{ env.MINIO_NAMESPACE }} - - - name: Deploy Minio - working-directory: ${{ github.workspace }}/.github/resources/minio - run: | - kustomize build . | oc -n ${{ env.MINIO_NAMESPACE }} apply -f - - - - name: Create MariaDB Namespace - run: | - kubectl create namespace ${{ env.MARIADB_NAMESPACE }} - - - name: Deploy MariaDB - working-directory: ${{ github.workspace }}/.github/resources/mariadb - run: | - kustomize build . | oc -n ${{ env.MARIADB_NAMESPACE }} apply -f - - - - name: Create Pypiserver Namespace - run: | - kubectl create namespace ${{ env.PYPISERVER_NAMESPACE }} - - - name: Deploy pypi-server - working-directory: ${{ github.workspace }}/.github/resources/pypiserver/base - run: | - kustomize build . | oc -n ${{ env.PYPISERVER_NAMESPACE }} apply -f - - - - name: Wait for Dependencies (DSPO, Minio, Mariadb, Pypi server) - run: | - kubectl wait -n opendatahub --timeout=60s --for=condition=Available=true deployment data-science-pipelines-operator-controller-manager - kubectl wait -n ${{ env.MARIADB_NAMESPACE }} --timeout=60s --for=condition=Available=true deployment mariadb - kubectl wait -n ${{ env.MINIO_NAMESPACE }} --timeout=60s --for=condition=Available=true deployment minio - kubectl wait -n ${{ env.PYPISERVER_NAMESPACE }} --timeout=60s --for=condition=Available=true deployment pypi-server - - - name: Upload Python Packages to pypi-server - working-directory: ${{ github.workspace }}/.github/scripts/python_package_upload - run: | - sh package_upload.sh - - - name: Create DSPA Namespace - run: | - kubectl create namespace ${{ env.DSPA_NAMESPACE }} - - - name: Create Namespace for DSPA with External connections - run: | - kubectl create namespace ${{ env.DSPA_EXTERNAL_NAMESPACE }} - - - name: Apply MariaDB and Minio Secrets and Configmaps in the External Namespace - working-directory: ${{ github.workspace }}/.github/resources/external-pre-reqs - run: | - kustomize build . | oc -n ${{ env.DSPA_EXTERNAL_NAMESPACE }} apply -f - - - - name: Apply PIP Server ConfigMap - env: - RESOURCES_DIR: ${{ github.workspace }}/.github/resources/pypiserver/base - run: | - kubectl apply -f $RESOURCES_DIR/nginx-tls-config.yaml -n ${{ env.DSPA_NAMESPACE }} - - - name: Run tests - working-directory: ${{ github.workspace }} - env: - NAMESPACE: ${{ env.DSPA_NAMESPACE }} - DSPA_NAME: ${{ env.DSPA_NAME }} - DSPA_PATH: ${{ env.DSPA_PATH }} - run: | - make integrationtest K8SAPISERVERHOST=$(oc whoami --show-server) DSPANAMESPACE=${NAMESPACE} DSPAPATH=${DSPA_PATH} - - - name: Run tests for DSPA with External Connections - working-directory: ${{ github.workspace }} - env: - NAMESPACE: ${{ env.DSPA_EXTERNAL_NAMESPACE }} - DSPA_NAME: ${{ env.DSPA_EXTERNAL_NAME }} - DSPA_EXTERNAL_PATH: ${{ env.DSPA_EXTERNAL_PATH }} - run: | - make integrationtest K8SAPISERVERHOST=$(oc whoami --show-server) DSPANAMESPACE=${NAMESPACE} DSPAPATH=${DSPA_EXTERNAL_PATH} - - - name: Collect DSPO Logs if any tests failed - if: failure() - run: | - tmp_dir=$(mktemp -d) - DSPO=$(kubectl get pod -n opendatahub --no-headers -o custom-columns=NAME:.metadata.name) - echo "DSPO Pod: $DSPO" - kubectl logs -n opendatahub $DSPO > $tmp_dir/$DSPO.log - kubectl get -n $DSPA_NAMESPACE events > $tmp_dir/events.log - kubectl get -n $DSPA_NAMESPACE all > $tmp_dir/resources.log - kubectl get -n $DSPA_NAMESPACE sa > $tmp_dir/serviceaccounts.log - kubectl get -n $DSPA_NAMESPACE secret > $tmp_dir/secrets.log - kubectl describe -n $DSPA_NAMESPACE deployment ds-pipeline-metadata-envoy-$DSPA_NAME > $tmp_dir/envoy.log - - - name: Collect test results - if: failure() - uses: actions/upload-artifact@v4 - with: - name: integration-test-artifacts - path: /tmp/tmp.*/* - - - name: Clean up - run: | - make undeploy-kind + sh kind-integration.sh diff --git a/api/v1alpha1/dspipeline_types.go b/api/v1alpha1/dspipeline_types.go index 52af0a1c0..657e0a2ad 100644 --- a/api/v1alpha1/dspipeline_types.go +++ b/api/v1alpha1/dspipeline_types.go @@ -414,7 +414,7 @@ type DSPAStatus struct { type ComponentStatus struct { // +kubebuilder:validation:Optional - Envoy ComponentDetailStatus `json:"envoy,omitempty"` + MLMDProxy ComponentDetailStatus `json:"mlmdProxy,omitempty"` APIServer ComponentDetailStatus `json:"apiServer,omitempty"` } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 6deeb3b07..ac5fa0ceb 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -99,7 +99,7 @@ func (in *ComponentDetailStatus) DeepCopy() *ComponentDetailStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ComponentStatus) DeepCopyInto(out *ComponentStatus) { *out = *in - out.Envoy = in.Envoy + out.MLMDProxy = in.MLMDProxy out.APIServer = in.APIServer } diff --git a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml index 2100f40c4..ac888ab93 100644 --- a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml +++ b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml @@ -895,7 +895,7 @@ spec: url: type: string type: object - envoy: + mlmdProxy: properties: externalUrl: type: string diff --git a/config/internal/ml-metadata/metadata-envoy.deployment.yaml.tmpl b/config/internal/ml-metadata/metadata-envoy.deployment.yaml.tmpl index 1831f79c7..edff20d94 100644 --- a/config/internal/ml-metadata/metadata-envoy.deployment.yaml.tmpl +++ b/config/internal/ml-metadata/metadata-envoy.deployment.yaml.tmpl @@ -71,6 +71,7 @@ spec: - mountPath: /etc/envoy.yaml name: envoy-config subPath: envoy.yaml + {{ if .MLMD.Envoy.DeployRoute }} - name: oauth-proxy args: - --https-address=:8443 @@ -118,6 +119,7 @@ spec: volumeMounts: - mountPath: /etc/tls/private name: proxy-tls + {{ end }} serviceAccountName: ds-pipeline-metadata-envoy-{{.Name}} volumes: - name: envoy-config diff --git a/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml b/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml index b411bb975..87d538d70 100644 --- a/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml +++ b/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml @@ -185,12 +185,12 @@ spec: # example status fields status: components: - envoy: - url: http://envoy.svc.cluster.local - externalUrl: https://envoy-dspa.example.com + mlmdProxy: + url: http://mlmd-proxy.svc.cluster.local + externalUrl: https://mlmd-proxy-dspa.example.com apiServer: - url: http://envoy.svc.cluster.local - externalUrl: https://envoy-dspa.example.com + url: http://apiserver.svc.cluster.local + externalUrl: https://apiserver-dspa.example.com conditions: - lastTransitionTime: '2024-03-14T22:04:25Z' message: Database connectivity successfully verified @@ -227,7 +227,7 @@ status: observedGeneration: 3 reason: MinimumReplicasAvailable status: 'True' - type: EnvoyReady + type: MLMDProxyReady - lastTransitionTime: '2024-03-14T22:06:37Z' message: All components are ready. observedGeneration: 3 diff --git a/controllers/apiserver.go b/controllers/apiserver.go index 7c5f38f68..33196d9ed 100644 --- a/controllers/apiserver.go +++ b/controllers/apiserver.go @@ -17,7 +17,6 @@ package controllers import ( "context" - "fmt" dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" v1 "github.com/openshift/api/route/v1" @@ -87,47 +86,3 @@ func (r *DSPAReconciler) ReconcileAPIServer(ctx context.Context, dsp *dspav1alph log.Info("Finished applying APIServer Resources") return nil } - -func (r *DSPAReconciler) GetAPIServerServiceHostname(ctx context.Context, dsp *dspav1alpha1.DataSciencePipelinesApplication) (string, error) { - service := &corev1.Service{} - namespacedNamed := types.NamespacedName{Name: "ds-pipeline-" + dsp.Name, Namespace: dsp.Namespace} - err := r.Get(ctx, namespacedNamed, service) - if err != nil { - return "", err - } - - // Loop over all Service ports, if a secured port is found - // set port and scheme to its secured ones and skip the loop - serviceScheme := "" - servicePort := "" - for i := 0; i < len(service.Spec.Ports); i++ { - servicePort = fmt.Sprintf("%d", service.Spec.Ports[i].Port) - if servicePort == "8443" || servicePort == "443" { - // If a secured port is found, just set scheme to 'https://' and skip the loop - serviceScheme = "https://" - break - } else { - serviceScheme = "http://" - } - } - - return serviceScheme + service.Name + "." + service.Namespace + ".svc.cluster.local:" + servicePort, nil -} - -func (r *DSPAReconciler) GetAPIServerRouteHostname(ctx context.Context, dsp *dspav1alpha1.DataSciencePipelinesApplication) (string, error) { - route := &v1.Route{} - namespacedNamed := types.NamespacedName{Name: "ds-pipeline-" + dsp.Name, Namespace: dsp.Namespace} - err := r.Get(ctx, namespacedNamed, route) - if err != nil { - return "", err - } - - serviceScheme := "" - if route.Spec.TLS != nil { - serviceScheme = "https://" - } else { - serviceScheme = "http://" - } - - return serviceScheme + route.Spec.Host, nil -} diff --git a/controllers/config/defaults.go b/controllers/config/defaults.go index 22fa53954..b3b0c2b3d 100644 --- a/controllers/config/defaults.go +++ b/controllers/config/defaults.go @@ -139,7 +139,7 @@ const ( APIServerReady = "APIServerReady" PersistenceAgentReady = "PersistenceAgentReady" ScheduledWorkflowReady = "ScheduledWorkflowReady" - EnvoyReady = "EnvoyReady" + MLMDProxyReady = "MLMDProxyReady" CrReady = "Ready" ) diff --git a/controllers/dspastatus/dspa_status.go b/controllers/dspastatus/dspa_status.go index 04cc74b68..f7da09781 100644 --- a/controllers/dspastatus/dspa_status.go +++ b/controllers/dspastatus/dspa_status.go @@ -21,7 +21,7 @@ type DSPAStatus interface { SetScheduledWorkflowStatus(scheduledWorkflowReady metav1.Condition) - SetEnvoyStatus(envoyReady metav1.Condition) + SetMLMDProxyStatus(mlmdProxyReady metav1.Condition) GetConditions() []metav1.Condition } @@ -32,7 +32,7 @@ func NewDSPAStatus(dspa *dspav1alpha1.DataSciencePipelinesApplication) DSPAStatu apiServerCondition := BuildUnknownCondition(config.APIServerReady) persistenceAgentCondition := BuildUnknownCondition(config.PersistenceAgentReady) scheduledWorkflowReadyCondition := BuildUnknownCondition(config.ScheduledWorkflowReady) - envoyReadyCondition := BuildUnknownCondition(config.EnvoyReady) + mlmdProxyReadyCondition := BuildUnknownCondition(config.MLMDProxyReady) return &dspaStatus{ dspa: dspa, @@ -41,7 +41,7 @@ func NewDSPAStatus(dspa *dspav1alpha1.DataSciencePipelinesApplication) DSPAStatu apiServerReady: &apiServerCondition, persistenceAgentReady: &persistenceAgentCondition, scheduledWorkflowReady: &scheduledWorkflowReadyCondition, - envoyReady: &envoyReadyCondition, + mlmdProxyReady: &mlmdProxyReadyCondition, } } @@ -52,7 +52,7 @@ type dspaStatus struct { apiServerReady *metav1.Condition persistenceAgentReady *metav1.Condition scheduledWorkflowReady *metav1.Condition - envoyReady *metav1.Condition + mlmdProxyReady *metav1.Condition } func (s *dspaStatus) SetDatabaseNotReady(err error, reason string) { @@ -96,8 +96,8 @@ func (s *dspaStatus) SetScheduledWorkflowStatus(scheduledWorkflowReady metav1.Co s.scheduledWorkflowReady = &scheduledWorkflowReady } -func (s *dspaStatus) SetEnvoyStatus(envoyReady metav1.Condition) { - s.envoyReady = &envoyReady +func (s *dspaStatus) SetMLMDProxyStatus(mlmdProxyReady metav1.Condition) { + s.mlmdProxyReady = &mlmdProxyReady } func (s *dspaStatus) GetConditions() []metav1.Condition { @@ -107,7 +107,7 @@ func (s *dspaStatus) GetConditions() []metav1.Condition { *s.getApiServerReadyCondition(), *s.getPersistenceAgentReadyCondition(), *s.getScheduledWorkflowReadyCondition(), - *s.getEnvoyReadyCondition(), + *s.getMLMDProxyReadyCondition(), } allReady := true @@ -145,7 +145,7 @@ func (s *dspaStatus) GetConditions() []metav1.Condition { *s.apiServerReady, *s.persistenceAgentReady, *s.scheduledWorkflowReady, - *s.envoyReady, + *s.mlmdProxyReady, crReady, } @@ -179,8 +179,8 @@ func (s *dspaStatus) getScheduledWorkflowReadyCondition() *metav1.Condition { return s.scheduledWorkflowReady } -func (s *dspaStatus) getEnvoyReadyCondition() *metav1.Condition { - return s.envoyReady +func (s *dspaStatus) getMLMDProxyReadyCondition() *metav1.Condition { + return s.mlmdProxyReady } func BuildTrueCondition(conditionType string, message string) metav1.Condition { diff --git a/controllers/dspipeline_controller.go b/controllers/dspipeline_controller.go index 528b43295..12e444e1d 100644 --- a/controllers/dspipeline_controller.go +++ b/controllers/dspipeline_controller.go @@ -306,10 +306,11 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. err = r.ReconcileMLMD(dspa, params) if err != nil { - r.setStatusAsNotReady(config.EnvoyReady, err, dspaStatus.SetEnvoyStatus) + r.setStatusAsNotReady(config.MLMDProxyReady, err, dspaStatus.SetMLMDProxyStatus) return ctrl.Result{}, err } else { - r.setStatus(ctx, "ds-pipeline-metadata-envoy-"+dspa.Name, config.EnvoyReady, dspa, dspaStatus.SetEnvoyStatus, log) + r.setStatus(ctx, params.MlmdProxyDefaultResourceName, config.MLMDProxyReady, dspa, + dspaStatus.SetMLMDProxyStatus, log) } err = r.ReconcileWorkflowController(dspa, params) @@ -329,7 +330,7 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. util.GetConditionByType(config.APIServerReady, conditions): APIServerReadyMetric, util.GetConditionByType(config.PersistenceAgentReady, conditions): PersistenceAgentReadyMetric, util.GetConditionByType(config.ScheduledWorkflowReady, conditions): ScheduledWorkflowReadyMetric, - util.GetConditionByType(config.EnvoyReady, conditions): EnvoyReadyMetric, + util.GetConditionByType(config.MLMDProxyReady, conditions): MLMDProxyReadyMetric, util.GetConditionByType(config.CrReady, conditions): CrReadyMetric, } r.PublishMetrics(dspa, metricsMap) @@ -512,40 +513,53 @@ func (r *DSPAReconciler) GetComponents(ctx context.Context, dspa *dspav1alpha1.D log := r.Log.WithValues("namespace", dspa.Namespace).WithValues("dspa_name", dspa.Name) log.Info("Updating components endpoints") - envoyUrl, err := r.GetEnvoyServiceHostname(ctx, dspa) + mlmdProxyResourceName := fmt.Sprintf("ds-pipeline-md-%s", dspa.Name) + apiServerResourceName := fmt.Sprintf("ds-pipeline-%s", dspa.Name) + + mlmdProxyUrl, err := util.GetServiceHostname(ctx, mlmdProxyResourceName, dspa.Namespace, r.Client) if err != nil { - log.Info(err.Error()) + log.Error(err, "Error retrieving MLMD Proxy Service endpoint") } - envoyExternalUrl, err := r.GetEnvoyRouteHostname(ctx, dspa) + mlmdProxyExternalUrl, err := util.GetRouteHostname(ctx, mlmdProxyResourceName, dspa.Namespace, r.Client) if err != nil { - log.Info(err.Error()) + log.Error(err, "Error retrieving MLMD Proxy Route endpoint") } - envoyComponent := &dspav1alpha1.ComponentDetailStatus{ - Url: envoyUrl, - ExternalUrl: envoyExternalUrl, + apiServerUrl, err := util.GetServiceHostname(ctx, apiServerResourceName, dspa.Namespace, r.Client) + if err != nil { + log.Error(err, "Error retrieving API Server Service endpoint") } - apiServerUrl, err := r.GetAPIServerServiceHostname(ctx, dspa) + apiServerExternalUrl, err := util.GetRouteHostname(ctx, apiServerResourceName, dspa.Namespace, r.Client) if err != nil { - log.Info(err.Error()) + log.Error(err, "Error retrieving API Server Route endpoint") } - apiServerExternalUrl, err := r.GetAPIServerRouteHostname(ctx, dspa) - if err != nil { - log.Info(err.Error()) + mlmdProxyComponent := &dspav1alpha1.ComponentDetailStatus{} + if mlmdProxyUrl != "" { + mlmdProxyComponent.Url = mlmdProxyUrl + } + if mlmdProxyExternalUrl != "" { + mlmdProxyComponent.ExternalUrl = mlmdProxyExternalUrl } - apiServerComponent := &dspav1alpha1.ComponentDetailStatus{ - Url: apiServerUrl, - ExternalUrl: apiServerExternalUrl, + apiServerComponent := &dspav1alpha1.ComponentDetailStatus{} + if apiServerUrl != "" { + apiServerComponent.Url = apiServerUrl + } + if apiServerExternalUrl != "" { + apiServerComponent.ExternalUrl = apiServerExternalUrl } - return dspav1alpha1.ComponentStatus{ - Envoy: *envoyComponent, - APIServer: *apiServerComponent, + status := dspav1alpha1.ComponentStatus{} + if mlmdProxyComponent.Url != "" && mlmdProxyComponent.ExternalUrl != "" { + status.MLMDProxy = *mlmdProxyComponent + } + if apiServerComponent.Url != "" && apiServerComponent.ExternalUrl != "" { + status.APIServer = *apiServerComponent } + return status } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/dspipeline_params.go b/controllers/dspipeline_params.go index aa5364ef4..6a65b2fee 100644 --- a/controllers/dspipeline_params.go +++ b/controllers/dspipeline_params.go @@ -65,6 +65,7 @@ type DSPAParams struct { MariaDB *dspa.MariaDB Minio *dspa.Minio MLMD *dspa.MLMD + MlmdProxyDefaultResourceName string WorkflowController *dspa.WorkflowController CustomKfpLauncherConfigMapData string DBConnection @@ -593,6 +594,7 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip p.Minio = dsp.Spec.ObjectStorage.Minio.DeepCopy() p.OAuthProxy = config.GetStringConfigWithDefault(config.OAuthProxyImagePath, config.DefaultImageValue) p.MLMD = dsp.Spec.MLMD.DeepCopy() + p.MlmdProxyDefaultResourceName = mlmdProxyDefaultResourceNamePrefix + dsp.Name p.CustomCABundleRootMountPath = config.CustomCABundleRootMountPath p.PiplinesCABundleMountPath = config.GetCABundleFileMountPath() p.PodToPodTLS = false diff --git a/controllers/metrics.go b/controllers/metrics.go index d4b25465e..82647f9d5 100644 --- a/controllers/metrics.go +++ b/controllers/metrics.go @@ -73,10 +73,10 @@ var ( "dspa_namespace", }, ) - EnvoyReadyMetric = prometheus.NewGaugeVec( + MLMDProxyReadyMetric = prometheus.NewGaugeVec( prometheus.GaugeOpts{ - Name: "data_science_pipelines_application_envoy_ready", - Help: "Data Science Pipelines Application - Envoy Ready Status", + Name: "data_science_pipelines_application_mlmdproxy_ready", + Help: "Data Science Pipelines Application - MLMD Proxy Ready Status", }, []string{ "dspa_name", @@ -102,6 +102,6 @@ func InitMetrics() { APIServerReadyMetric, PersistenceAgentReadyMetric, ScheduledWorkflowReadyMetric, - EnvoyReadyMetric, + MLMDProxyReadyMetric, CrReadyMetric) } diff --git a/controllers/mlmd.go b/controllers/mlmd.go index bc348a8e5..bd3f3f10c 100644 --- a/controllers/mlmd.go +++ b/controllers/mlmd.go @@ -16,18 +16,13 @@ limitations under the License. package controllers import ( - "context" - "fmt" - dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" - v1 "github.com/openshift/api/route/v1" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/types" ) const ( - mlmdTemplatesDir = "ml-metadata" - mlmdEnvoyRoute = mlmdTemplatesDir + "/route/metadata-envoy.route.yaml.tmpl" + mlmdTemplatesDir = "ml-metadata" + mlmdEnvoyRoute = mlmdTemplatesDir + "/route/metadata-envoy.route.yaml.tmpl" + mlmdProxyDefaultResourceNamePrefix = "ds-pipeline-scheduledworkflow-" ) func (r *DSPAReconciler) ReconcileMLMD(dsp *dspav1alpha1.DataSciencePipelinesApplication, @@ -78,47 +73,3 @@ func (r *DSPAReconciler) ReconcileMLMD(dsp *dspav1alpha1.DataSciencePipelinesApp log.Info("Finished applying MLMD Resources") return nil } - -func (r *DSPAReconciler) GetEnvoyServiceHostname(ctx context.Context, dsp *dspav1alpha1.DataSciencePipelinesApplication) (string, error) { - service := &corev1.Service{} - namespacedNamed := types.NamespacedName{Name: "ds-pipeline-md-" + dsp.Name, Namespace: dsp.Namespace} - err := r.Get(ctx, namespacedNamed, service) - if err != nil { - return "", err - } - - // Loop over all Service ports, if a secured port is found - // set port and scheme to its secured ones and skip the loop - serviceScheme := "" - servicePort := "" - for i := 0; i < len(service.Spec.Ports); i++ { - servicePort = fmt.Sprintf("%d", service.Spec.Ports[i].Port) - if servicePort == "8443" || servicePort == "443" { - // If a secured port is found, just set scheme to 'https://' and skip the loop - serviceScheme = "https://" - break - } else { - serviceScheme = "http://" - } - } - - return serviceScheme + service.Name + "." + service.Namespace + ".svc.cluster.local:" + servicePort, nil -} - -func (r *DSPAReconciler) GetEnvoyRouteHostname(ctx context.Context, dsp *dspav1alpha1.DataSciencePipelinesApplication) (string, error) { - route := &v1.Route{} - namespacedNamed := types.NamespacedName{Name: "ds-pipeline-md-" + dsp.Name, Namespace: dsp.Namespace} - err := r.Get(ctx, namespacedNamed, route) - if err != nil { - return "", err - } - - serviceScheme := "" - if route.Spec.TLS != nil { - serviceScheme = "https://" - } else { - serviceScheme = "http://" - } - - return serviceScheme + route.Spec.Host, nil -} diff --git a/controllers/mlmd_test.go b/controllers/mlmd_test.go index d993b714d..0fa941da9 100644 --- a/controllers/mlmd_test.go +++ b/controllers/mlmd_test.go @@ -24,6 +24,7 @@ import ( dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" ) @@ -820,6 +821,7 @@ func TestDontDeployEnvoyRouteV2(t *testing.T) { func boolPtr(b bool) *bool { return &b +} func TestGetEndpointsMLMDV2(t *testing.T) { testNamespace := "testnamespace" @@ -830,10 +832,15 @@ func TestGetEndpointsMLMDV2(t *testing.T) { // Construct DSPA Spec with MLMD Enabled dspa := &dspav1alpha1.DataSciencePipelinesApplication{ Spec: dspav1alpha1.DSPASpec{ - DSPVersion: "v2", - APIServer: &dspav1alpha1.APIServer{}, + DSPVersion: "v2", + PodToPodTLS: boolPtr(false), + APIServer: &dspav1alpha1.APIServer{}, MLMD: &dspav1alpha1.MLMD{ Deploy: true, + Envoy: &dspav1alpha1.Envoy{ + Image: "someimage", + DeployRoute: true, + }, }, Database: &dspav1alpha1.Database{ DisableHealthCheck: false, @@ -858,38 +865,38 @@ func TestGetEndpointsMLMDV2(t *testing.T) { // Create Context, Fake Controller and Params ctx, params, reconciler := CreateNewTestObjects() err := params.ExtractParams(ctx, dspa, reconciler.Client, reconciler.Log) - assert.Nil(t, err) + require.Nil(t, err) // Ensure MLMD-Envoy resources doesn't yet exist deployment := &appsv1.Deployment{} created, err := reconciler.IsResourceCreated(ctx, deployment, expectedMLMDEnvoyName, testNamespace) - assert.False(t, created) - assert.Nil(t, err) + require.False(t, created) + require.Nil(t, err) // Ensure MLMD-Envoy route doesn't yet exist route := &v1.Route{} created, err = reconciler.IsResourceCreated(ctx, route, expectedMLMDEnvoyRouteName, testNamespace) - assert.False(t, created) - assert.Nil(t, err) + require.False(t, created) + require.Nil(t, err) // Run test reconciliation err = reconciler.ReconcileMLMD(dspa, params) - assert.Nil(t, err) + require.Nil(t, err) // Ensure MLMD-Envoy resources now exists deployment = &appsv1.Deployment{} created, err = reconciler.IsResourceCreated(ctx, deployment, expectedMLMDEnvoyName, testNamespace) - assert.True(t, created) - assert.Nil(t, err) + require.True(t, created) + require.Nil(t, err) // Ensure MLMD-Envoy route now exists route = &v1.Route{} created, err = reconciler.IsResourceCreated(ctx, route, expectedMLMDEnvoyRouteName, testNamespace) - assert.True(t, created) - assert.Nil(t, err) + require.True(t, created) + require.Nil(t, err) dspa_created := &dspav1alpha1.DataSciencePipelinesApplication{} created, err = reconciler.IsResourceCreated(ctx, dspa, testDSPAName, testNamespace) - assert.NotNil(t, dspa_created.Status.Components.Envoy.Url) - assert.NotNil(t, dspa_created.Status.Components.Envoy.ExternalUrl) + require.NotNil(t, dspa_created.Status.Components.MLMDProxy.Url) + require.NotNil(t, dspa_created.Status.Components.MLMDProxy.ExternalUrl) } diff --git a/controllers/util/util.go b/controllers/util/util.go index 5bb269450..f0f767b1e 100644 --- a/controllers/util/util.go +++ b/controllers/util/util.go @@ -17,17 +17,22 @@ limitations under the License. package util import ( - "github.com/opendatahub-io/data-science-pipelines-operator/controllers/config" + "fmt" "os" "path/filepath" + "github.com/opendatahub-io/data-science-pipelines-operator/controllers/config" + "context" "crypto/x509" + "net/url" + + routev1 "github.com/openshift/api/route/v1" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "net/url" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -122,3 +127,74 @@ func GetSystemCerts() ([]byte, error) { } return data, err } + +func GetServiceHostname(ctx context.Context, svcName, ns string, client client.Client) (string, error) { + serviceHostname := "" + isAvailable, service, err := GetServiceIfAvailable(ctx, svcName, ns, client) + if err != nil { + return "", err + } + + if isAvailable { + // Loop over all Service ports, if a secured port is found + // set port and scheme to its secured ones and exit the loop + servicePort := "" + scheme := "http" + for _, port := range service.Spec.Ports { + servicePort = fmt.Sprintf("%d", port.Port) + if servicePort == "8443" || servicePort == "443" { + // If a secured port is found, just set scheme to 'https://' and exit the loop + scheme = "https" + break + } + } + serviceHostname = scheme + "://" + service.Name + "." + service.Namespace + ".svc.cluster.local:" + servicePort + } + return serviceHostname, nil +} + +func GetRouteHostname(ctx context.Context, routeName, ns string, client client.Client) (string, error) { + routeHostname := "" + isAvailable, route, err := GetRouteIfAvailable(ctx, routeName, ns, client) + if err != nil { + return "", err + } + + if isAvailable { + scheme := "http" + if route.Spec.TLS != nil { + scheme = "https" + } + + routeHostname = scheme + "://" + route.Spec.Host + } + return routeHostname, nil +} + +func GetServiceIfAvailable(ctx context.Context, svcName, ns string, client client.Client) (bool, *v1.Service, error) { + service := &v1.Service{} + namespacedNamed := types.NamespacedName{Name: svcName, Namespace: ns} + err := client.Get(ctx, namespacedNamed, service) + if err != nil { + if errors.IsNotFound(err) { + return false, nil, nil + } else { + return false, nil, err + } + } + return true, service, nil +} + +func GetRouteIfAvailable(ctx context.Context, routeName, ns string, client client.Client) (bool, *routev1.Route, error) { + route := &routev1.Route{} + namespacedNamed := types.NamespacedName{Name: routeName, Namespace: ns} + err := client.Get(ctx, namespacedNamed, route) + if err != nil { + if errors.IsNotFound(err) { + return false, nil, nil + } else { + return false, nil, err + } + } + return true, route, nil +} diff --git a/tests/resources/dspa-external-lite.yaml b/tests/resources/dspa-external-lite.yaml index e6ca8c717..0a0679514 100644 --- a/tests/resources/dspa-external-lite.yaml +++ b/tests/resources/dspa-external-lite.yaml @@ -40,6 +40,7 @@ spec: mlmd: deploy: true envoy: + image: quay.io/maistra/proxyv2-ubi8:2.5.0 resources: limits: cpu: 20m diff --git a/tests/resources/dspa-lite.yaml b/tests/resources/dspa-lite.yaml index 48c512066..483603caf 100644 --- a/tests/resources/dspa-lite.yaml +++ b/tests/resources/dspa-lite.yaml @@ -40,7 +40,7 @@ spec: mlmd: deploy: true envoy: - deployRoute: true + image: quay.io/maistra/proxyv2-ubi8:2.5.0 resources: limits: cpu: 20m diff --git a/tests/suite_test.go b/tests/suite_test.go index 248068438..d3168c190 100644 --- a/tests/suite_test.go +++ b/tests/suite_test.go @@ -32,12 +32,10 @@ import ( "github.com/go-logr/logr" mfc "github.com/manifestival/controller-runtime-client" mf "github.com/manifestival/manifestival" - "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" testUtil "github.com/opendatahub-io/data-science-pipelines-operator/tests/util" "github.com/stretchr/testify/suite" "go.uber.org/zap/zapcore" - "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -175,7 +173,7 @@ func (suite *IntegrationTestSuite) SetupSuite() { } err = testUtil.WaitForDSPAReady(suite.T(), ctx, clientmgr.k8sClient, DSPA.Name, DSPANamespace, DeployTimeout, PollInterval) - require.NoError(suite.T(), err, fmt.Sprintf("Error Deploying DSPA:\n%s", printConditions(ctx, DSPA, DSPANamespace, clientmgr.k8sClient))) + require.NoError(suite.T(), err, fmt.Sprintf("Error Deploying DSPA:\n%s", testUtil.PrintConditions(ctx, DSPA, DSPANamespace, clientmgr.k8sClient))) loggr.Info("DSPA Deployed.") loggr.Info("Setting up Portforwarding service.") @@ -197,22 +195,6 @@ func (suite *IntegrationTestSuite) SetupSuite() { loggr.Info("Portforwarding service Successfully set up.") } -func printConditions(ctx context.Context, dspa *v1alpha1.DataSciencePipelinesApplication, namespace string, client client.Client) string { - nsn := types.NamespacedName{ - Name: dspa.Name, - Namespace: namespace, - } - err := client.Get(ctx, nsn, dspa) - if err != nil { - return "No conditions" - } - conditions := "" - for _, condition := range dspa.Status.Conditions { - conditions = conditions + fmt.Sprintf("Type: %s, Status: %s, Message: %s\n", condition.Type, condition.Status, condition.Message) - } - return conditions -} - func (suite *IntegrationTestSuite) TearDownSuite() { if !skipCleanup { err := testUtil.DeleteDSPA(suite.T(), ctx, clientmgr.k8sClient, DSPA.Name, DSPANamespace, DeployTimeout, PollInterval) diff --git a/tests/util/resources.go b/tests/util/resources.go index 0620494be..d00327b8c 100644 --- a/tests/util/resources.go +++ b/tests/util/resources.go @@ -137,3 +137,19 @@ func waitFor(ctx context.Context, timeout, interval time.Duration, conditionFunc } return fmt.Errorf("timed out waiting for condition") } + +func PrintConditions(ctx context.Context, dspa *v1alpha1.DataSciencePipelinesApplication, namespace string, client client.Client) string { + nsn := types.NamespacedName{ + Name: dspa.Name, + Namespace: namespace, + } + err := client.Get(ctx, nsn, dspa) + if err != nil { + return "No conditions" + } + conditions := "" + for _, condition := range dspa.Status.Conditions { + conditions = conditions + fmt.Sprintf("Type: %s, Status: %s, Message: %s\n", condition.Type, condition.Status, condition.Message) + } + return conditions +} From 89b8aeee527d9eef5c2198479a765b8a79aebcb1 Mon Sep 17 00:00:00 2001 From: Humair Khan Date: Tue, 13 Aug 2024 12:54:46 -0400 Subject: [PATCH 13/18] add newlines between certs Signed-off-by: Humair Khan --- controllers/dspipeline_params.go | 2 +- .../declarative/case_6/deploy/00_configmap.yaml | 1 + .../expected/created/configmap_dspa_trusted_ca.yaml | 3 +++ .../expected/created/configmap_dspa_trusted_ca.yaml | 2 ++ controllers/testutil/equalities.go | 11 +++++++++++ 5 files changed, 18 insertions(+), 1 deletion(-) diff --git a/controllers/dspipeline_params.go b/controllers/dspipeline_params.go index 6a65b2fee..6b2c0b5e1 100644 --- a/controllers/dspipeline_params.go +++ b/controllers/dspipeline_params.go @@ -798,7 +798,7 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip }, Data: map[string]string{ - p.CustomCABundle.ConfigMapKey: string(bytes.Join(p.APICustomPemCerts, []byte{})), + p.CustomCABundle.ConfigMapKey: string(bytes.Join(p.APICustomPemCerts, []byte("\n"))), }, } diff --git a/controllers/testdata/declarative/case_6/deploy/00_configmap.yaml b/controllers/testdata/declarative/case_6/deploy/00_configmap.yaml index ea0a42cdd..7c1b22101 100644 --- a/controllers/testdata/declarative/case_6/deploy/00_configmap.yaml +++ b/controllers/testdata/declarative/case_6/deploy/00_configmap.yaml @@ -36,6 +36,7 @@ data: WBV3KJBsYK/wijtLeip1oKobU76oE0ML/bnhV10k6usvl4n8cDmcONo5FnGoT8Pk 80htx6w5fanMFu4MnoBeyJhhzNfg7ywJcc2VZSM27s2B -----END CERTIFICATE----- + odh-ca-bundle.crt: | -----BEGIN CERTIFICATE----- MIIFLTCCAxWgAwIBAgIUIvY4jV0212P/ddjuCZhcUyJfoocwDQYJKoZIhvcNAQEL diff --git a/controllers/testdata/declarative/case_6/expected/created/configmap_dspa_trusted_ca.yaml b/controllers/testdata/declarative/case_6/expected/created/configmap_dspa_trusted_ca.yaml index a286c123f..e662cb8b3 100644 --- a/controllers/testdata/declarative/case_6/expected/created/configmap_dspa_trusted_ca.yaml +++ b/controllers/testdata/declarative/case_6/expected/created/configmap_dspa_trusted_ca.yaml @@ -34,6 +34,7 @@ data: WBV3KJBsYK/wijtLeip1oKobU76oE0ML/bnhV10k6usvl4n8cDmcONo5FnGoT8Pk 80htx6w5fanMFu4MnoBeyJhhzNfg7ywJcc2VZSM27s2B -----END CERTIFICATE----- + -----BEGIN CERTIFICATE----- MIIFLTCCAxWgAwIBAgIUIvY4jV0212P/ddjuCZhcUyJfoocwDQYJKoZIhvcNAQEL BQAwJjELMAkGA1UEBhMCWFgxFzAVBgNVBAMMDnJoLWRzcC1kZXZzLmlvMB4XDTI0 @@ -64,6 +65,7 @@ data: WBV3KJBsYK/wijtLeip1oKobU76oE0ML/bnhV10k6usvl4n8cDmcONo5FnGoT8Pk 80htx6w5fanMFu4MnoBeyJhhzNfg7ywJcc2VZSM27s2B -----END CERTIFICATE----- + -----BEGIN CERTIFICATE----- MIIFlTCCA32gAwIBAgIUQTPwwkR17jDrdIe4VqhzNQ6OY1MwDQYJKoZIhvcNAQEL BQAwJjELMAkGA1UEBhMCWFgxFzAVBgNVBAMMDnJoLWRzcC1kZXZzLmlvMB4XDTI0 @@ -96,6 +98,7 @@ data: lsiMw+o9r32W0fzjQRwipTLNM0lEbgWyErsVXFb67vY/rjy9ybuFlKMMOIlZpmut wcr1vUGA985Lhv2jire2GTlixOiqZtuQS08lGa7kkcO8sB+7MdRdgEI= -----END CERTIFICATE----- + -----BEGIN CERTIFICATE----- MIIFLTCCAxWgAwIBAgIUIvY4jV0212P/ddjuCZhcUyJfoocwDQYJKoZIhvcNAQEL BQAwJjELMAkGA1UEBhMCWFgxFzAVBgNVBAMMDnJoLWRzcC1kZXZzLmlvMB4XDTI0 diff --git a/controllers/testdata/declarative/case_8/expected/created/configmap_dspa_trusted_ca.yaml b/controllers/testdata/declarative/case_8/expected/created/configmap_dspa_trusted_ca.yaml index 49524cc9f..567b05bfa 100644 --- a/controllers/testdata/declarative/case_8/expected/created/configmap_dspa_trusted_ca.yaml +++ b/controllers/testdata/declarative/case_8/expected/created/configmap_dspa_trusted_ca.yaml @@ -34,6 +34,7 @@ data: WBV3KJBsYK/wijtLeip1oKobU76oE0ML/bnhV10k6usvl4n8cDmcONo5FnGoT8Pk 80htx6w5fanMFu4MnoBeyJhhzNfg7ywJcc2VZSM27s2B -----END CERTIFICATE----- + -----BEGIN CERTIFICATE----- MIIFLTCCAxWgAwIBAgIUIvY4jV0212P/ddjuCZhcUyJfoocwDQYJKoZIhvcNAQEL BQAwJjELMAkGA1UEBhMCWFgxFzAVBgNVBAMMDnJoLWRzcC1kZXZzLmlvMB4XDTI0 @@ -64,6 +65,7 @@ data: WBV3KJBsYK/wijtLeip1oKobU76oE0ML/bnhV10k6usvl4n8cDmcONo5FnGoT8Pk 80htx6w5fanMFu4MnoBeyJhhzNfg7ywJcc2VZSM27s2B -----END CERTIFICATE----- + -----BEGIN CERTIFICATE----- MIIFLTCCAxWgAwIBAgIUIvY4jV0212P/ddjuCZhcUyJfoocwDQYJKoZIhvcNAQEL BQAwJjELMAkGA1UEBhMCWFgxFzAVBgNVBAMMDnJoLWRzcC1kZXZzLmlvMB4XDTI0 diff --git a/controllers/testutil/equalities.go b/controllers/testutil/equalities.go index bed19bb32..336922e39 100644 --- a/controllers/testutil/equalities.go +++ b/controllers/testutil/equalities.go @@ -18,6 +18,7 @@ package testutil import ( "fmt" + "strings" "github.com/go-test/deep" @@ -52,6 +53,16 @@ func configMapsAreEqual(expected, actual *unstructured.Unstructured) (bool, erro return false, notEqualMsg("Configmap Names are not equal.") } + // Functional tests are very buggy when it comes to accounting for trailing white spaces and can be hard to + // diagnose, we trim these so we are only comparing the core contents, to account for whitespace testing + // defer to unit testing + for k := range expectedConfigMap.Data { + expectedConfigMap.Data[k] = strings.TrimSpace(expectedConfigMap.Data[k]) + } + for k := range actualConfigMap.Data { + actualConfigMap.Data[k] = strings.TrimSpace(actualConfigMap.Data[k]) + } + diff := deep.Equal(expectedConfigMap.Data, actualConfigMap.Data) if diff != nil { return false, notDeeplyEqualMsg("Configmap's Data values", diff) From d1cef16bb1431a4ddade21f93f3e2f792e73a483 Mon Sep 17 00:00:00 2001 From: vmudadla Date: Thu, 15 Aug 2024 08:34:17 -0500 Subject: [PATCH 14/18] Added MLMD GRPC Server Network Policy Signed-off-by: vmudadla --- .../metadata-grpc.networkpolicy.yaml.tmpl | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 config/internal/ml-metadata/metadata-grpc.networkpolicy.yaml.tmpl diff --git a/config/internal/ml-metadata/metadata-grpc.networkpolicy.yaml.tmpl b/config/internal/ml-metadata/metadata-grpc.networkpolicy.yaml.tmpl new file mode 100644 index 000000000..9885eb85a --- /dev/null +++ b/config/internal/ml-metadata/metadata-grpc.networkpolicy.yaml.tmpl @@ -0,0 +1,24 @@ +kind: NetworkPolicy +apiVersion: networking.k8s.io/v1 +metadata: + name: ds-pipeline-metadata-grpc-{{ .Name }} + namespace: {{ .Namespace }} +spec: + podSelector: + matchLabels: + app: ds-pipeline-metadata-grpc-{{ .Name }} + component: data-science-pipelines + ingress: + - ports: + - protocol: TCP + port: 8080 + from: + - podSelector: + matchLabels: + pipelines.kubeflow.org/v2_component: 'true' + - podSelector: + matchLabels: + app: ds-pipeline-metadata-envoy-{{.Name}} + component: data-science-pipelines + policyTypes: + - Ingress From 67fd52986faf381182dc6fb8148cae2345034109 Mon Sep 17 00:00:00 2001 From: Humair Khan Date: Fri, 16 Aug 2024 15:09:11 -0400 Subject: [PATCH 15/18] fix default envoy route behavior Signed-off-by: Humair Khan --- controllers/dspipeline_params.go | 6 +++++- tests/resources/dspa-external-lite.yaml | 1 + tests/resources/dspa-lite.yaml | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/controllers/dspipeline_params.go b/controllers/dspipeline_params.go index 6b2c0b5e1..2e45c8932 100644 --- a/controllers/dspipeline_params.go +++ b/controllers/dspipeline_params.go @@ -504,6 +504,9 @@ func (p *DSPAParams) SetupMLMD(dsp *dspa.DataSciencePipelinesApplication, log lo log.Info("MLMD not specified, but is a required component for V2 Pipelines. Including MLMD with default specs.") p.MLMD = &dspa.MLMD{ Deploy: true, + Envoy: &dspa.Envoy{ + DeployRoute: true, + }, } } else if !p.MLMD.Deploy { return fmt.Errorf(MlmdIsRequiredInV2Msg) @@ -515,7 +518,8 @@ func (p *DSPAParams) SetupMLMD(dsp *dspa.DataSciencePipelinesApplication, log lo if p.MLMD.Envoy == nil { p.MLMD.Envoy = &dspa.Envoy{ - Image: config.GetStringConfigWithDefault(MlmdEnvoyImagePath, config.DefaultImageValue), + Image: config.GetStringConfigWithDefault(MlmdEnvoyImagePath, config.DefaultImageValue), + DeployRoute: true, } } if p.MLMD.GRPC == nil { diff --git a/tests/resources/dspa-external-lite.yaml b/tests/resources/dspa-external-lite.yaml index 0a0679514..6b0645119 100644 --- a/tests/resources/dspa-external-lite.yaml +++ b/tests/resources/dspa-external-lite.yaml @@ -41,6 +41,7 @@ spec: deploy: true envoy: image: quay.io/maistra/proxyv2-ubi8:2.5.0 + deployRoute: false resources: limits: cpu: 20m diff --git a/tests/resources/dspa-lite.yaml b/tests/resources/dspa-lite.yaml index 483603caf..f09e480da 100644 --- a/tests/resources/dspa-lite.yaml +++ b/tests/resources/dspa-lite.yaml @@ -41,6 +41,7 @@ spec: deploy: true envoy: image: quay.io/maistra/proxyv2-ubi8:2.5.0 + deployRoute: false resources: limits: cpu: 20m From aa95bfa7965335ee67e1618b39684d3316dc10b0 Mon Sep 17 00:00:00 2001 From: Humair Khan Date: Fri, 16 Aug 2024 15:43:53 -0400 Subject: [PATCH 16/18] make mlmd grpc NP more permissive Signed-off-by: Humair Khan --- .../internal/ml-metadata/metadata-grpc.networkpolicy.yaml.tmpl | 1 - 1 file changed, 1 deletion(-) diff --git a/config/internal/ml-metadata/metadata-grpc.networkpolicy.yaml.tmpl b/config/internal/ml-metadata/metadata-grpc.networkpolicy.yaml.tmpl index 9885eb85a..d5439dcef 100644 --- a/config/internal/ml-metadata/metadata-grpc.networkpolicy.yaml.tmpl +++ b/config/internal/ml-metadata/metadata-grpc.networkpolicy.yaml.tmpl @@ -18,7 +18,6 @@ spec: pipelines.kubeflow.org/v2_component: 'true' - podSelector: matchLabels: - app: ds-pipeline-metadata-envoy-{{.Name}} component: data-science-pipelines policyTypes: - Ingress From 2cef55778d31c19426da1e83b9dd6fa251ab0394 Mon Sep 17 00:00:00 2001 From: Humair Khan Date: Mon, 19 Aug 2024 13:49:10 -0400 Subject: [PATCH 17/18] update compatibility doc for 2.5 Signed-off-by: Humair Khan --- docs/release/compatibility.md | 1 + docs/release/compatibility.yaml | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/docs/release/compatibility.md b/docs/release/compatibility.md index e7e2bcb95..61eece9fb 100644 --- a/docs/release/compatibility.md +++ b/docs/release/compatibility.md @@ -6,6 +6,7 @@ Each row outlines the versions for individual subcomponents and images that are | dsp | kfp | argo | ml-metadata | envoy | ocp-pipelines | oauth-proxy | mariadb-103 | ubi-minimal | ubi-micro | openshift | |-----|-----|-----|-----|-----|-----|-----|-----|-----|-----|-----| +| 2.5 | 2.0.5 | 3.3.10 | 1.14.0 | 1.22.11 | N/A | v4.10 | 1 | N/A | N/A | 4.14,4.15,4.16 | | 2.4 | 2.0.5 | 3.3.10 | 1.14.0 | 1.22.11 | N/A | v4.10 | 1 | N/A | N/A | 4.14,4.15,4.16 | | 2.3 | 2.0.5 | 3.3.10 | 1.14.0 | 1.22.11 | N/A | v4.10 | 1 | N/A | N/A | 4.13,4.14,4.15 | | 2.2 | 2.0.5 | 3.3.10 | 1.14.0 | 1.22.11 | N/A | v4.10 | 1 | N/A | N/A | 4.13,4.14,4.15 | diff --git a/docs/release/compatibility.yaml b/docs/release/compatibility.yaml index e2ab069e4..b4974ba77 100644 --- a/docs/release/compatibility.yaml +++ b/docs/release/compatibility.yaml @@ -1,3 +1,14 @@ +- dsp: 2.5 + kfp: 2.0.5 + argo: 3.3.10 + ml-metadata: 1.14.0 + envoy: 1.22.11 + ocp-pipelines: "N/A" + oauth-proxy: v4.10 + mariadb-103: 1 + ubi-minimal: "N/A" + ubi-micro: "N/A" + openshift: 4.14,4.15,4.16 - dsp: 2.4 kfp: 2.0.5 argo: 3.3.10 From 092b2543ac8fa3d8a985358d57bd3fcd5ed7885b Mon Sep 17 00:00:00 2001 From: dsp-developers <140449482+dsp-developers@users.noreply.github.com> Date: Mon, 19 Aug 2024 18:03:41 +0000 Subject: [PATCH 18/18] Generate params for 2.5 --- config/base/params.env | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/config/base/params.env b/config/base/params.env index f861cb30e..eabbbf4ed 100644 --- a/config/base/params.env +++ b/config/base/params.env @@ -1,19 +1,19 @@ -IMAGES_APISERVER=quay.io/opendatahub/ds-pipelines-api-server:v1.6.3 -IMAGES_ARTIFACT=quay.io/opendatahub/ds-pipelines-artifact-manager:v1.6.3 -IMAGES_PERSISTENTAGENT=quay.io/opendatahub/ds-pipelines-persistenceagent:v1.6.3 -IMAGES_SCHEDULEDWORKFLOW=quay.io/opendatahub/ds-pipelines-scheduledworkflow:v1.6.3 -IMAGES_MLMDENVOY=quay.io/opendatahub/ds-pipelines-metadata-envoy:v1.6.3 -IMAGES_MLMDGRPC=quay.io/opendatahub/ds-pipelines-metadata-grpc:v1.6.3 -IMAGES_MLMDWRITER=quay.io/opendatahub/ds-pipelines-metadata-writer:v1.6.3 -IMAGES_DSPO=quay.io/opendatahub/data-science-pipelines-operator:latest -V2_LAUNCHER_IMAGE=quay.io/opendatahub/ds-pipelines-launcher:latest -V2_DRIVER_IMAGE=quay.io/opendatahub/ds-pipelines-driver:latest -IMAGESV2_ARGO_APISERVER=quay.io/opendatahub/ds-pipelines-api-server:latest -IMAGESV2_ARGO_PERSISTENCEAGENT=quay.io/opendatahub/ds-pipelines-persistenceagent:latest -IMAGESV2_ARGO_SCHEDULEDWORKFLOW=quay.io/opendatahub/ds-pipelines-scheduledworkflow:latest -IMAGESV2_ARGO_WORKFLOWCONTROLLER=quay.io/opendatahub/ds-pipelines-argo-workflowcontroller:3.3.10-upstream -IMAGESV2_ARGO_ARGOEXEC=quay.io/opendatahub/ds-pipelines-argo-argoexec:3.3.10-upstream -IMAGESV2_ARGO_MLMDGRPC=quay.io/opendatahub/mlmd-grpc-server:latest +IMAGES_APISERVER=quay.io/opendatahub/ds-pipelines-api-server@sha256:caca0638eef179e61c97878cc4adc1a39473b3c67f46c808175560cf824fac82 +IMAGES_ARTIFACT=quay.io/opendatahub/ds-pipelines-artifact-manager@sha256:1d322d5f72fe5b4599cb88810aa8b86da6af7859b395400543e24696cdfc3948 +IMAGES_PERSISTENTAGENT=quay.io/opendatahub/ds-pipelines-persistenceagent@sha256:02bca29b5e5bcde17d18d991d0893a1aef357dbd175729698fc246ec72ee967d +IMAGES_SCHEDULEDWORKFLOW=quay.io/opendatahub/ds-pipelines-scheduledworkflow@sha256:0c09b453151cf037b2055a8e94ec36716207da43cf99a9faa19dfc52ac8cb67f +IMAGES_MLMDENVOY=quay.io/opendatahub/ds-pipelines-metadata-envoy@sha256:c491e63c8885c7d59005f9305b77cd1fa776b50e63db90c4f8ccdee963759630 +IMAGES_MLMDGRPC=quay.io/opendatahub/ds-pipelines-metadata-grpc@sha256:4af88c246d77cce33099489090508734978aafa83a0a5745408ae8d139d5378a +IMAGES_MLMDWRITER=quay.io/opendatahub/ds-pipelines-metadata-writer@sha256:393ddb21afe13166e972022153479450bb66b2c9b1896e62a484aed2ac62b388 +IMAGES_DSPO=quay.io/opendatahub/data-science-pipelines-operator@sha256:d8dde8938509ad0679ff4c245324cb5d5b3d01075891b39f8069b8e585020ed1 +V2_LAUNCHER_IMAGE=quay.io/opendatahub/ds-pipelines-launcher@sha256:1a6b6328d30036ffd399960b84db4a306522f92f6ef8e8d2a0f88f112d401a7d +V2_DRIVER_IMAGE=quay.io/opendatahub/ds-pipelines-driver@sha256:ea1ceae99e7a4768da104076915b5271e88fd541e4f804aafee8798798db991d +IMAGESV2_ARGO_APISERVER=quay.io/opendatahub/ds-pipelines-api-server@sha256:4182a598d8235ef3ea40d48b6bf66705fa97daf9f2a11a457c98d80514cd69c0 +IMAGESV2_ARGO_PERSISTENCEAGENT=quay.io/opendatahub/ds-pipelines-persistenceagent@sha256:95aa99338610abdbaddddc88f02e8ce55fb61317fd3c90f92cf9c30b8000eb90 +IMAGESV2_ARGO_SCHEDULEDWORKFLOW=quay.io/opendatahub/ds-pipelines-scheduledworkflow@sha256:e4fd9dfff1622ff125a645970496d25af28a14062019a34eed9b13bb04eca183 +IMAGESV2_ARGO_WORKFLOWCONTROLLER=quay.io/opendatahub/ds-pipelines-argo-workflowcontroller@sha256:4a2ccfc397ae6f3470df09eaace4d568d27378085466a38e68a2b56981c3e5f9 +IMAGESV2_ARGO_ARGOEXEC=quay.io/opendatahub/ds-pipelines-argo-argoexec@sha256:b2b3bc54744d2780c32f1aa564361a1ae4a42532c6d16662e45ad1025acff1ea +IMAGESV2_ARGO_MLMDGRPC=quay.io/opendatahub/mlmd-grpc-server@sha256:9e905b2de2fb6801716a14ebd6e589cac82fef26741825d06717d695a37ff199 IMAGESV2_ARGO_MLMDENVOY=registry.redhat.io/openshift-service-mesh/proxyv2-rhel8@sha256:a744c1b386fd5e4f94e43543e829df1bfdd1b564137917372a11da06872f4bcb IMAGES_MARIADB=registry.redhat.io/rhel8/mariadb-103@sha256:3d30992e60774f887c4e7959c81b0c41b0d82d042250b3b56f05ab67fd4cdee1 IMAGES_OAUTHPROXY=registry.redhat.io/openshift4/ose-oauth-proxy@sha256:ab112105ac37352a2a4916a39d6736f5db6ab4c29bad4467de8d613e80e9bb33