Skip to content

Conversation

trgeiger
Copy link

@trgeiger trgeiger commented Sep 5, 2025

Adds a controller for reconciling ClusterCatalogs. Currently it uses a dummy "CatalogService" that doesn't actually do anything.

This is a reformatting and cherry-pick of commits in this draft PR so we can land the controller first and iterate on its functionality in a future PR to add the actual CatalogService implementation and front-end/handler logic.

Vendor and dependency updates are in their own commit to make it easier to review the actual code changes.

The work for creating this controller is tracked in OPRUN-4086.

@openshift-ci openshift-ci bot requested review from jhadvig and rhamilto September 5, 2025 15:43
@openshift-ci openshift-ci bot added component/backend Related to backend needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 5, 2025
Copy link
Contributor

openshift-ci bot commented Sep 5, 2025

Hi @trgeiger. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@spadgett
Copy link
Member

spadgett commented Sep 9, 2025

/ok-to-test

@openshift-ci openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 9, 2025
@trgeiger
Copy link
Author

trgeiger commented Sep 9, 2025

do not merge until we add tech preview feature flag support to console--I will then wrap all this functionality in that feature flag

Copy link
Member

@TheRealJon TheRealJon left a comment

Choose a reason for hiding this comment

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

Just a couple of questions

// The ClusterCatalog has been found on the cluster, add to or update cache
baseURL := clusterCatalog.Status.URLs.Base

if baseURL != "" {
Copy link
Member

Choose a reason for hiding this comment

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

Is this Status.URLs.Base a required property in the ClusterCatalog API? If not, should we add a debug-level log message when it's missing?

Copy link
Author

Choose a reason for hiding this comment

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

I think URLs is not required but if it exists, Base is a required field on a URL. So yeah I'll add some logic for catching that.

- Use klog for logging
- Handle missing URLs or Base fields
- Check err when creating controller manager
Default to :8081 to avoid collision with existing metrics endpoint at :8080.
@trgeiger trgeiger force-pushed the solo-catalog-controller branch from 1e49535 to 0dafc53 Compare September 23, 2025 15:35
@trgeiger trgeiger force-pushed the solo-catalog-controller branch from 0dafc53 to c3ea325 Compare September 23, 2025 15:37
@TheRealJon
Copy link
Member

QE Approver
/assign @yapei

Docs Approver:
/assign @jseseCCS

PX Approver:
/assign @sferich888

Copy link
Contributor

openshift-ci bot commented Sep 23, 2025

@TheRealJon: GitHub didn't allow me to assign the following users: jseseCCS.

Note that only openshift members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

QE Approver
/assign @yapei

Docs Approver:
/assign @jseseCCS

PX Approver:
/assign @sferich888

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copy link
Member

@TheRealJon TheRealJon left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 23, 2025
Copy link
Contributor

openshift-ci bot commented Sep 23, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: TheRealJon, trgeiger

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

The pull request process is described 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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 23, 2025
@trgeiger trgeiger changed the title Add ClusterCatalog controller OPRUN-4086: Add ClusterCatalog controller Sep 23, 2025
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 23, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 23, 2025

@trgeiger: This pull request references OPRUN-4086 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

In response to this:

Adds a controller for reconciling ClusterCatalogs. Currently it uses a dummy "CatalogService" that doesn't actually do anything.

This is a reformatting and cherry-pick of commits in this draft PR so we can land the controller first and iterate on its functionality in a future PR to add the actual CatalogService implementation and front-end/handler logic.

Vendor and dependency updates are in their own commit to make it easier to review the actual code changes.

The work for creating this controller is tracked in OPRUN-4086.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@trgeiger
Copy link
Author

/retest

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 23, 2025
Copy link
Contributor

openshift-ci bot commented Sep 23, 2025

New changes are detected. LGTM label has been removed.

@trgeiger trgeiger force-pushed the solo-catalog-controller branch 2 times, most recently from d9b60b8 to 81ce7c3 Compare September 23, 2025 20:50
🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@trgeiger trgeiger force-pushed the solo-catalog-controller branch from 81ce7c3 to 2f29492 Compare September 23, 2025 21:12
Copy link
Contributor

openshift-ci bot commented Sep 23, 2025

@trgeiger: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/backend 81ce7c3 link true /test backend

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@jianzhangbjz
Copy link

/assign @bandrade

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/backend Related to backend jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants