Introduce services.opendatahub.io api group #1389

VaishnaviHire · 2024-11-25T16:51:16Z

Description

This PR includes following changes:

Initialize services.opendatahub.io api group and Monitoring API
Generalize ComponentReconciler --> Reconciler
Update components.opendatahub.io/part-of label --> platform.opendatahub.io/part-of

How Has This Been Tested?

Screenshot or short clip

Merge criteria

You have read the contributors guide.
Commit messages are meaningful - have a clear and concise summary and detailed explanation of what was changed and why.
Pull Request contains a description of the solution, a link to the JIRA issue, and to any dependent or related Pull Request.
Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
The developer has manually tested the changes and verified that the changes work

codecov · 2024-11-26T00:45:14Z

Codecov Report

Attention: Patch coverage is 14.28571% with 42 lines in your changes missing coverage. Please review.

Please upload report for BASE (feature-operator-refactor@15ca54e). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
pkg/controller/reconciler/reconciler_support.go	0.00%	21 Missing ⚠️
pkg/controller/reconciler/reconciler.go	0.00%	11 Missing ⚠️
...ctions/render/kustomize/action_render_manifests.go	16.66%	3 Missing and 2 partials ⚠️
...actions/render/template/action_render_templates.go	16.66%	3 Missing and 2 partials ⚠️

Additional details and impacted files

@@                     Coverage Diff                      @@
##             feature-operator-refactor    #1389   +/-   ##
============================================================
  Coverage                             ?   26.37%           
============================================================
  Files                                ?       65           
  Lines                                ?     5210           
  Branches                             ?        0           
============================================================
  Hits                                 ?     1374           
  Misses                               ?     3674           
  Partials                             ?      162

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lburgazzoli · 2024-11-28T07:52:05Z

apis/services/service.go

+type ServiceObject interface {
+	client.Object
+	WithStatus
+}


Not for this PR, but we can probably move the Status, WithStatus, ServiceObject to a shared packages and reuse it across all our at least internal APIs

lburgazzoli · 2024-11-28T07:56:39Z

pkg/controller/reconciler/reconciler.go

