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

chore: Upgrade manifests to 2.3.0 #583

Merged
merged 2 commits into from
Nov 21, 2024

Conversation

orfeas-k
Copy link
Contributor

@orfeas-k orfeas-k commented Nov 13, 2024

Upgrade manifests to 2.3.0. For producing the manifests, instructions in the CONTRIBUTING.md file were followed.

This includes:

  • Upgrading images
  • Updating kfp-api auth-manifests
  • Introducing a new envvar in kfp-ui

Fixes #580

Tests

CI is fails mostly intermittently due to #602 and #601. Locally, they are successful (I hit #601 only once which looks unrelated to charms changes). I 've also refreshed a CKF deployment with the PR's charms and ran some pipelines, the kfp UAT and also enabled a recurring one.

@orfeas-k orfeas-k force-pushed the kf-6540-upgrade-manifests branch from 12b6f5b to d3dbfd6 Compare November 14, 2024 13:32
@orfeas-k orfeas-k changed the base branch from main to dev-upgrade-kfp-2.3.0 November 19, 2024 10:30
@orfeas-k orfeas-k closed this Nov 19, 2024
@orfeas-k orfeas-k reopened this Nov 19, 2024
Copy link
Contributor

@DnPlas DnPlas left a comment

Choose a reason for hiding this comment

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

All changes LGTM, for testing this I deployed all kfp charms and checked the status, as well as their logs.
Since kfp-ui is the charm that received the most changes, I focused on it and inspected the pebble logs. While most of it looks normal, I noticed two things:

  1. There is a 2024-11-20T04:12:03.662Z [ml-pipeline-ui] Failed reading json data from '/etc/config/viewer-pod-template.json': 2024-11-20T04:12:03.664Z [ml-pipeline-ui] SyntaxError: Unexpected end of JSON input message.
  2. There are a lot of warnings like 2024-11-20T04:12:03.686Z [ml-pipeline-ui] (node:15) Warning: Accessing non-existent property 'which' of module exports inside circular dependency

I don't think any of this is affecting the charm, but it's worth double checking.

@orfeas-k
Copy link
Contributor Author

orfeas-k commented Nov 20, 2024

Hey @DnPlas, thank you for taking a thorough look. For the first one, I have already filed this issue #603. This has been observed in latest/edge kfp-ui (without the upgrades) and I think it should be present in 1.9 too. Regarding 2, this is also present in latest/edge without the upgrades. Let me file an issue for this as well and we 'd need to investigate separately. Regarding the charm's functionality, I used it in a CKF deployment without issues.

EDIT: Filed an issue #609

Copy link
Contributor

@NohaIhab NohaIhab left a comment

Choose a reason for hiding this comment

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

Thanks @orfeas-k , bundle-integration-v2 tests all passed locally
I can confirm the logs reported by @DnPlas do not affect the functionality

one thing missed: to upgrade the driver and launcher images in config.yaml of kfp-api charm. Note: we could use adding this to CONTRIBUTING.md since it does not come up in kubeflow/manifests diff.

@orfeas-k
Copy link
Contributor Author

@NohaIhab Very good catch Noha. Updated the PR. Also, I 've opened #610 against the dev branch updating the contributing.md file.

Copy link
Contributor

@DnPlas DnPlas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @orfeas-k

@orfeas-k orfeas-k merged commit 93a8049 into dev-upgrade-kfp-2.3.0 Nov 21, 2024
73 of 75 checks passed
@orfeas-k orfeas-k deleted the kf-6540-upgrade-manifests branch November 21, 2024 11:32
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.

Upgrade the manifests for 2.3.0
3 participants