-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(backend): postgres integration #12379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
kaikaila
wants to merge
6
commits into
kubeflow:master
Choose a base branch
from
kaikaila:feature/postgres-integration
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+4,220
−2,366
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
740ccdc
refactor(backend): integrate postgresql
kaikaila efacb60
ci(pg): add PostgreSQL CI workflows and manifests
kaikaila 61a48ab
refactor(experiment_store): improve subquery code readability in Arch…
kaikaila cd66d09
Diff since Humair's Oct 31 review:
kaikaila b888433
add CI job for postgres with argo 3.6.7
kaikaila bafc918
database ”" to “mysql”
kaikaila File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
16 changes: 16 additions & 0 deletions
16
.github/resources/manifests/standalone/postgresql/apiserver-env.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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:latest | ||
| - name: V2_LAUNCHER_IMAGE | ||
| value: kind-registry:5000/launcher:latest | ||
| - name: LOG_LEVEL | ||
| value: "debug" |
26 changes: 26 additions & 0 deletions
26
.github/resources/manifests/standalone/postgresql/kustomization.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| 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: | ||
| - ../../../../../manifests/kustomize/env/platform-agnostic-postgresql | ||
|
|
||
| images: | ||
| - name: ghcr.io/kubeflow/kfp-api-server | ||
| newName: kind-registry:5000/apiserver | ||
| newTag: latest | ||
| - name: ghcr.io/kubeflow/kfp-cache-server | ||
| newName: kind-registry:5000/cache-server | ||
| newTag: latest | ||
|
|
||
| patches: | ||
| - path: apiserver-env.yaml |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| name: API Server Tests - Postgres | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - master | ||
| pull_request: | ||
| paths: | ||
| - "backend/**" | ||
| - "manifests/kustomize/third-party/postgresql/**" | ||
| - ".github/resources/manifests/standalone/**" | ||
| - ".github/workflows/kfp-backend-v2-postgres-tests.yml" | ||
| - "!**/*.md" | ||
| - "!**/OWNERS" | ||
| env: | ||
| NAMESPACE: kubeflow | ||
| POSTGRES_NAMESPACE: kubeflow | ||
| DB_TYPE: postgres | ||
| DB_DRIVER: pgx | ||
| DB_PORT: "5432" | ||
| # The IP address for port-forwarding the database. Go tests will connect to this IP. | ||
| # This should be kept in sync with other postgres test workflows and local test scripts. | ||
| # Using 127.0.0.1 to match MySQL workflow behavior and Kind local development setup. | ||
| DB_FORWARD_IP: 127.0.0.1 | ||
| DB_USER: user | ||
| DB_PASSWORD: password | ||
| DB_NAME: mlpipeline | ||
| jobs: | ||
| build: | ||
| uses: ./.github/workflows/image-builds-with-cache.yml | ||
| postgres-pgx: | ||
| runs-on: ubuntu-latest | ||
| needs: build | ||
| continue-on-error: false | ||
| strategy: | ||
| matrix: | ||
| cache_enabled: [true, false] | ||
| fail-fast: false # Ensure all jobs in the matrix run, even if one fails | ||
| name: KFP Backend V2 Postgres Tests (Cache ${{ matrix.cache_enabled }}) | ||
|
|
||
| steps: | ||
| - name: Checkout target code | ||
| uses: actions/checkout@v5 | ||
| - name: Set up Go | ||
| uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: "1.22" | ||
| cache: true | ||
|
|
||
| - name: Create KFP cluster | ||
| uses: ./.github/actions/create-cluster | ||
| with: | ||
| k8s_version: "v1.30.2" | ||
|
|
||
| - name: Deploy KFP with Postgres | ||
| uses: ./.github/actions/deploy | ||
| with: | ||
| db_type: "pgx" | ||
| pipeline_store: "database" | ||
| cache_enabled: ${{ matrix.cache_enabled }} | ||
| image_path: ${{ needs.build.outputs.IMAGE_PATH }} | ||
| image_tag: ${{ needs.build.outputs.IMAGE_TAG }} | ||
| image_registry: ${{ needs.build.outputs.IMAGE_REGISTRY }} | ||
| forward_port: "true" | ||
| - name: Port-forward Postgres | ||
| run: kubectl -n "$POSTGRES_NAMESPACE" port-forward svc/postgres-service ${{ env.DB_PORT }}:${{ env.DB_PORT }} --address=${{ env.DB_FORWARD_IP }} & | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. curious why you had to do |
||
|
|
||
| - name: Port-forward ML Metadata service | ||
| run: kubectl -n "$NAMESPACE" port-forward svc/metadata-grpc-service 8080:8080 & | ||
| # Exclude upgrade tests for the following reasons: | ||
| # 1. Responsibility: Upgrade tests are handled by the dedicated `upgrade-test.yml` workflow. | ||
| # 2. Incompatibility: This workflow runs tests against a single, clean deployment. It cannot | ||
| # accommodate the two-phase nature of upgrade tests (prepare on an old version, then | ||
| # verify on the new one). | ||
| # 3. No Baseline: As PostgreSQL was not officially supported before, there is no prior | ||
| # stable release to serve as a baseline for an upgrade test. | ||
| - name: Run v2 api tests | ||
| run: | | ||
| go run github.com/onsi/ginkgo/v2/ginkgo -r -v --label-filter="!UpgradePreparation && !UpgradeVerification" ./backend/test/v2/api/... -- \ | ||
| -namespace="$NAMESPACE" | ||
| - name: Run v2 integration tests | ||
| run: | | ||
| # v2/integration tests use testify/suite framework, not Ginkgo, so we must use 'go test' instead of 'ginkgo' | ||
| # Build the go test command with appropriate flags | ||
| TEST_CMD="go test -v -timeout 30m ./backend/test/v2/integration/..." | ||
|
|
||
| # Arguments for the test binary (passed after -args) | ||
| TEST_ARGS="-runIntegrationTests=true -namespace=$NAMESPACE -cacheEnabled=${{ matrix.cache_enabled }}" | ||
|
|
||
| if [[ "${{ matrix.cache_enabled }}" == "false" ]]; then | ||
| # When cache is disabled, we must skip the cache test itself. | ||
| # Use Go test's -skip flag to exclude TestCache | ||
| TEST_CMD="$TEST_CMD -skip TestCache" | ||
| fi | ||
|
|
||
| # Execute the test command with arguments | ||
| eval "$TEST_CMD -args $TEST_ARGS" | ||
|
|
||
| - name: Collect pod logs | ||
| if: always() | ||
| run: | | ||
| mkdir -p /tmp/tmp.kfp /tmp/tmp.postgres | ||
| ./.github/resources/scripts/collect-logs.sh --ns "$NAMESPACE" --output /tmp/tmp.kfp/pod_log.txt | ||
| ./.github/resources/scripts/collect-logs.sh --ns "$POSTGRES_NAMESPACE" --output /tmp/tmp.postgres/pod_log.txt | ||
|
|
||
| - name: Upload test artifacts | ||
| if: always() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: api-server-postgres-test-artifacts-cache-${{ matrix.cache_enabled }} | ||
| path: /tmp/tmp*/* | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like us to explore whether we can leverage the existing
api-server-tests.yml, this would allow us to continue to focus api tests in one workflow. Can we also use the./.github/actions/test-and-reportaction like the other jobs inapi-server-tests.yml, this way we can continue to have good test reports generated for the postgresql case.If we need to ignore upgrade tests, we can update the action to take in optional
--label-filtervaluescc @nsingla