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.

Sign up for GitHub

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

Jump to bottom

feat: enhances FeatureTracker with spec #17

Merged
merged 16 commits into from
Jan 12, 2024
Merged

feat: enhances FeatureTracker with spec #17

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
b68188a
initial add tracker spec
cam-garrison Jan 5, 2024
d527e05
update tests, update crd
cam-garrison Jan 9, 2024
a75c127
add omitempty to origin struct
cam-garrison Jan 9, 2024
aa7dd82
undo accidental tag change
cam-garrison Jan 9, 2024
b9e0f99
re add empty line
cam-garrison Jan 9, 2024
4ae4406
move pointer operator
cam-garrison Jan 9, 2024
622d556
add testing
cam-garrison Jan 9, 2024
c2f7b71
lint
cam-garrison Jan 9, 2024
1e87b53
re-lint changes
cam-garrison Jan 10, 2024
103593f
add ownertype, move newOrigin() to shared util
cam-garrison Jan 10, 2024
387b013
Update apis/features/v1/features_types.go
cam-garrison Jan 10, 2024
0571826
remove origin from featureinitializer
cam-garrison Jan 10, 2024
b10d36f
modify kserve sm step to match dashboard's
cam-garrison Jan 11, 2024
b6c18cc
make dsci servicemesh setup like dashboard's
cam-garrison Jan 11, 2024
f632227
Merge branch 'service-mesh-integration' into feature-tracker-spec
cam-garrison Jan 12, 2024
3207250
fix merge issues, lint
cam-garrison Jan 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions apis/features/v1/features_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ const (
ConditionPhaseProcessTemplates = "ProcessTemplates"
ConditionPhaseApplyManifests = "ApplyManifests"
ConditionPhasePostConditions = "FeaturePostConditions"
ComponentType = "Component"
DSCIType = "DSCI"
Copy link

Choose a reason for hiding this comment

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

How about improving this slightly and make sub-type for string?

type OwnerType string

const (
	ComponentOwner OwnerType = "Component"
	DSCIOwner OwnerType = "DSCI"
)

and use this type instead? That would make it a bit more focused and type-safe. We already use something similar across the codebase - ManagementState

Copy link

Choose a reason for hiding this comment

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

On that note - maybe a follow-up PR to our "feature" branch could similarly make the condition phase its own type. Somehow we missed that opportunity in the original PR.

Copy link
Author

Choose a reason for hiding this comment

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

done this here 103593f

)

func (s *FeatureTracker) ToOwnerReference() metav1.OwnerReference {
Expand All @@ -40,8 +42,16 @@ func (s *FeatureTracker) ToOwnerReference() metav1.OwnerReference {
}
}

// Origin describes the type of object that created the related Feature to this FeatureTracker
cam-garrison marked this conversation as resolved.
Show resolved Hide resolved
type Origin struct {
Type string `json:"type,omitempty"`
Name string `json:"name,omitempty"`
}

// FeatureTrackerSpec defines the desired state of FeatureTracker.
type FeatureTrackerSpec struct {
Origin Origin `json:"origin,omitempty"`
AppNamespace string `json:"appNamespace,omitempty"`
}

// FeatureTrackerStatus defines the observed state of FeatureTracker.
Expand Down
6 changes: 5 additions & 1 deletion components/dashboard/dashboard.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
featurev1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/components"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/common"
Expand Down Expand Up @@ -217,7 +218,10 @@ func (d *Dashboard) Cleanup(cli client.Client, dscispec *dsciv1.DSCInitializatio
}

