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.

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

feat: simplifies Feature API #831

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
13 changes: 13 additions & 0 deletions apis/features/v1/features_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,19 @@ type FeatureTracker struct {
Status FeatureTrackerStatus `json:"status,omitempty"`
}

// NewFeatureTracker instantiate FeatureTracker instance.
func NewFeatureTracker(name, appNamespace string) *FeatureTracker {
return &FeatureTracker{
TypeMeta: metav1.TypeMeta{
APIVersion: "features.opendatahub.io/v1",
Kind: "FeatureTracker",
},
ObjectMeta: metav1.ObjectMeta{
Name: appNamespace + "-" + name,
},
}
}

type FeaturePhase string
type OwnerType string

Expand Down
20 changes: 6 additions & 14 deletions components/kserve/kserve.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client, resC
}
}
// check on dependent operators if all installed in cluster
dependOpsErrors := checkDepedentOps(cli).ErrorOrNil()
dependOpsErrors := checkDependentOperators(cli).ErrorOrNil()
if dependOpsErrors != nil {
return dependOpsErrors
}
Expand Down Expand Up @@ -187,30 +187,22 @@ func (k *Kserve) configureServerless(instance *dsciv1.DSCInitializationSpec) err
return fmt.Errorf("ServiceMesh is need to set to 'Managed' in DSCI CR, it is required by KServe serving field")
}

serverlessInitializer := feature.ComponentFeaturesInitializer(k, instance, k.configureServerlessFeatures())
serverlessFeatures := feature.ComponentFeaturesHandler(k, instance, k.configureServerlessFeatures())

if err := serverlessInitializer.Prepare(); err != nil {
return err
}

if err := serverlessInitializer.Apply(); err != nil {
if err := serverlessFeatures.Apply(); err != nil {
return err
}
}
return nil
}

func (k *Kserve) removeServerlessFeatures(instance *dsciv1.DSCInitializationSpec) error {
serverlessInitializer := feature.ComponentFeaturesInitializer(k, instance, k.configureServerlessFeatures())

if err := serverlessInitializer.Prepare(); err != nil {
return err
}
serverlessFeatures := feature.ComponentFeaturesHandler(k, instance, k.configureServerlessFeatures())

return serverlessInitializer.Delete()
return serverlessFeatures.Delete()
}

