Skip to content

feat(kfp-api): add s3-credentials relation to relate with s3-integrator#956

Merged
mvlassis merged 37 commits into
mainfrom
kf-8704-kfp-api
Jun 26, 2026
Merged

feat(kfp-api): add s3-credentials relation to relate with s3-integrator#956
mvlassis merged 37 commits into
mainfrom
kf-8704-kfp-api

Conversation

@mvlassis

@mvlassis mvlassis commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add s3-credentials relation using the s3 interface.
  • Implemented the RelationCountGate component manually
  • The s3 endpoint is parsed into the host, port and TLS flag kfp expects (OBJECTSTORECONFIG_*)
  • Update the S3 service to include the tls-ca-chain, similar to the structure for the S3 manager in object-storage-integrators
  • Add integration tests, split into 2 distinct files:
    • test_charm_s3.py, which tests the relation with s3-integrator:s3-credentials
    • test_charm_object_storage.py, which tests the relation with minio:object-storage
  • Add test for migrating from minio-operator to s3-integrator in integration-object-storage

Removal

This PR removes the minio-service service created by kfp-api. The context is:

Notes

  • Split out of feat: support s3-integrator as an object storage backend #955. Companion PRs add the same support to kfp-ui and kfp-profile-controller.
  • The root tox.ini change is identical across the three split PRs (conflict-free); the ci.yaml matrix enables integration-s3 only for kfp-api here, so merging the companion PRs will require a small exclude-list merge.
  • I removed all references of mariadb and replaced them with mysql-k8s :)

Closes #964

Add the `s3` interface (via an `s3-credentials` relation) so kfp-api can
use an s3-integrator as an alternative to the in-cluster MinIO
(`object-storage`) backend. Exactly one of the two relations may be
related at a time; relating both blocks the charm. The s3 endpoint is
parsed into the host, port and TLS flag kfp expects, and bucket creation
is skipped for s3 (the integrator manages its own buckets).

Integration tests are split into object-storage and s3 suites, wired up
through a new `integration-s3` tox env and CI matrix entry.
@ckfbot ckfbot added backport track/2.16 Backport to track/2.16 backport track/2.15 Backport to track/2.15 backport track/2.5 Backport to track/2.5 labels Jun 18, 2026
@ckfbot

ckfbot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

🤖 Backport labels populated

Labels to this pull request were added automatically by the populate-labels.yaml action.

When the PR is merged, backport PRs according to the labels will be automatically created. To skip the backport creation, remove any unneeded labels before merging the PR.

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

Comment thread tox.ini Outdated
Comment thread charms/kfp-api/src/charm.py Outdated
Comment thread charms/kfp-api/src/charm.py Outdated
Comment thread charms/kfp-api/src/services/s3.py
mvlassis and others added 3 commits June 24, 2026 11:54
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Manos Vlassis <57320708+mvlassis@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Manos Vlassis <57320708+mvlassis@users.noreply.github.com>

@NohaIhab NohaIhab left a comment

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.

thanks @mvlassis, left some comments

Comment thread charms/kfp-api/src/charm.py Outdated
Comment thread charms/kfp-api/src/charm.py Outdated
Comment thread charms/kfp-api/src/charm.py Outdated
Comment thread charms/kfp-api/tests/integration/test_charm_object_storage.py

Copilot AI left a comment

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.

Comment thread charms/kfp-api/src/services/s3.py
Comment thread charms/kfp-api/src/services/s3.py Outdated
Comment thread charms/kfp-api/src/charm.py
Comment thread charms/kfp-api/tox.ini
mvlassis and others added 2 commits June 24, 2026 14:52
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Manos Vlassis <57320708+mvlassis@users.noreply.github.com>

@NohaIhab NohaIhab left a comment

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.

thanks @mvlassis

@dariofaccin dariofaccin left a comment

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.

Left some suggestions and questions.
In the integration tests we are deploying grafana-agent, checking alert rules (test_alert_rules), metrics (test_metrics_endpoint), logging (test_logging) and container security context (test_container_security_context) in both object-storage and s3 scenarios. Does it provide more coverage or is it CI time wasted?

Comment thread charms/kfp-api/src/services/s3.py Outdated
Comment thread charms/kfp-api/src/charm.py Outdated
Comment thread charms/kfp-api/src/charm.py Outdated
Comment thread charms/kfp-api/src/charm.py
Comment thread charms/kfp-api/src/charm.py Outdated
Comment thread charms/kfp-api/src/services/s3.py Outdated
Comment thread charms/kfp-api/src/services/s3.py Outdated
Comment thread charms/kfp-api/src/charm.py Outdated
Comment thread charms/kfp-api/tests/unit/test_operator.py
mvlassis and others added 4 commits June 25, 2026 14:27
Co-authored-by: Dario Faccin <35623244+dariofaccin@users.noreply.github.com>
Signed-off-by: Manos Vlassis <57320708+mvlassis@users.noreply.github.com>
@mvlassis

Copy link
Copy Markdown
Contributor Author

We discussed this question by @dariofaccin:

In the integration tests we are deploying grafana-agent, checking alert rules (test_alert_rules), metrics (test_metrics_endpoint), logging (test_logging) and container security context (test_container_security_context) in both object-storage and s3 scenarios. Does it provide more coverage or is it CI time wasted?

And the decision was to keep these tests, as it is the same pattern we use for the test_charm_ambient.py tests. The future plan is to deprecate the object-storage integration in the future, and thus remove the test_charm_object_storage.py file completely.

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

Comment thread charms/kfp-api/src/charm.py
Comment thread charms/kfp-api/src/charm.py Outdated
Comment thread bundle.yaml Outdated
mvlassis and others added 4 commits June 25, 2026 14:30
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Manos Vlassis <57320708+mvlassis@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Manos Vlassis <57320708+mvlassis@users.noreply.github.com>

@dariofaccin dariofaccin left a comment

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.

LGTM. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add s3 interface to kfp-api to relate with s3-integrator [feature] remove hard-coded configs in KFP V2 launcher

6 participants