Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update kubeflow/pipelines manifests from 2.0.5 #2623

Conversation

rimolive
Copy link
Member

Which issue is resolved by this Pull Request:
Resolves #

Description of your changes:

Checklist:

  • Unit tests pass:
    Make sure you have installed kustomize == 5.0.3
    1. make generate-changed-only
    2. make test

@juliusvonkohout
Copy link
Member

juliusvonkohout commented Feb 26, 2024

The git diff looks empy which is good
/manifests/apps/pipeline/upstream$ git diff /home/julius/Downloads/pipelines-2.0.5/manifests/kustomize .
but there are two topics:

  1. updating kfp-tekton to 2.0.5
  2. fixing the integration tests. ill just try a rerun for now.

I will add a script to apps/pipeline to automate the manifest synchronization.

@juliusvonkohout
Copy link
Member

Lets use #2625 for updates

@rimolive rimolive force-pushed the sync-kubeflow-pipelines-manifests-2.0.5 branch from e85cb65 to c9d249d Compare February 26, 2024 19:41
@juliusvonkohout
Copy link
Member

Maybe you need to cherry pick some test fixes from #2544

@rimolive
Copy link
Member Author

rimolive commented Feb 26, 2024

@chensun Need your help here. This PR is crucial to release Kubeflow 1.8.1.

Integration test is falling because of something related to cert-manager. Can you take a look?

@juliusvonkohout
Copy link
Member

juliusvonkohout commented Feb 27, 2024

@rimolive this has to be merged to master as well. Usually we start with Master and cherry pick to the 1.8 branch.

@chensun
Copy link
Member

chensun commented Feb 28, 2024

@chensun Need your help here. This PR is crucial to release Kubeflow 1.8.1.

Integration test is falling because of something related to cert-manager. Can you take a look?

Cert-manager was introduced by Amazon folks, my understanding is they should only apply to AWS deployment.
@surajkota Can you help take a look? Thanks!

@juliusvonkohout
Copy link
Member

juliusvonkohout commented Feb 28, 2024

cert-manager is used by default. Sometimes it just does not become ready fast enough. a wait for cert-manger as done here might help.

kubectl wait --for=jsonpath='{.subsets[0].addresses[0].targetRef.kind}'=Pod endpoints -l 'app in (cert-manager,webhook)' --timeout=180s -n cert-manager

juliusvonkohout added a commit that referenced this pull request Feb 28, 2024
@juliusvonkohout
Copy link
Member

@rimolive i pushed 0d53c9e please rebase

@juliusvonkohout
Copy link
Member

Now we get

Traceback (most recent call last):
  File "/home/runner/work/manifests/manifests/./tests/gh-actions/kf-objects/test_pipeline.py", line 6, in <module>
    @comp.create_component_from_func
AttributeError: module 'kfp.components' has no attribute 'create_component_from_func'. Did you mean: 'load_component_from_file'?
Error: Process completed with exit code 1.

@juliusvonkohout
Copy link
Member

https://github.com/kubeflow/manifests/blob/master/tests/gh-actions/kf-objects/test_pipeline.py must be adjusted or we downgrade pip3 install kfp==2.7.0 to 1.8.22

@juliusvonkohout
Copy link
Member

juliusvonkohout commented Feb 28, 2024

@rimolive Somewhere around here

./tests/e2e/hack/proxy_pipelines.sh
kubectl apply -f tests/e2e/yamls
python3 ./tests/gh-actions/kf-objects/test_pipeline.py
./tests/gh-actions/install_argo_cli.sh
argo wait @latest -n kubeflow-user-example-com --request-timeout 60
it is failing

Maybe the timeout is too tight, but i would really like to drop this outdated argo file and test and just check the workflow status directly with kubectl

@juliusvonkohout
Copy link
Member

juliusvonkohout commented Feb 28, 2024

@rimolive i need access to your branch to sign my commits. You can also sign them if you want.

@rimolive rimolive force-pushed the sync-kubeflow-pipelines-manifests-2.0.5 branch from 8f76ccb to c9d249d Compare February 29, 2024 13:53
Signed-off-by: Ricardo M. Oliveira <rmartine@redhat.com>
With KFP 2.0 release, we need to bump SDK version to the latest release (2.7.0)