func checkDepedentOps(cli client.Client) *multierror.Error {
func checkDependentOperators(cli client.Client) *multierror.Error {
var multiErr *multierror.Error

if found, err := deploy.OperatorExists(cli, ServiceMeshOperator); err != nil {
Expand Down
50 changes: 22 additions & 28 deletions components/kserve/serverless_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,12 @@ import (
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/servicemesh"
)

const (
templatesDir = "templates/serverless"
)

func (k *Kserve) configureServerlessFeatures() feature.DefinedFeatures {
return func(initializer *feature.FeaturesInitializer) error {
servingDeployment, err := feature.CreateFeature("serverless-serving-deployment").
With(initializer.DSCInitializationSpec).
From(initializer.Source).
func (k *Kserve) configureServerlessFeatures() feature.FeaturesProvider {
return func(handler *feature.FeaturesHandler) error {
servingDeploymentErr := feature.CreateFeature("serverless-serving-deployment").
For(handler).
Manifests(
path.Join(templatesDir, "serving-install"),
path.Join(feature.ServerlessDir, "serving-install"),
).
WithData(PopulateComponentSettings(k)).
PreConditions(
Expand All @@ -27,47 +22,46 @@ func (k *Kserve) configureServerlessFeatures() feature.DefinedFeatures {
servicemesh.EnsureServiceMeshInstalled,
feature.CreateNamespaceIfNotExists(serverless.KnativeServingNamespace),
).
PostConditions(
feature.WaitForPodsToBeReady(serverless.KnativeServingNamespace),
).
Load()
if err != nil {
return err
if servingDeploymentErr != nil {
return servingDeploymentErr
}
initializer.Features = append(initializer.Features, servingDeployment)

servingNetIstioSecretFiltering, err := feature.CreateFeature("serverless-net-istio-secret-filtering").
With(initializer.DSCInitializationSpec).
From(initializer.Source).
servingNetIstioSecretFilterinErr := feature.CreateFeature("serverless-net-istio-secret-filtering").
For(handler).
Manifests(
path.Join(templatesDir, "serving-net-istio-secret-filtering.patch.tmpl"),
path.Join(feature.ServerlessDir, "serving-net-istio-secret-filtering.patch.tmpl"),
).
WithData(PopulateComponentSettings(k)).
PreConditions(serverless.EnsureServerlessServingDeployed).
PostConditions(
feature.WaitForPodsToBeReady(serverless.KnativeServingNamespace),
).
Load()
if err != nil {
return err
if servingNetIstioSecretFilterinErr != nil {
return servingNetIstioSecretFilterinErr
}
initializer.Features = append(initializer.Features, servingNetIstioSecretFiltering)

servingIstioGateways, err := feature.CreateFeature("serverless-serving-gateways").
With(initializer.DSCInitializationSpec).
From(initializer.Source).
serverlessGwErr := feature.CreateFeature("serverless-serving-gateways").
For(handler).
PreConditions(serverless.EnsureServerlessServingDeployed).
WithData(
PopulateComponentSettings(k),
serverless.ServingDefaultValues,
serverless.ServingIngressDomain,
PopulateComponentSettings(k),
).
WithResources(serverless.ServingCertificateResource).
Manifests(
path.Join(templatesDir, "serving-istio-gateways"),
path.Join(feature.ServerlessDir, "serving-istio-gateways"),
).
Load()
if err != nil {
return err
if serverlessGwErr != nil {
return serverlessGwErr
}
initializer.Features = append(initializer.Features, servingIstioGateways)

return nil
}
}
Expand Down
56 changes: 20 additions & 36 deletions controllers/dscinitialization/servicemesh_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,12 @@ import (
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/servicemesh"
)

const templatesDir = "templates/servicemesh"

func (r *DSCInitializationReconciler) configureServiceMesh(instance *dsciv1.DSCInitialization) error {
switch instance.Spec.ServiceMesh.ManagementState {
case operatorv1.Managed:
serviceMeshInitializer := feature.ClusterFeaturesInitializer(instance, configureServiceMeshFeatures())
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")
return err
}
serviceMeshFeatures := feature.ClusterFeaturesHandler(instance, configureServiceMeshFeatures())

if err := serviceMeshInitializer.Apply(); err != nil {
if err := serviceMeshFeatures.Apply(); err != nil {
r.Log.Error(err, "failed applying service mesh resources")
r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DSCInitializationReconcileError", "failed applying service mesh resources")
return err
Expand All @@ -43,15 +36,9 @@ 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 trigger DSCI reconcile to cleanup
if instance.Spec.ServiceMesh.ManagementState == operatorv1.Managed {
serviceMeshInitializer := feature.ClusterFeaturesInitializer(instance, configureServiceMeshFeatures())
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")

return err
}
serviceMeshFeatures := feature.ClusterFeaturesHandler(instance, configureServiceMeshFeatures())

if err := serviceMeshInitializer.Delete(); err != nil {
if err := serviceMeshFeatures.Delete(); err != nil {
r.Log.Error(err, "failed deleting service mesh resources")
r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DSCInitializationReconcileError", "failed deleting service mesh resources")

Expand All @@ -62,15 +49,14 @@ func (r *DSCInitializationReconciler) removeServiceMesh(instance *dsciv1.DSCInit
return nil
}

func configureServiceMeshFeatures() feature.DefinedFeatures {
return func(initializer *feature.FeaturesInitializer) error {
serviceMeshSpec := initializer.DSCInitializationSpec.ServiceMesh
func configureServiceMeshFeatures() feature.FeaturesProvider {
return func(handler *feature.FeaturesHandler) error {
serviceMeshSpec := handler.DSCInitializationSpec.ServiceMesh

smcpCreation, errSmcp := feature.CreateFeature("mesh-control-plane-creation").
With(initializer.DSCInitializationSpec).
From(initializer.Source).
smcpCreationErr := feature.CreateFeature("mesh-control-plane-creation").
For(handler).
Manifests(
path.Join(templatesDir, "base", "create-smcp.tmpl"),
path.Join(feature.ServiceMeshDir, "base", "create-smcp.tmpl"),
).
PreConditions(
servicemesh.EnsureServiceMeshOperatorInstalled,
Expand All @@ -81,27 +67,25 @@ func configureServiceMeshFeatures() feature.DefinedFeatures {
).
Load()

if errSmcp != nil {
return errSmcp
if smcpCreationErr != nil {
return smcpCreationErr
}
initializer.Features = append(initializer.Features, smcpCreation)

if serviceMeshSpec.ControlPlane.MetricsCollection == "Istio" {
metricsCollection, errMetrics := feature.CreateFeature("mesh-metrics-collection").
With(initializer.DSCInitializationSpec).
From(initializer.Source).
Manifests(
path.Join(templatesDir, "metrics-collection"),
).
metricsCollectionErr := feature.CreateFeature("mesh-metrics-collection").
For(handler).
PreConditions(
servicemesh.EnsureServiceMeshInstalled,
).
Manifests(
path.Join(feature.ServiceMeshDir, "metrics-collection"),
).
Load()
if errMetrics != nil {
return errMetrics
if metricsCollectionErr != nil {
return metricsCollectionErr
}
initializer.Features = append(initializer.Features, metricsCollection)
}

return nil
}
}
54 changes: 23 additions & 31 deletions pkg/feature/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,58 +13,48 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"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"
)

type partialBuilder func(f *Feature) error

type featureBuilder struct {
name string
config *rest.Config
builders []partialBuilder
fsys fs.FS
name string
config *rest.Config
builders []partialBuilder
featuresHanlder *FeaturesHandler
fsys fs.FS
}

func CreateFeature(name string) *featureName { //nolint:golint,revive //No need to export featureBuilder.
return &featureName{
func CreateFeature(name string) *usingFeaturesHandler { //nolint:golint,revive //No need to export featureBuilder.
return &usingFeaturesHandler{
name: name,
}
}

type featureName struct {
type usingFeaturesHandler struct {
name string
}

func (fn *featureName) With(spec *v1.DSCInitializationSpec) *featureSource {
return &featureSource{
spec: spec,
name: fn.name,
}
}

type featureSource struct {
spec *v1.DSCInitializationSpec
name string
}

func (fo *featureSource) From(source featurev1.Source) *featureBuilder {
func (u *usingFeaturesHandler) For(featuresHandler *FeaturesHandler) *featureBuilder {
Copy link
Contributor

Choose a reason for hiding this comment

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

For is reborn!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OwnedBy, HandledBy... naming is hard. The idea here is that handler will have an ownership and control over this feature. Somewhat tightly coupled using the builder, but OTOH this makes it easier to... handle... on the consuming side (controllers)

createSpec := func(f *Feature) error {
f.Spec = &Spec{
ServiceMeshSpec: &fo.spec.ServiceMesh,
ServiceMeshSpec: &featuresHandler.DSCInitializationSpec.ServiceMesh,
Serving: &infrav1.ServingSpec{},
Source: &source,
AppNamespace: fo.spec.ApplicationsNamespace,
Source: &featuresHandler.source,
AppNamespace: featuresHandler.DSCInitializationSpec.ApplicationsNamespace,
}

return nil
}

fb := &featureBuilder{
name: fo.name,
fsys: embeddedFiles,
name: u.name,
featuresHanlder: featuresHandler,
fsys: embeddedFiles,
}

// Ensures creation of .Spec object is always invoked first
fb.builders = append([]partialBuilder{createSpec}, fb.builders...)

Expand Down Expand Up @@ -172,28 +162,30 @@ func (fb *featureBuilder) WithResources(resources ...Action) *featureBuilder {
return fb
}

func (fb *featureBuilder) Load() (*Feature, error) {
func (fb *featureBuilder) Load() error {
feature := newFeature(fb.name)

// UsingConfig builder wasn't called while constructing this feature.
// Get default settings and create needed clients.
if fb.config == nil {
if err := fb.withDefaultClient(); err != nil {
return nil, err
return err
}
}

if err := createClients(fb.config)(feature); err != nil {
return nil, err
return err
}

for i := range fb.builders {
if err := fb.builders[i](feature); err != nil {
return nil, err
return err
}
}

return feature, nil
fb.featuresHanlder.features = append(fb.featuresHanlder.features, feature)

return nil
}

func (fb *featureBuilder) withDefaultClient() error {
Expand Down
Loading
Loading