Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/actions/deploy/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ inputs:
argo_version:
required: false
description: "Argo version to use for the cluster"
db_type:
description: "The type of database to deploy for testing (mysql or pgx)."
required: false
default: ""
forward_port:
required: false
default: 'true'
Expand Down Expand Up @@ -180,6 +184,12 @@ runs:
if [ "${{inputs.pod_to_pod_tls_enabled }}" = "true" ]; then
ARGS="${ARGS} --tls-enabled"
fi

if [ -n "${{ inputs.db_type }}" ]; then
echo "Deploying with database type ${{ inputs.db_type }}"
ARGS="${ARGS} --db-type ${{ inputs.db_type }}"
fi

echo "ARGS=$ARGS" >> "$GITHUB_OUTPUT"

- name: Deploy KFP
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: ml-pipeline
spec:
template:
spec:
containers:
- name: ml-pipeline-api-server
env:
- name: V2_DRIVER_IMAGE
value: kind-registry:5000/driver:ci
- name: V2_LAUNCHER_IMAGE
value: kind-registry:5000/launcher:ci
- name: LOG_LEVEL
value: "debug"
112 changes: 112 additions & 0 deletions .github/resources/manifests/multiuser/postgresql/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

# CI overlay for Multi-user + PostgreSQL testing
# This CI overlay does three things:
# 1. It uses `platform-agnostic-multi-user-postgresql` as its base
# 2. It applies CI-specific environment variables
# 3. It overrides image names to use locally built images from Kind registry
resources:
- ../../base
- ../../../../../manifests/kustomize/env/platform-agnostic-multi-user-postgresql

# The Kind PostgreSQL instance does not use TLS. Configure that deliberate CI
# choice through the same ConfigMap input consumed by both database clients.
configMapGenerator:
- name: pipeline-install-config
behavior: merge
literals:
- postgresExtraParams={"sslmode":"disable"}

images:
- name: ghcr.io/kubeflow/kfp-api-server
newName: kind-registry:5000/apiserver
newTag: latest
- name: ghcr.io/kubeflow/kfp-persistence-agent
newName: kind-registry:5000/persistenceagent
newTag: latest
- name: ghcr.io/kubeflow/kfp-scheduled-workflow-controller
newName: kind-registry:5000/scheduledworkflow
newTag: latest
- name: ghcr.io/kubeflow/kfp-frontend
newName: kind-registry:5000/frontend
newTag: latest
- name: ghcr.io/kubeflow/kfp-metadata-writer
newName: kind-registry:5000/metadata-writer
newTag: latest
- name: ghcr.io/kubeflow/kfp-viewer-crd-controller
newName: kind-registry:5000/viewer-crd-controller
newTag: latest
- name: ghcr.io/kubeflow/kfp-visualization-server
newName: kind-registry:5000/visualization-server
newTag: latest
- name: ghcr.io/kubeflow/kfp-cache-deployer
newName: kind-registry:5000/cache-deployer
newTag: latest
- name: ghcr.io/kubeflow/kfp-cache-server
newName: kind-registry:5000/cache-server
newTag: latest
- name: ghcr.io/kubeflow/kfp-metadata-envoy
newName: kind-registry:5000/metadata-envoy
newTag: latest

patches:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This multi-user PostgreSQL CI overlay drops the pipeline-crd-rbac.yaml patch that the other three multi-user CI overlays apply.

  • Why it matters: multiuser/default, multiuser/artifact-proxy, and multiuser/cache-disabled all apply ../pipeline-crd-rbac.yaml (adds pipelines.kubeflow.org pipelines/pipelineversions verbs to the ml-pipeline ClusterRole). This overlay is deployed for the multi-user pgx lane, so omitting it is an RBAC inconsistency that could surface as permission errors on pipeline-CRD paths.
  • Please add the ../pipeline-crd-rbac.yaml patch here for parity.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have a concern about this. All four multi-user CI overlays deploy in database pipeline store mode (--pipelinesStoreKubernetes is never set), so the ml-pipeline ClusterRole should not need full CRUD permissions on pipelines.kubeflow.org CRDs — the default get/list/watch from the base manifest is sufficient for the webhook's needs.

I've opened #13919 to discuss whether we should remove pipeline-crd-rbac.yaml from the other three multi-user overlays as well, rather than adding it here.

- path: ../../base/apiserver-env.yaml
target:
kind: Deployment
name: ml-pipeline
- path: ../../base/ci-stability-tuning.yaml
- path: apiserver-env.yaml
target:
kind: Deployment
name: ml-pipeline
- path: ../../base/grpc-specs.yaml
target:
kind: Deployment
name: metadata-grpc-deployment
- path: ../../base/cache-specs.yaml
target:
kind: Deployment
name: cache-server
- path: ../../base/metadata-writer-pull-policy.yaml
target:
kind: Deployment
name: metadata-writer
- path: ../../base/viewer-crd-pull-policy.yaml
target:
kind: Deployment
name: ml-pipeline-viewer-crd
- path: ../../base/cache-deployer-pull-policy.yaml
target:
kind: Deployment
name: cache-deployer-deployment
- path: ../../base/cache-server-pull-policy.yaml
target:
kind: Deployment
name: cache-server
- path: ../../base/metadata-envoy-pull-policy.yaml
target:
kind: Deployment
name: metadata-envoy-deployment

replacements:
- source:
kind: ConfigMap
name: dns-config
fieldPath: data.namespaceDns
targets:
- select:
kind: Deployment
name: ml-pipeline
fieldPaths:
- spec.template.spec.dnsConfig.searches.[=NAMESPACE.svc.cluster.local]
- select:
kind: Deployment
name: metadata-grpc-deployment
fieldPaths:
- spec.template.spec.dnsConfig.searches.[=NAMESPACE.svc.cluster.local]
- select:
kind: Deployment
name: cache-server
fieldPaths:
- spec.template.spec.dnsConfig.searches.[=NAMESPACE.svc.cluster.local]
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: ml-pipeline
spec:
template:
spec:
containers:
- name: ml-pipeline-api-server
env:
- name: V2_DRIVER_IMAGE
value: kind-registry:5000/driver:ci
- name: V2_LAUNCHER_IMAGE
value: kind-registry:5000/launcher:ci
- name: LOG_LEVEL
value: "debug"
117 changes: 117 additions & 0 deletions .github/resources/manifests/standalone/postgresql/kustomization.yaml

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The PostgreSQL CI overlays (both standalone and multiuser) are missing the ../../base/ci-stability-tuning.yaml patch that all other CI overlays include. This patch increases probe timeouts and failure thresholds to prevent flaky CI failures under load. Without it, PostgreSQL CI jobs may fail intermittently when the API server takes longer to start.

Verified: standalone/default/kustomization.yaml includes - path: ../../base/ci-stability-tuning.yaml but standalone/postgresql/kustomization.yaml does not. Same for multiuser/postgresql.

Suggested fix: Add to both standalone/postgresql/kustomization.yaml and multiuser/postgresql/kustomization.yaml under patches::

  - path: ../../base/ci-stability-tuning.yaml

Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

# This CI overlay for PostgreSQL testing does three things:
# 1. It uses `platform-agnostic-postgresql` as its base. This is the project's
# standard way to deploy KFP with PostgreSQL, which correctly includes both
# the KFP core components and the third-party PostgreSQL instance, and
# patches the API server to use the 'pgx' driver.
# 2. It applies an additional patch (`apiserver-env.yaml`) to inject
# CI-specific environment variables, like the V2 image path. This aligns
# with the pattern used in other CI overlays like `minio`.
# 3. It overrides the image names to use the locally built images from the
# Kind registry, which is standard practice for all CI tests.
resources:
- ../../base
- ../../../../../manifests/kustomize/env/platform-agnostic-postgresql

# The Kind PostgreSQL instance does not use TLS. Configure that deliberate CI
# choice through the same ConfigMap input consumed by both database clients.
configMapGenerator:
- name: pipeline-install-config
behavior: merge
literals:
- postgresExtraParams={"sslmode":"disable"}

images:
- name: ghcr.io/kubeflow/kfp-api-server
newName: kind-registry:5000/apiserver
newTag: latest
- name: ghcr.io/kubeflow/kfp-persistence-agent
newName: kind-registry:5000/persistenceagent
newTag: latest
- name: ghcr.io/kubeflow/kfp-scheduled-workflow-controller
newName: kind-registry:5000/scheduledworkflow
newTag: latest
- name: ghcr.io/kubeflow/kfp-frontend
newName: kind-registry:5000/frontend
newTag: latest
- name: ghcr.io/kubeflow/kfp-metadata-writer
newName: kind-registry:5000/metadata-writer
newTag: latest
- name: ghcr.io/kubeflow/kfp-viewer-crd-controller
newName: kind-registry:5000/viewer-crd-controller
newTag: latest
- name: ghcr.io/kubeflow/kfp-visualization-server
newName: kind-registry:5000/visualization-server
newTag: latest
- name: ghcr.io/kubeflow/kfp-cache-deployer
newName: kind-registry:5000/cache-deployer
newTag: latest
- name: ghcr.io/kubeflow/kfp-cache-server
newName: kind-registry:5000/cache-server
newTag: latest
- name: ghcr.io/kubeflow/kfp-metadata-envoy
newName: kind-registry:5000/metadata-envoy
newTag: latest

