Skip to content

Commit

Permalink
feat: reworks Feature to be generic data container
Browse files Browse the repository at this point in the history
Feature Struct Improvements

- Eliminates the `.Spec` field, eliminating a single point of coupling with
  various domain structs used across the project.
- Introduces a generic map to store keys relevant to a given feature,
  which are utilized in templates and other functions.

Feature Builder Changes

- Renamed few functions to better reflect their intent
    - entry builder function `feature.CreateFeature` becomes
  `feature.Define`
   - Load() becomes Create()
- `.WithData` builder function serves as an entry point to define the
  contents of a given feature. This chain method allows adding key-values
  to the feature's context. These values are later used in templates and
  can also be accessed in pre/post conditions and for programmatic
  resource creation.

Additional Changes

- Simplifies and decouples the use of `FeatureHandler`. It is no longer
  necessary to pass it to the Feature builder to group features together.
- Introduces a new `FeatureRegistry` interface, allowing convenient
  addition of feature sets to the handler via the `FeatureProvider` function.
- Introduces a convention for defining `FeatureData` for specific parts of
  the platform (e.g., Serverless or Service Mesh setup). This approach
  enhances readability and promotes the reuse of commonly used data entries.
- Adds a `README.md` file providing a high-level overview of the `Feature DSL`.
  • Loading branch information
