Skip to content

Commit

Permalink
feat: reworks Feature to be generic data container (opendatahub-io#1052)
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 patss 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 authored Jul 10, 2024
1 parent 022f456 commit 973ff74
Show file tree
Hide file tree
Showing 38 changed files with 1,400 additions and 691 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
4 changes: 2 additions & 2 deletions components/kserve/kserve_config_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func (k *Kserve) configureServerless(ctx context.Context, cli client.Client, ins
return dependOpsErrors
}

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

if err := serverlessFeatures.Apply(ctx); err != nil {
return err
Expand All @@ -150,7 +150,7 @@ func (k *Kserve) configureServerless(ctx context.Context, cli client.Client, ins
}

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

return serverlessFeatures.Delete(ctx)
}
Expand Down
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 }}
67 changes: 27 additions & 40 deletions components/kserve/serverless_setup.go
Original file line number Diff line number Diff line change
@@ -1,23 +1,26 @@
package kserve

import (
"context"
"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.Define(&k.Serving).AsAction(),
serverless.FeatureData.Serving.Define(&k.Serving).AsAction(),
servicemesh.FeatureData.ControlPlane.Define(dsciSpec).AsAction(),
).
PreConditions(
serverless.EnsureServerlessOperatorInstalled,
serverless.EnsureServerlessAbsent,
Expand All @@ -26,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.Define(&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.Define(&k.Serving).AsAction(),
serverless.FeatureData.CertificateName.Define(&k.Serving).AsAction(),
serverless.FeatureData.Serving.Define(&k.Serving).AsAction(),
servicemesh.FeatureData.ControlPlane.Define(dsciSpec).AsAction(),
).
WithResources(serverless.ServingCertificateResource).
PreConditions(serverless.EnsureServerlessServingDeployed)

func PopulateComponentSettings(k *Kserve) feature.Action {
return func(_ context.Context, 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 @@ -17,7 +17,7 @@ import (
func (k *Kserve) configureServiceMesh(ctx context.Context, 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(ctx, cli))
serviceMeshInitializer := feature.ComponentFeaturesHandler(k.GetComponentName(), dscispec.ApplicationsNamespace, k.defineServiceMeshFeatures(ctx, cli, dscispec))
return serviceMeshInitializer.Apply(ctx)
}
if dscispec.ServiceMesh.ManagementState == operatorv1.Unmanaged && k.GetManagementState() == operatorv1.Managed {
Expand All @@ -29,30 +29,35 @@ func (k *Kserve) configureServiceMesh(ctx context.Context, cli client.Client, ds
}

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

func (k *Kserve) defineServiceMeshFeatures(ctx context.Context, cli client.Client) feature.FeaturesProvider {
return func(handler *feature.FeaturesHandler) error {
func (k *Kserve) defineServiceMeshFeatures(ctx context.Context, cli client.Client, dscispec *dsciv1.DSCInitializationSpec) feature.FeaturesProvider {
return func(registry feature.FeaturesRegistry) error {
authorinoInstalled, err := cluster.SubscriptionExists(ctx, 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).
Managed().
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()
Managed().
WithData(
feature.Entry("Domain", cluster.GetDomain),
servicemesh.FeatureData.ControlPlane.Define(dscispec).AsAction(),
).
WithData(
servicemesh.FeatureData.Authorization.All(dscispec)...,
),
)

if kserveExtAuthzErr != nil {
return kserveExtAuthzErr
Expand All @@ -61,20 +66,13 @@ func (k *Kserve) defineServiceMeshFeatures(ctx context.Context, cli client.Clien
fmt.Println("WARN: Authorino operator is not installed on the cluster, skipping authorization capability")
}

temporaryFixesErr := feature.CreateFeature("kserve-temporary-fixes").
For(handler).
Managed().
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
Managed().
WithData(servicemesh.FeatureData.ControlPlane.Define(dscispec).AsAction()),
)
}
}
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 973ff74

Please sign in to comment.