Signed-off-by: Ricardo M. Oliveira <rmartine@redhat.com>
@rimolive rimolive force-pushed the sync-kubeflow-pipelines-manifests-2.0.5 branch from c9d249d to 20fccdf Compare February 29, 2024 14:07
@juliusvonkohout
Copy link
Member

Why did you revert my fix for KFP 2.7.0 to 1.8.22? We can stay on the 1.8.22 SDK. V2 pipelines are still on a beta level. Or at least we should test v1 and V2 pipelines.

@rimolive
Copy link
Member Author

Sorry, I did not notice you added that commit. Can you send it again?

@juliusvonkohout
Copy link
Member

It is just replacing the value 2.7.0 with 1.8.22

@juliusvonkohout juliusvonkohout force-pushed the sync-kubeflow-pipelines-manifests-2.0.5 branch from e06bc3e to 1992d4c Compare March 1, 2024 08:44
Signed-off-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>
@juliusvonkohout juliusvonkohout force-pushed the sync-kubeflow-pipelines-manifests-2.0.5 branch from 1992d4c to 0311979 Compare March 1, 2024 09:05
@juliusvonkohout
Copy link
Member

Ok we have to increase the timeout i guess, but that is something for the master branch.

@juliusvonkohout
Copy link
Member

/lgtm
/approve

Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: juliusvonkohout, rimolive
Once this PR has been reviewed and has the lgtm label, please assign stefanofioravanzo for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@juliusvonkohout juliusvonkohout merged commit 0fe0065 into kubeflow:v1.8-branch Mar 1, 2024
4 of 5 checks passed
juliusvonkohout added a commit that referenced this pull request Mar 1, 2024
* Update kubeflow/pipelines manifests from 2.0.5

Signed-off-by: Ricardo M. Oliveira <rmartine@redhat.com>

* Fix integration test

With KFP 2.0 release, we need to bump SDK version to the latest release (2.7.0)

Signed-off-by: Ricardo M. Oliveira <rmartine@redhat.com>

* only v1 is tested for now. We can add tests for v2 later on.

Signed-off-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>

---------

Signed-off-by: Ricardo M. Oliveira <rmartine@redhat.com>
Signed-off-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>
Co-authored-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>
juliusvonkohout added a commit that referenced this pull request Mar 1, 2024
* Update kubeflow/pipelines manifests from 2.0.5

Signed-off-by: Ricardo M. Oliveira <rmartine@redhat.com>

* Fix integration test

With KFP 2.0 release, we need to bump SDK version to the latest release (2.7.0)

Signed-off-by: Ricardo M. Oliveira <rmartine@redhat.com>

* only v1 is tested for now. We can add tests for v2 later on.

Signed-off-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>

---------

Signed-off-by: Ricardo M. Oliveira <rmartine@redhat.com>
Signed-off-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>
Co-authored-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>
juliusvonkohout added a commit that referenced this pull request Mar 1, 2024
* Merge pull request #2626 from rimolive/sync-kubeflow-kfp-tekton-manifests-2.0.5

Update kubeflow/kfp-tekton manifests from 2.0.5

Signed-off-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>

* Update kubeflow/pipelines manifests from 2.0.5 (#2623)

* Update kubeflow/pipelines manifests from 2.0.5

Signed-off-by: Ricardo M. Oliveira <rmartine@redhat.com>

* Fix integration test

With KFP 2.0 release, we need to bump SDK version to the latest release (2.7.0)

Signed-off-by: Ricardo M. Oliveira <rmartine@redhat.com>

* only v1 is tested for now. We can add tests for v2 later on.

Signed-off-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>

---------

Signed-off-by: Ricardo M. Oliveira <rmartine@redhat.com>
Signed-off-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>
Co-authored-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>

* fix readme

Signed-off-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>

---------

Signed-off-by: juliusvonkohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: Ricardo M. Oliveira <rmartine@redhat.com>
Co-authored-by: Ricardo Martinelli de Oliveira <rmartine@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants