Skip to content

Commit

Permalink
fix the PR review comments and rebase
Browse files Browse the repository at this point in the history
Signed-off-by: Abdul Hameed <[email protected]>
  • Loading branch information
redhatHameed committed Jan 31, 2025
1 parent cfb0119 commit d15be4d
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 13 deletions.
1 change: 1 addition & 0 deletions apis/components/v1alpha1/feastoperator_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

const (
// FeastOperatorName is the name of the new component
//"feastoperator" is named to distinguish it from "feast," as it specifically refers to the operator responsible for deploying, managing, Feast feature store services and to avoid confusion with the core Feast components.
FeastOperatorComponentName = "feastoperator"

// FeastOperatorInstanceName is the singleton name for the FeastOperator instance
Expand Down
7 changes: 3 additions & 4 deletions controllers/components/feastoperator/feastoperator.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/controllers/status"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
cr "github.com/opendatahub-io/opendatahub-operator/v2/pkg/componentsregistry"
odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations"
Expand Down Expand Up @@ -56,7 +55,7 @@ func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) common.Pla
}

Check warning on line 55 in controllers/components/feastoperator/feastoperator.go

View check run for this annotation

Codecov / codecov/patch

controllers/components/feastoperator/feastoperator.go#L40-L55

Added lines #L40 - L55 were not covered by tests
}

func (s *componentHandler) Init(platform cluster.Platform) error {
func (s *componentHandler) Init(_ common.Platform) error {
if err := odhdeploy.ApplyParams(manifestPath().String(), imageParamMap); err != nil {
return fmt.Errorf("failed to update images on path %s: %w", manifestPath(), err)
}

Check warning on line 61 in controllers/components/feastoperator/feastoperator.go

View check run for this annotation

Codecov / codecov/patch

controllers/components/feastoperator/feastoperator.go#L58-L61

Added lines #L58 - L61 were not covered by tests
Expand All @@ -70,7 +69,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
return errors.New("failed to convert to FeastOperator")
}

Check warning on line 70 in controllers/components/feastoperator/feastoperator.go

View check run for this annotation

Codecov / codecov/patch

controllers/components/feastoperator/feastoperator.go#L66-L70

Added lines #L66 - L70 were not covered by tests

dsc.Status.InstalledComponents[LegacyComponentName] = false
dsc.Status.InstalledComponents[ComponentName] = false
dsc.Status.Components.FeastOperator.ManagementSpec.ManagementState = s.GetManagementState(dsc)
dsc.Status.Components.FeastOperator.FeastOperatorCommonStatus = nil

Expand All @@ -83,7 +82,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl

switch s.GetManagementState(dsc) {
case operatorv1.Managed:
dsc.Status.InstalledComponents[LegacyComponentName] = true
dsc.Status.InstalledComponents[ComponentName] = true
dsc.Status.Components.FeastOperator.FeastOperatorCommonStatus = c.Status.FeastOperatorCommonStatus.DeepCopy()

if rc := meta.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,25 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
Owns(&corev1.ConfigMap{}).
Owns(&rbacv1.RoleBinding{}).
Owns(&rbacv1.Role{}).
Owns(&rbacv1.ClusterRoleBinding{}).
Owns(&rbacv1.ClusterRole{}).
Owns(&corev1.ServiceAccount{}).
Owns(&corev1.Service{}).
Owns(&appsv1.Deployment{}, reconciler.WithPredicates(resources.NewDeploymentPredicate())).
Watches(
&extv1.CustomResourceDefinition{},
reconciler.WithEventHandler(
handlers.ToNamed(componentApi.FeastOperatorInstanceName)),
reconciler.WithPredicates(
component.ForLabel(labels.ODH.Component(LegacyComponentName), labels.True)),
component.ForLabel(labels.ODH.Component(ComponentName), labels.True)),
).
// Add FeastOperator-specific actions
WithAction(initialize).
WithAction(devFlags).
WithAction(kustomize.NewAction(
kustomize.WithCache(),
kustomize.WithLabel(labels.ODH.Component(LegacyComponentName), labels.True),
kustomize.WithLabel(labels.K8SCommon.PartOf, LegacyComponentName),
kustomize.WithLabel(labels.ODH.Component(ComponentName), labels.True),
kustomize.WithLabel(labels.K8SCommon.PartOf, ComponentName),
)).
WithAction(deploy.NewAction(
deploy.WithCache(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,5 @@ func devFlags(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
rr.Manifests[0].SourcePath = manifestConfig.SourcePath
}

Check warning on line 35 in controllers/components/feastoperator/feastoperator_controller_actions.go

View check run for this annotation

Codecov / codecov/patch

controllers/components/feastoperator/feastoperator_controller_actions.go#L23-L35

Added lines #L23 - L35 were not covered by tests
}
// TODO: Implement devflags logmode logic
return nil

Check warning on line 37 in controllers/components/feastoperator/feastoperator_controller_actions.go

View check run for this annotation

Codecov / codecov/patch

controllers/components/feastoperator/feastoperator_controller_actions.go#L37

Added line #L37 was not covered by tests
}
5 changes: 0 additions & 5 deletions controllers/components/feastoperator/feastoperator_support.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ const (

ReadyConditionType = conditionsv1.ConditionType(componentApi.FeastOperatorKind + status.ReadySuffix)

// LegacyComponentName is the name of the component that is assigned to deployments
// via Kustomize. Since a deployment selector is immutable, we can't upgrade existing
// deployment to the new component name, so keep it around till we figure out a solution.
LegacyComponentName = "feastoperator"

ManifestsSourcePath = "overlays/odh"
)

Expand Down

0 comments on commit d15be4d

Please sign in to comment.