patches:
Comment thread
HumairAK marked this conversation as resolved.
- path: ../../base/apiserver-env.yaml
target:
kind: Deployment
name: ml-pipeline
- path: ../../base/ci-stability-tuning.yaml
- path: apiserver-env.yaml
target:
kind: Deployment
name: ml-pipeline
- path: ../../base/grpc-specs.yaml
target:
kind: Deployment
name: metadata-grpc-deployment
- path: ../../base/cache-specs.yaml
target:
kind: Deployment
name: cache-server
- path: ../../base/metadata-writer-pull-policy.yaml
target:
kind: Deployment
name: metadata-writer
- path: ../../base/viewer-crd-pull-policy.yaml
target:
kind: Deployment
name: ml-pipeline-viewer-crd
- path: ../../base/cache-deployer-pull-policy.yaml
target:
kind: Deployment
name: cache-deployer-deployment
- path: ../../base/cache-server-pull-policy.yaml
target:
kind: Deployment
name: cache-server
- path: ../../base/metadata-envoy-pull-policy.yaml
target:
kind: Deployment
name: metadata-envoy-deployment

replacements:
- source:
kind: ConfigMap
name: dns-config
fieldPath: data.namespaceDns
targets:
- select:
kind: Deployment
name: ml-pipeline
fieldPaths:
- spec.template.spec.dnsConfig.searches.[=NAMESPACE.svc.cluster.local]
- select:
kind: Deployment
name: metadata-grpc-deployment
fieldPaths:
- spec.template.spec.dnsConfig.searches.[=NAMESPACE.svc.cluster.local]
- select:
kind: Deployment
name: cache-server
fieldPaths:
- spec.template.spec.dnsConfig.searches.[=NAMESPACE.svc.cluster.local]
49 changes: 44 additions & 5 deletions .github/resources/scripts/deploy-kfp.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ AWF_VERSION=""
POD_TO_POD_TLS_ENABLED=false
IAM_CONNTRACK_WINDOW_ACTIVE=false
IAM_CONNTRACK_BASELINE="/tmp/kfp-seaweedfs-iam-conntrack-window.tsv"
DB_TYPE=""

report_iam_conntrack_window() {
if [ "$IAM_CONNTRACK_WINDOW_ACTIVE" != "true" ]; then
Expand Down Expand Up @@ -86,6 +87,16 @@ while [ "$#" -gt 0 ]; do
POD_TO_POD_TLS_ENABLED=true
shift
;;
--db-type)
shift
if [[ -n "$1" ]]; then
DB_TYPE="$1"
shift
else
echo "ERROR: --db-type requires an argument"
exit 1
fi
;;
esac
done