if shouldConfigureServiceMesh {
serviceMeshInitializer := feature.NewFeaturesInitializer(dscispec, d.defineServiceMeshFeatures(dscispec))
serviceMeshInitializer := feature.NewFeaturesInitializer(dscispec, d.defineServiceMeshFeatures(dscispec), featurev1.Origin{
Copy link

Choose a reason for hiding this comment

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

I would extract it to it's own origin variable for better readability. Also, if the only reason to pass it is to use it in .For of the defineXYFeatures I think it could be a parameter of this function instead. That would also remove a need for keeping it as a field of FeaturesInitializer.

Copy link
Author

Choose a reason for hiding this comment

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

done here:
0571826

learned about go's closures for this : ) , didn't know how to accomplish this at first which is what led to adding origin to the featureinitializer

Type: featurev1.ComponentType,
Name: d.GetComponentName(),
})

if err := serviceMeshInitializer.Prepare(); err != nil {
return err
Expand Down
8 changes: 6 additions & 2 deletions components/dashboard/servicemesh_setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

dsci "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
featurev1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/servicemesh"
Expand All @@ -21,7 +22,10 @@ func (d *Dashboard) configureServiceMesh(cli client.Client, owner metav1.Object,
}

if shouldConfigureServiceMesh {
serviceMeshInitializer := feature.NewFeaturesInitializer(dscispec, d.defineServiceMeshFeatures(dscispec))
serviceMeshInitializer := feature.NewFeaturesInitializer(dscispec, d.defineServiceMeshFeatures(dscispec), featurev1.Origin{
Type: featurev1.ComponentType,
Name: d.GetComponentName(),
})

if err := serviceMeshInitializer.Prepare(); err != nil {
return err
Expand All @@ -43,7 +47,7 @@ func (d *Dashboard) configureServiceMesh(cli client.Client, owner metav1.Object,
func (d *Dashboard) defineServiceMeshFeatures(dscispec *dsci.DSCInitializationSpec) feature.DefinedFeatures {
return func(s *feature.FeaturesInitializer) error {
createMeshResources, err := feature.CreateFeature("dashboard-create-service-mesh-routing-resources").
For(dscispec).
For(dscispec, s.Origin).
Manifests(
path.Join(feature.ControlPlaneDir, "components", d.GetComponentName()),
).
Expand Down
13 changes: 10 additions & 3 deletions components/kserve/kserve.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
featurev1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/components"
infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/infrastructure/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
Expand Down Expand Up @@ -185,9 +186,12 @@ func (k *Kserve) configureServerless(instance *dsciv1.DSCInitializationSpec) err
case operatorv1.Managed: // standard workflow to create CR
switch instance.ServiceMesh.ManagementState {
case operatorv1.Unmanaged, operatorv1.Removed:
return fmt.Errorf("ServiceMesh is need to set to 'Managaed' in DSCI CR, it is required by KServe serving field")
return fmt.Errorf("ServiceMesh is need to set to 'Managed' in DSCI CR, it is required by KServe serving field")
}
serverlessInitializer := feature.NewFeaturesInitializer(instance, k.configureServerlessFeatures)
serverlessInitializer := feature.NewFeaturesInitializer(instance, k.configureServerlessFeatures, featurev1.Origin{
Type: featurev1.ComponentType,
Name: k.GetComponentName(),
})

if err := serverlessInitializer.Prepare(); err != nil {
return err
Expand All @@ -201,7 +205,10 @@ func (k *Kserve) configureServerless(instance *dsciv1.DSCInitializationSpec) err
}

func (k *Kserve) removeServerlessFeatures(instance *dsciv1.DSCInitializationSpec) error {
serverlessInitializer := feature.NewFeaturesInitializer(instance, k.configureServerlessFeatures)
serverlessInitializer := feature.NewFeaturesInitializer(instance, k.configureServerlessFeatures, featurev1.Origin{
Type: featurev1.ComponentType,
Name: k.GetComponentName(),
})

if err := serverlessInitializer.Prepare(); err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions components/kserve/serverless_setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const (

func (k *Kserve) configureServerlessFeatures(s *feature.FeaturesInitializer) error {
servingDeployment, err := feature.CreateFeature("serverless-serving-deployment").
For(s.DSCInitializationSpec).
For(s.DSCInitializationSpec, s.Origin).
Manifests(
path.Join(templatesDir, "serving-install"),
).
Expand All @@ -37,7 +37,7 @@ func (k *Kserve) configureServerlessFeatures(s *feature.FeaturesInitializer) err
s.Features = append(s.Features, servingDeployment)

servingIstioGateways, err := feature.CreateFeature("serverless-serving-gateways").
For(s.DSCInitializationSpec).
For(s.DSCInitializationSpec, s.Origin).
PreConditions(
// Check serverless is installed
feature.WaitForResourceToBeCreated(knativeServingNamespace, gvr.KnativeServing),
Expand Down
12 changes: 12 additions & 0 deletions config/crd/bases/features.opendatahub.io_featuretrackers.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ spec:
type: object
spec:
description: FeatureTrackerSpec defines the desired state of FeatureTracker.
properties:
appNamespace:
type: string
origin:
description: Origin describes the type of object that created the
related Feature to this FeatureTracker
properties:
name:
type: string
type:
type: string
type: object
type: object
status:
description: FeatureTrackerStatus defines the observed state of FeatureTracker.
Expand Down
27 changes: 17 additions & 10 deletions controllers/dscinitialization/servicemesh_setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
corev1 "k8s.io/api/core/v1"

dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
featurev1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/servicemesh"
Expand All @@ -16,7 +17,7 @@ func defineServiceMeshFeatures(f *feature.FeaturesInitializer) error {
serviceMeshSpec := f.ServiceMesh

smcpCreation, errSmcp := feature.CreateFeature("service-mesh-control-plane-creation").
For(f.DSCInitializationSpec).
For(f.DSCInitializationSpec, f.Origin).
Manifests(
path.Join(feature.ControlPlaneDir, "base", "control-plane.tmpl"),
).
Expand All @@ -35,7 +36,7 @@ func defineServiceMeshFeatures(f *feature.FeaturesInitializer) error {

if serviceMeshSpec.ControlPlane.MetricsCollection == "Istio" {
metricsCollection, errMetrics := feature.CreateFeature("service-mesh-monitoring").
For(f.DSCInitializationSpec).
For(f.DSCInitializationSpec, f.Origin).
Manifests(
path.Join(feature.MonitoringDir),
).
Expand All @@ -51,7 +52,7 @@ func defineServiceMeshFeatures(f *feature.FeaturesInitializer) error {
}

if oauth, err := feature.CreateFeature("service-mesh-control-plane-configure-oauth").
For(f.DSCInitializationSpec).
For(f.DSCInitializationSpec, f.Origin).
Manifests(
path.Join(feature.ControlPlaneDir, "base"),
path.Join(feature.ControlPlaneDir, "oauth"),
Expand Down Expand Up @@ -79,7 +80,7 @@ func defineServiceMeshFeatures(f *feature.FeaturesInitializer) error {
}

if cfMaps, err := feature.CreateFeature("shared-config-maps").
For(f.DSCInitializationSpec).
For(f.DSCInitializationSpec, f.Origin).
WithResources(servicemesh.ConfigMaps).
Load(); err != nil {
return err
Expand All @@ -88,7 +89,7 @@ func defineServiceMeshFeatures(f *feature.FeaturesInitializer) error {
}

if serviceMesh, err := feature.CreateFeature("app-add-namespace-to-service-mesh").
For(f.DSCInitializationSpec).
For(f.DSCInitializationSpec, f.Origin).
Manifests(
path.Join(feature.ControlPlaneDir, "smm.tmpl"),
path.Join(feature.ControlPlaneDir, "namespace.patch.tmpl"),
Expand All @@ -101,7 +102,7 @@ func defineServiceMeshFeatures(f *feature.FeaturesInitializer) error {
}

if gatewayRoute, err := feature.CreateFeature("service-mesh-create-gateway-route").
For(f.DSCInitializationSpec).
For(f.DSCInitializationSpec, f.Origin).
Manifests(
path.Join(feature.ControlPlaneDir, "routing"),
).
Expand All @@ -116,7 +117,7 @@ func defineServiceMeshFeatures(f *feature.FeaturesInitializer) error {
}

if dataScienceProjects, err := feature.CreateFeature("app-migrate-data-science-projects").
For(f.DSCInitializationSpec).
For(f.DSCInitializationSpec, f.Origin).
WithResources(servicemesh.MigratedDataScienceProjects).
Load(); err != nil {
return err
Expand All @@ -125,7 +126,7 @@ func defineServiceMeshFeatures(f *feature.FeaturesInitializer) error {
}

if extAuthz, err := feature.CreateFeature("service-mesh-control-plane-setup-external-authorization").
For(f.DSCInitializationSpec).
For(f.DSCInitializationSpec, f.Origin).
Manifests(
path.Join(feature.AuthDir, "auth-smm.tmpl"),
path.Join(feature.AuthDir, "base"),
Expand Down Expand Up @@ -168,7 +169,10 @@ func (r *DSCInitializationReconciler) configureServiceMesh(instance *dsciv1.DSCI

switch instance.Spec.ServiceMesh.ManagementState {
case operatorv1.Managed:
serviceMeshInitializer := feature.NewFeaturesInitializer(&instance.Spec, defineServiceMeshFeatures)
serviceMeshInitializer := feature.NewFeaturesInitializer(&instance.Spec, defineServiceMeshFeatures, featurev1.Origin{
Type: featurev1.DSCIType,
Name: instance.Name,
})
if err := serviceMeshInitializer.Prepare(); err != nil {
r.Log.Error(err, "failed configuring service mesh resources")
r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DSCInitializationReconcileError", "failed configuring service mesh resources")
Expand All @@ -195,7 +199,10 @@ func (r *DSCInitializationReconciler) configureServiceMesh(instance *dsciv1.DSCI
func (r *DSCInitializationReconciler) removeServiceMesh(instance *dsciv1.DSCInitialization) error {
// on condition of Managed, do not handle Removed when set to Removed it tigger DSCI reconcile to cleanup
if instance.Spec.ServiceMesh.ManagementState == operatorv1.Managed {
serviceMeshInitializer := feature.NewFeaturesInitializer(&instance.Spec, defineServiceMeshFeatures)
serviceMeshInitializer := feature.NewFeaturesInitializer(&instance.Spec, defineServiceMeshFeatures, featurev1.Origin{
Type: featurev1.DSCIType,
Name: instance.Name,
})

if err := serviceMeshInitializer.Prepare(); err != nil {
r.Log.Error(err, "failed configuring service mesh resources")
Expand Down
4 changes: 3 additions & 1 deletion pkg/feature/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/config"

v1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
featurev1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1"
infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/infrastructure/v1"
)

Expand All @@ -26,12 +27,13 @@ func CreateFeature(name string) *featureBuilder {
return &featureBuilder{name: name}
}

func (fb *featureBuilder) For(spec *v1.DSCInitializationSpec) *featureBuilder {
func (fb *featureBuilder) For(spec *v1.DSCInitializationSpec, origin featurev1.Origin) *featureBuilder {
createSpec := func(f *Feature) error {
f.Spec = &Spec{
AppNamespace: spec.ApplicationsNamespace,
ServiceMeshSpec: &spec.ServiceMesh,
Serving: &infrav1.ServingSpec{},
Origin: &origin,
}

return nil
Expand Down
4 changes: 4 additions & 0 deletions pkg/feature/feature.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ func (f *Feature) createFeatureTracker() error {
ObjectMeta: metav1.ObjectMeta{
Name: f.Spec.AppNamespace + "-" + common.TrimToRFC1123Name(f.Name),
},
Spec: featurev1.FeatureTrackerSpec{
Origin: *f.Spec.Origin,
AppNamespace: f.Spec.AppNamespace,
},
}

foundTracker, err := f.DynamicClient.Resource(gvr.FeatureTracker).Get(context.TODO(), tracker.Name, metav1.GetOptions{})
Expand Down
5 changes: 4 additions & 1 deletion pkg/feature/initializer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,23 @@ import (
"github.com/hashicorp/go-multierror"

v1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
featurev1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1"
)

type FeaturesInitializer struct {
*v1.DSCInitializationSpec
definedFeatures DefinedFeatures
Features []*Feature
Origin featurev1.Origin
}

type DefinedFeatures func(featuresInitializer *FeaturesInitializer) error

func NewFeaturesInitializer(spec *v1.DSCInitializationSpec, def DefinedFeatures) *FeaturesInitializer {
func NewFeaturesInitializer(spec *v1.DSCInitializationSpec, def DefinedFeatures, origin featurev1.Origin) *FeaturesInitializer {
return &FeaturesInitializer{
DSCInitializationSpec: spec,
definedFeatures: def,
Origin: origin,
}
}

Expand Down
1 change: 1 addition & 0 deletions pkg/feature/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type Spec struct {
KnativeCertificateSecret string
KnativeIngressDomain string
Tracker *featurev1.FeatureTracker
Origin *featurev1.Origin
}

type OAuth struct {
Expand Down
Loading
Loading