-	object components.ComponentObject,
-) (*ComponentReconciler, error) {
+// NewReconciler creates a new reconciler for the given type.
+func NewReconciler[T client.Object](mgr manager.Manager, name string, object T) (*Reconciler[T], error) {


do we need generics ?

Maybe we should add a check to allow only ComponentObject and ServiceObject?

if we merge ServiceObject and ComponentObject as per #1389 (comment), then we can maybe just use the new type ?

In that case, I will update the PR to merge the 2 objects

lburgazzoli · 2024-11-28T08:01:57Z

Quickly skimmed through the code and beside some minor comments, looks ok

zdtsw

two questions:

what is dscmonitorings?
do we want this service to be created for all ODH+RHOAIs ?

apis/services/service.go

apis/services/v1/monitoring_types.go

config/crd/patches/cainjection_in_services_monitorings.yaml

config/monitoring/base/kustomization.yaml

config/monitoring/prometheus/base/prometheus-cm-service-ca.yaml

pkg/controller/reconciler/reconciler.go

controllers/services/monitoring/monitoring_controller.go

VaishnaviHire · 2024-12-02T10:50:49Z

two questions:
what is dscmonitorings?

Its same as in components https://github.com/opendatahub-io/opendatahub-operator/blob/feature-operator-refactor/apis/components/v1/dashboard_types.go#L92 . It contains all the fields as exposed in the DSCI spec

do we want this service to be created for all ODH+RHOAIs ?

The Monitoring object will only be created only when managementState is set to Managed for monitoring in DSCI. We are not changing any operator behavior yet i.e the manifests will only get deployed when it's a Managed RHOAI. Moved manifests upstream to have same code upstream and downstream.

lburgazzoli · 2024-12-03T07:06:55Z

pkg/controller/reconciler/component_reconciler_actions_test.go

this can be renamed too

lburgazzoli · 2024-12-03T07:07:50Z

Just a small comment, that can be fixed in another branch, but LGTM, @zdtsw ?

zdtsw · 2024-12-03T07:45:56Z

Just a small comment, that can be fixed in another branch, but LGTM, @zdtsw ?

in general, LGTM,

but these should be removed:

config/rbac/services_dscmonitoring_editor_role.yaml
config/rbac/services_dscmonitoring_viewer_role.yaml

then renegerate bundle

bundle/manifests/services.opendatahub.io_monitorings.yaml
bundle/manifests/opendatahub-operator.clusterserviceversion.yaml
bundle/manifests/dscinitialization.opendatahub.io_dscinitializations.yaml

VaishnaviHire · 2024-12-03T12:53:04Z

@lburgazzoli @zdtsw Addressed your last comments

openshift-ci · 2024-12-03T13:44:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zdtsw

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:

~~OWNERS~~ [zdtsw]

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

…flux/component-updates/odh-ml-pipelines-scheduledworkflow-v2-v2-17 chore(deps): update odh-ml-pipelines-scheduledworkflow-v2-v2-17 to dda4a87

VaishnaviHire marked this pull request as draft November 25, 2024 16:51

openshift-ci bot requested review from CFSNM and StevenTobin November 25, 2024 16:51

openshift-ci bot added the do-not-merge/work-in-progress label Nov 25, 2024

VaishnaviHire added the feature-operator-refactor label Nov 25, 2024

openshift-merge-robot added the needs-rebase label Nov 27, 2024

VaishnaviHire changed the title ~~Add monitoring API~~ Introduce services.opendatahub.io api group Nov 27, 2024

VaishnaviHire force-pushed the add_monitoring branch from 51e8117 to 9007747 Compare November 27, 2024 17:00

openshift-merge-robot removed the needs-rebase label Nov 27, 2024

VaishnaviHire removed the do-not-merge/work-in-progress label Nov 27, 2024

VaishnaviHire marked this pull request as ready for review November 27, 2024 17:02

openshift-ci bot requested review from jackdelahunt and MarianMacik November 27, 2024 17:03

VaishnaviHire force-pushed the add_monitoring branch from 4c88ef1 to 1d3cf7d Compare November 27, 2024 17:08

lburgazzoli added feature-operator-refactor and removed feature-operator-refactor labels Nov 28, 2024

lburgazzoli reviewed Nov 28, 2024

View reviewed changes

zdtsw reviewed Nov 28, 2024

View reviewed changes

openshift-merge-robot added the needs-rebase label Dec 2, 2024

VaishnaviHire added 7 commits December 2, 2024 09:42

Add monitoring api

7f355be

Create services structs

4075fa5

Initialize service reconciler

0df8b03

Fix kustomize errors

5829c3b

Update to sync with latest changes

08ccdd4

Converge reconcilers for components and services

e0a721f

Update label to platform.opendatahub.io

325b324

VaishnaviHire added 4 commits December 2, 2024 10:16

sync with latest changes

3e10cae

Update api docs

79823a4

Update bundle

6a46d95

Add common types

bc2bff6

VaishnaviHire force-pushed the add_monitoring branch from 0e8c97a to bc2bff6 Compare December 2, 2024 16:19

openshift-merge-robot removed the needs-rebase label Dec 2, 2024

VaishnaviHire mentioned this pull request Dec 2, 2024

Update api version for internal apis to v1alpha1 #1411

Closed

5 tasks

lburgazzoli reviewed Dec 3, 2024

View reviewed changes

pkg/controller/reconciler/component_reconciler_actions_test.go Outdated

Copy link

Contributor

lburgazzoli Dec 3, 2024

Choose a reason for hiding this comment

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

this can be renamed too

Remove dscmonitoring role

04c1a21

zdtsw approved these changes Dec 3, 2024

View reviewed changes

openshift-ci bot assigned zdtsw Dec 3, 2024

openshift-ci bot added the lgtm label Dec 3, 2024

openshift-ci bot added the approved label Dec 3, 2024

openshift-merge-bot bot merged commit 970b40b into opendatahub-io:feature-operator-refactor Dec 3, 2024
10 checks passed

This was referenced Dec 8, 2024

A reconciler should not require the DSCI to perform cleanup action to avoid deadlock #1428

Closed

Refactor DSC controller #1430

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce services.opendatahub.io api group #1389

Introduce services.opendatahub.io api group #1389

VaishnaviHire commented Nov 25, 2024 •

edited

Loading

codecov bot commented Nov 26, 2024 •

edited

Loading

lburgazzoli Nov 28, 2024

lburgazzoli Nov 28, 2024

VaishnaviHire Nov 28, 2024

lburgazzoli Nov 28, 2024 •

edited

Loading

VaishnaviHire Dec 2, 2024

lburgazzoli commented Nov 28, 2024

zdtsw left a comment

VaishnaviHire commented Dec 2, 2024 •

edited

Loading

lburgazzoli Dec 3, 2024

lburgazzoli commented Dec 3, 2024

zdtsw commented Dec 3, 2024

VaishnaviHire commented Dec 3, 2024

openshift-ci bot commented Dec 3, 2024

Introduce services.opendatahub.io api group #1389

Introduce services.opendatahub.io api group #1389

Conversation

VaishnaviHire commented Nov 25, 2024 • edited Loading

Description

How Has This Been Tested?

Screenshot or short clip

Merge criteria

codecov bot commented Nov 26, 2024 • edited Loading

Codecov Report

lburgazzoli Nov 28, 2024

Choose a reason for hiding this comment

lburgazzoli Nov 28, 2024

Choose a reason for hiding this comment

VaishnaviHire Nov 28, 2024

Choose a reason for hiding this comment

lburgazzoli Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

VaishnaviHire Dec 2, 2024

Choose a reason for hiding this comment

lburgazzoli commented Nov 28, 2024

zdtsw left a comment

Choose a reason for hiding this comment

VaishnaviHire commented Dec 2, 2024 • edited Loading

lburgazzoli Dec 3, 2024

Choose a reason for hiding this comment

lburgazzoli commented Dec 3, 2024

zdtsw commented Dec 3, 2024

VaishnaviHire commented Dec 3, 2024

openshift-ci bot commented Dec 3, 2024

VaishnaviHire commented Nov 25, 2024 •

edited

Loading

codecov bot commented Nov 26, 2024 •

edited

Loading

lburgazzoli Nov 28, 2024 •

edited

Loading

VaishnaviHire commented Dec 2, 2024 •

edited

Loading