bartoszmajsak committed Jun 20, 2024
1 parent 7afb0cf commit 573f99e
Show file tree
Hide file tree
Showing 42 changed files with 1,383 additions and 683 deletions.
1 change: 1 addition & 0 deletions apis/features/v1/features_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ var ConditionReason = struct {
const (
ComponentType OwnerType = "Component"
DSCIType OwnerType = "DSCI"
UnknownType OwnerType = "Unknown"
)

func (s *FeatureTracker) ToOwnerReference() metav1.OwnerReference {
Expand Down
6 changes: 3 additions & 3 deletions components/kserve/kserve.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,10 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
return nil
}

func (k *Kserve) Cleanup(cli client.Client, instance *dsciv1.DSCInitializationSpec) error {
if removeServerlessErr := k.removeServerlessFeatures(instance); removeServerlessErr != nil {
func (k *Kserve) Cleanup(cli client.Client, dscispec *dsciv1.DSCInitializationSpec) error {
if removeServerlessErr := k.removeServerlessFeatures(dscispec); removeServerlessErr != nil {
return removeServerlessErr
}

return k.removeServiceMeshConfigurations(cli, instance)
return k.removeServiceMeshConfigurations(cli, dscispec)
}
12 changes: 6 additions & 6 deletions components/kserve/kserve_config_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,24 +114,24 @@ func (k *Kserve) setDefaultDeploymentMode(ctx context.Context, cli client.Client
return nil
}

func (k *Kserve) configureServerless(_ client.Client, instance *dsciv1.DSCInitializationSpec) error {
func (k *Kserve) configureServerless(_ client.Client, dscispec *dsciv1.DSCInitializationSpec) error {
switch k.Serving.ManagementState {
case operatorv1.Unmanaged: // Bring your own CR
fmt.Println("Serverless CR is not configured by the operator, we won't do anything")

case operatorv1.Removed: // we remove serving CR
fmt.Println("existing Serverless CR (owned by operator) will be removed")
if err := k.removeServerlessFeatures(instance); err != nil {
if err := k.removeServerlessFeatures(dscispec); err != nil {
return err
}

case operatorv1.Managed: // standard workflow to create CR
switch instance.ServiceMesh.ManagementState {
switch dscispec.ServiceMesh.ManagementState {
case operatorv1.Unmanaged, operatorv1.Removed:
return fmt.Errorf("ServiceMesh is need to set to 'Managed' in DSCI CR, it is required by KServe serving field")
}

serverlessFeatures := feature.ComponentFeaturesHandler(k.GetComponentName(), instance, k.configureServerlessFeatures())
serverlessFeatures := feature.ComponentFeaturesHandler(k.GetComponentName(), dscispec.ApplicationsNamespace, k.configureServerlessFeatures(dscispec))

if err := serverlessFeatures.Apply(); err != nil {
return err
Expand All @@ -140,8 +140,8 @@ func (k *Kserve) configureServerless(_ client.Client, instance *dsciv1.DSCInitia
return nil
}

func (k *Kserve) removeServerlessFeatures(instance *dsciv1.DSCInitializationSpec) error {
serverlessFeatures := feature.ComponentFeaturesHandler(k.GetComponentName(), instance, k.configureServerlessFeatures())
func (k *Kserve) removeServerlessFeatures(dscispec *dsciv1.DSCInitializationSpec) error {
serverlessFeatures := feature.ComponentFeaturesHandler(k.GetComponentName(), dscispec.ApplicationsNamespace, k.configureServerlessFeatures(dscispec))

return serverlessFeatures.Delete()
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ metadata:
namespace: {{ .ControlPlane.Namespace }}
spec:
provider:
name: {{ .AppNamespace }}-auth-provider
name: {{ .AuthExtensionName }}
66 changes: 27 additions & 39 deletions components/kserve/serverless_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,24 @@ package kserve
import (
"path"

dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/serverless"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/servicemesh"
)

func (k *Kserve) configureServerlessFeatures() feature.FeaturesProvider {
return func(handler *feature.FeaturesHandler) error {
servingDeploymentErr := feature.CreateFeature("serverless-serving-deployment").
For(handler).
func (k *Kserve) configureServerlessFeatures(dsciSpec *dsciv1.DSCInitializationSpec) feature.FeaturesProvider {
return func(registry feature.FeaturesRegistry) error {
servingDeployment := feature.Define("serverless-serving-deployment").
ManifestsLocation(Resources.Location).
Manifests(
path.Join(Resources.InstallDir),
).
WithData(PopulateComponentSettings(k)).
WithData(
serverless.FeatureData.IngressDomain.Create(&k.Serving).AsAction(),
serverless.FeatureData.Serving.Create(&k.Serving).AsAction(),
servicemesh.FeatureData.ControlPlane.Create(dsciSpec).AsAction(),
).
PreConditions(
serverless.EnsureServerlessOperatorInstalled,
serverless.EnsureServerlessAbsent,
Expand All @@ -25,53 +29,37 @@ func (k *Kserve) configureServerlessFeatures() feature.FeaturesProvider {
).
PostConditions(
feature.WaitForPodsToBeReady(serverless.KnativeServingNamespace),
).
Load()
if servingDeploymentErr != nil {
return servingDeploymentErr
}
)

servingNetIstioSecretFilteringErr := feature.CreateFeature("serverless-net-istio-secret-filtering").
For(handler).
istioSecretFiltering := feature.Define("serverless-net-istio-secret-filtering").
ManifestsLocation(Resources.Location).
Manifests(
path.Join(Resources.BaseDir, "serving-net-istio-secret-filtering.patch.tmpl.yaml"),
).
WithData(PopulateComponentSettings(k)).
WithData(serverless.FeatureData.Serving.Create(&k.Serving).AsAction()).
PreConditions(serverless.EnsureServerlessServingDeployed).
PostConditions(
feature.WaitForPodsToBeReady(serverless.KnativeServingNamespace),
).
Load()
if servingNetIstioSecretFilteringErr != nil {
return servingNetIstioSecretFilteringErr
}
)

serverlessGwErr := feature.CreateFeature("serverless-serving-gateways").
For(handler).
PreConditions(serverless.EnsureServerlessServingDeployed).
WithData(
PopulateComponentSettings(k),
serverless.ServingDefaultValues,
serverless.ServingIngressDomain,
).
WithResources(serverless.ServingCertificateResource).
servingGateway := feature.Define("serverless-serving-gateways").
ManifestsLocation(Resources.Location).
Manifests(
path.Join(Resources.GatewaysDir),
).
Load()
if serverlessGwErr != nil {
return serverlessGwErr
}

return nil
}
}
WithData(
serverless.FeatureData.IngressDomain.Create(&k.Serving).AsAction(),
serverless.FeatureData.Certificate.Create(&k.Serving).AsAction(),
serverless.FeatureData.Serving.Create(&k.Serving).AsAction(),
servicemesh.FeatureData.ControlPlane.Create(dsciSpec).AsAction(),
).
WithResources(serverless.ServingCertificateResource).
PreConditions(serverless.EnsureServerlessServingDeployed)

func PopulateComponentSettings(k *Kserve) feature.Action {
return func(f *feature.Feature) error {
f.Spec.Serving = &k.Serving
return nil
return registry.Add(
servingDeployment,
istioSecretFiltering,
servingGateway,
)
}
}
38 changes: 18 additions & 20 deletions components/kserve/servicemesh_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,44 +13,49 @@ import (
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/servicemesh"
)

func (k *Kserve) configureServiceMesh(c client.Client, dscispec *dsciv1.DSCInitializationSpec) error {
func (k *Kserve) configureServiceMesh(cli client.Client, dscispec *dsciv1.DSCInitializationSpec) error {
if dscispec.ServiceMesh != nil {
if dscispec.ServiceMesh.ManagementState == operatorv1.Managed && k.GetManagementState() == operatorv1.Managed {
serviceMeshInitializer := feature.ComponentFeaturesHandler(k.GetComponentName(), dscispec, k.defineServiceMeshFeatures(c))
serviceMeshInitializer := feature.ComponentFeaturesHandler(k.GetComponentName(), dscispec.ApplicationsNamespace, k.defineServiceMeshFeatures(cli, dscispec))
return serviceMeshInitializer.Apply()
}
if dscispec.ServiceMesh.ManagementState == operatorv1.Unmanaged && k.GetManagementState() == operatorv1.Managed {
return nil
}
}

return k.removeServiceMeshConfigurations(c, dscispec)
return k.removeServiceMeshConfigurations(cli, dscispec)
}

func (k *Kserve) removeServiceMeshConfigurations(cli client.Client, dscispec *dsciv1.DSCInitializationSpec) error {
serviceMeshInitializer := feature.ComponentFeaturesHandler(k.GetComponentName(), dscispec, k.defineServiceMeshFeatures(cli))
serviceMeshInitializer := feature.ComponentFeaturesHandler(k.GetComponentName(), dscispec.ApplicationsNamespace, k.defineServiceMeshFeatures(cli, dscispec))
return serviceMeshInitializer.Delete()
}

func (k *Kserve) defineServiceMeshFeatures(cli client.Client) feature.FeaturesProvider {
return func(handler *feature.FeaturesHandler) error {
func (k *Kserve) defineServiceMeshFeatures(cli client.Client, dscispec *dsciv1.DSCInitializationSpec) feature.FeaturesProvider {
return func(registry feature.FeaturesRegistry) error {
authorinoInstalled, err := cluster.SubscriptionExists(cli, "authorino-operator")
if err != nil {
return fmt.Errorf("failed to list subscriptions %w", err)
}

if authorinoInstalled {
kserveExtAuthzErr := feature.CreateFeature("kserve-external-authz").
For(handler).
kserveExtAuthzErr := registry.Add(feature.Define("kserve-external-authz").
ManifestsLocation(Resources.Location).
Manifests(
path.Join(Resources.ServiceMeshDir, "activator-envoyfilter.tmpl.yaml"),
path.Join(Resources.ServiceMeshDir, "envoy-oauth-temp-fix.tmpl.yaml"),
path.Join(Resources.ServiceMeshDir, "kserve-predictor-authorizationpolicy.tmpl.yaml"),
path.Join(Resources.ServiceMeshDir, "z-migrations"),
).
WithData(servicemesh.ClusterDetails).
Load()
WithData(
feature.Entry("Domain", cluster.GetDomain),
servicemesh.FeatureData.ControlPlane.Create(dscispec).AsAction(),
).
WithData(
servicemesh.FeatureData.Authorization.All(dscispec)...,
),
)

if kserveExtAuthzErr != nil {
return kserveExtAuthzErr
Expand All @@ -59,19 +64,12 @@ func (k *Kserve) defineServiceMeshFeatures(cli client.Client) feature.FeaturesPr
fmt.Println("WARN: Authorino operator is not installed on the cluster, skipping authorization capability")
}

temporaryFixesErr := feature.CreateFeature("kserve-temporary-fixes").
For(handler).
return registry.Add(feature.Define("kserve-temporary-fixes").
ManifestsLocation(Resources.Location).
Manifests(
path.Join(Resources.ServiceMeshDir, "grpc-envoyfilter-temp-fix.tmpl.yaml"),
).
WithData(servicemesh.ClusterDetails).
Load()

if temporaryFixesErr != nil {
return temporaryFixesErr
}

return nil
WithData(servicemesh.FeatureData.ControlPlane.Create(dscispec).AsAction()),
)
}
}
4 changes: 2 additions & 2 deletions controllers/dscinitialization/monitoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func configureAlertManager(ctx context.Context, dsciInit *dsci.DSCInitialization
// r.Log.Info("Success: inject alertmanage-configs.yaml")

// special handling for dev-mod
consolelinkDomain, err := cluster.GetDomain(r.Client)
consolelinkDomain, err := cluster.GetDomain(ctx, r.Client)
if err != nil {
return fmt.Errorf("error getting console route URL : %w", err)
}
Expand Down Expand Up @@ -208,7 +208,7 @@ func configurePrometheus(ctx context.Context, dsciInit *dsci.DSCInitialization,
return err
}
// Update prometheus-config for dashboard, dsp and workbench
consolelinkDomain, err := cluster.GetDomain(r.Client)
consolelinkDomain, err := cluster.GetDomain(ctx, r.Client)
if err != nil {
return fmt.Errorf("error getting console route URL : %w", err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: maistra.io/v1
kind: ServiceMeshMember
metadata:
name: default
namespace: {{ .Auth.Namespace }}
namespace: {{ .AuthNamespace }}
spec:
controlPlaneRef:
namespace: {{ .ControlPlane.Namespace }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: operator.authorino.kuadrant.io/v1beta1
kind: Authorino
metadata:
name: {{ .AuthProviderName }}
namespace: {{ .Auth.Namespace }}
namespace: {{ .AuthNamespace }}
spec:
authConfigLabelSelectors: security.opendatahub.io/authorization-group=default
clusterWide: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: apps/v1
kind: Deployment
metadata:
name: {{ .AuthProviderName }}
namespace: {{ .Auth.Namespace }}
namespace: {{ .AuthNamespace }}
spec:
template:
metadata:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ spec:
techPreview:
meshConfig:
extensionProviders:
- name: {{ .AppNamespace }}-auth-provider
- name: {{ .AuthExtensionName }}
envoyExtAuthzGrpc:
service: {{ .AuthProviderName }}-authorino-authorization.{{ .Auth.Namespace }}.svc.cluster.local
service: {{ .AuthProviderName }}-authorino-authorization.{{ .AuthNamespace }}.svc.cluster.local
port: 50051
Loading

0 comments on commit 573f99e

Please sign in to comment.