Expand Down Expand Up @@ -126,6 +137,10 @@ if [ "${PIPELINES_STORE}" == "kubernetes" ] || [ "${POD_TO_POD_TLS_ENABLED}" ==
fi


# Pin kubeflow/manifests to a specific commit for reproducibility.
# To upgrade: verify all four paths below exist at the new SHA before updating.
KUBEFLOW_MANIFESTS_SHA="88716b3f7f62b12f98d82bcfc59635bb07e7845c"

# Deploy multi-user prerequisites if multi-user mode is enabled
# wait_for_pods_ready waits for pods to reach condition=Ready and, on timeout,
# dumps pod status, per-container readiness (including any istio-proxy sidecar),
Expand Down Expand Up @@ -172,9 +187,9 @@ wait_for_pods_ready() {

if [ "${MULTI_USER}" == "true" ]; then
echo "Installing Istio..."
kubectl apply -k https://github.com/kubeflow/manifests/common/istio/istio-crds/base?ref=master
kubectl apply -k https://github.com/kubeflow/manifests/common/istio/istio-namespace/base?ref=master
kubectl apply -k https://github.com/kubeflow/manifests/common/istio/istio-install/base?ref=master
kubectl apply -k "https://github.com/kubeflow/manifests/common/istio/istio-crds/base?ref=${KUBEFLOW_MANIFESTS_SHA}"
kubectl apply -k "https://github.com/kubeflow/manifests/common/istio/istio-namespace/base?ref=${KUBEFLOW_MANIFESTS_SHA}"
kubectl apply -k "https://github.com/kubeflow/manifests/common/istio/istio-install/base?ref=${KUBEFLOW_MANIFESTS_SHA}"
echo "Waiting for all Istio Pods to become ready..."
wait_for_pods_ready istio-system "" 300s "Istio pods"

Expand All @@ -183,13 +198,23 @@ if [ "${MULTI_USER}" == "true" ]; then
kubectl wait --for condition=established --timeout=30s crd/compositecontrollers.metacontroller.k8s.io

echo "Installing Profile Controller Resources..."
kubectl apply -k https://github.com/kubeflow/manifests/applications/dashboard/upstream/profile-controller/overlays/kubeflow?ref=master
kubectl apply -k "https://github.com/kubeflow/manifests/applications/dashboard/upstream/profile-controller/overlays/kubeflow?ref=${KUBEFLOW_MANIFESTS_SHA}"
echo "Profile controller applied; its readiness will be joined after the KFP rollout."
fi

# Manifests will be deployed according to the flag provided
if [ "${MULTI_USER}" == "false" ] && [ "${PIPELINES_STORE}" != "kubernetes" ]; then
TEST_MANIFESTS="${TEST_MANIFESTS}/standalone"

if $POD_TO_POD_TLS_ENABLED && [ "${DB_TYPE}" == "pgx" ]; then
echo "Error: POD_TO_POD_TLS_ENABLED and DB_TYPE=pgx cannot be used together (TLS+PostgreSQL overlay not yet implemented)." >&2
exit 1
fi
if [ "${DB_TYPE}" == "pgx" ] && { $CACHE_DISABLED || $USE_PROXY; }; then
echo "Error: DB_TYPE=pgx cannot be combined with CACHE_DISABLED or USE_PROXY." >&2
exit 1
fi

if $CACHE_DISABLED && $USE_PROXY; then
TEST_MANIFESTS="${TEST_MANIFESTS}/cache-disabled-proxy"
elif $CACHE_DISABLED; then
Expand All @@ -198,6 +223,8 @@ if [ "${MULTI_USER}" == "false" ] && [ "${PIPELINES_STORE}" != "kubernetes" ]; t
TEST_MANIFESTS="${TEST_MANIFESTS}/proxy"
elif $POD_TO_POD_TLS_ENABLED; then
TEST_MANIFESTS="${TEST_MANIFESTS}/tls-enabled"
elif [ "${DB_TYPE}" == "pgx" ]; then
TEST_MANIFESTS="${TEST_MANIFESTS}/postgresql"
else
TEST_MANIFESTS="${TEST_MANIFESTS}/default"
fi
Expand All @@ -210,7 +237,19 @@ elif [ "${MULTI_USER}" == "false" ] && [ "${PIPELINES_STORE}" == "kubernetes" ];
fi
elif [ "${MULTI_USER}" == "true" ]; then
TEST_MANIFESTS="${TEST_MANIFESTS}/multiuser"
if $ARTIFACT_PROXY_ENABLED; then

if [ "${DB_TYPE}" == "pgx" ] && $CACHE_DISABLED; then
echo "Error: DB_TYPE=pgx cannot be combined with CACHE_DISABLED in multi-user mode (no corresponding overlay exists)." >&2
exit 1
fi
if [ "${DB_TYPE}" == "pgx" ] && $ARTIFACT_PROXY_ENABLED; then
echo "Error: DB_TYPE=pgx cannot be combined with ARTIFACT_PROXY_ENABLED in multi-user mode (no corresponding overlay exists)." >&2
exit 1
fi

if [ "${DB_TYPE}" == "pgx" ]; then
TEST_MANIFESTS="${TEST_MANIFESTS}/postgresql"
elif $ARTIFACT_PROXY_ENABLED; then
TEST_MANIFESTS="${TEST_MANIFESTS}/artifact-proxy"
elif $CACHE_DISABLED; then
TEST_MANIFESTS="${TEST_MANIFESTS}/cache-disabled"
Expand Down
Loading
Loading