From 2b92731613cf4b47dfc39c800a72f0baba1c21b3 Mon Sep 17 00:00:00 2001 From: Bartosz Majsak Date: Fri, 1 Sep 2023 20:52:46 +0200 Subject: [PATCH] feat: introduces conditional control plane installation (#10) * chore: extracts smcp GVR to a var and reuse it in smcp-related funcs * fix: returns err on oauth creation failure instead of nil by mistake * feat: adds ability to enable feature based on predicates in certain cases this information can only be determined at runtime, for example based on user-defined values of the plugin spec. * feat: introduces control plane installation coupled with wait condition * fix: makes feature enabled by default * chore: reworks smcp creation It relies on owner reference so can be cleaned up as other resources * fix: applies cleanups only if feature is enabled Additionally moves OssmResourceTracker creation to .Apply func * feat: reworks plugin integration to use Apply and Delete funcs of KfApp This way we can cleanup the resources in case deploying service mesh manifests because creating those in Generate phase leaves them hanging due to failing Delete hook * fix(reconcile): corrects KfDef status in case of any error Additionally propagates the error through reconcile to keep trying, as it was ignored. * fix: reworks SMCP component readiness check * chore: improves KfApp func docs * chore: reworks logging in verifiers * chore: simplifies CRD existence check * chore: adds tests for feature enablement * fix: sets interval for SMCP polling to 5s * fix: handles types which are derived for built-in ones for example `type InstallationMode string` was previously failing when converting the default value defined in a custom tag. Now there's a check performed and if value can be convereted to a given type it will. In all other cases it will return an error. * fix: sets default control plane installation mode rely on existing installation Proposed installation modes are: - minimal - pre-installed --- .../v1alpha1/ossm_types.go | 22 ++- ...ossm.plugins.kubeflow.org_ossmplugins.yaml | 8 ++ ...n.internal.kubeflow.org_kfossmplugins.yaml | 5 + .../kfdef_controller.go | 7 +- controllers/kfdef.apps.kubeflow.org/status.go | 6 +- pkg/kfapp/coordinator/coordinator.go | 25 +++- pkg/kfapp/ossm/feature/builder.go | 34 ++++- pkg/kfapp/ossm/feature/cleanup.go | 36 ++--- pkg/kfapp/ossm/feature/feature.go | 26 +++- pkg/kfapp/ossm/feature/resources.go | 7 +- pkg/kfapp/ossm/feature/verification.go | 82 ++++++----- pkg/kfapp/ossm/ossm_installer.go | 67 ++++++++- pkg/kfapp/ossm/ossm_installer_noop.go | 24 ---- .../control-plane/control-plane-minimal.tmpl | 19 +++ pkg/kfconfig/ossmplugin/types.go | 49 +++++-- tests/data/test-data.tar.gz | Bin 681 -> 681 bytes tests/integration/ossm_installer_int_test.go | 132 ++++++++++++++++-- 17 files changed, 415 insertions(+), 134 deletions(-) delete mode 100644 pkg/kfapp/ossm/ossm_installer_noop.go create mode 100644 pkg/kfapp/ossm/templates/control-plane/control-plane-minimal.tmpl diff --git a/apis/ossm.plugins.kubeflow.org/v1alpha1/ossm_types.go b/apis/ossm.plugins.kubeflow.org/v1alpha1/ossm_types.go index 32cb7abef20..b02ef767a5c 100644 --- a/apis/ossm.plugins.kubeflow.org/v1alpha1/ossm_types.go +++ b/apis/ossm.plugins.kubeflow.org/v1alpha1/ossm_types.go @@ -24,11 +24,27 @@ type OssmPluginSpec struct { Auth AuthSpec `json:"auth,omitempty"` } +// InstallationMode defines how the plugin should handle OpenShift Service Mesh installation. +// If not specified `use-existing` is assumed. +type InstallationMode string + +var ( + // PreInstalled indicates that KfDef plugin for Openshift Service Mesh will use existing + // installation and patch Service Mesh Control Plane. + PreInstalled InstallationMode = "pre-installed" + + // Minimal results in installing Openshift Service Mesh Control Plane + // in defined namespace with minimal required configuration. + Minimal InstallationMode = "minimal" +) + // MeshSpec holds information on how Service Mesh should be configured. type MeshSpec struct { - Name string `json:"name,omitempty"` - Namespace string `json:"namespace,omitempty"` - Certificate CertSpec `json:"certificate,omitempty"` + Name string `json:"name,omitempty"` + Namespace string `json:"namespace,omitempty"` + // +kubebuilder:validation:Enum=minimal;pre-installed + InstallationMode InstallationMode `json:"installationMode,omitempty"` + Certificate CertSpec `json:"certificate,omitempty"` } type CertSpec struct { diff --git a/config/crd/bases/ossm.plugins.kubeflow.org_ossmplugins.yaml b/config/crd/bases/ossm.plugins.kubeflow.org_ossmplugins.yaml index 59b16a11552..9c316ab4ea3 100644 --- a/config/crd/bases/ossm.plugins.kubeflow.org_ossmplugins.yaml +++ b/config/crd/bases/ossm.plugins.kubeflow.org_ossmplugins.yaml @@ -62,6 +62,14 @@ spec: name: type: string type: object + installationMode: + description: InstallationMode defines how the plugin should handle + OpenShift Service Mesh installation. If not specified `use-existing` + is assumed. + enum: + - minimal + - pre-installed + type: string name: type: string namespace: diff --git a/config/crd/bases/ossmplugin.internal.kubeflow.org_kfossmplugins.yaml b/config/crd/bases/ossmplugin.internal.kubeflow.org_kfossmplugins.yaml index 832c0ce65a3..837f0fdaf37 100644 --- a/config/crd/bases/ossmplugin.internal.kubeflow.org_kfossmplugins.yaml +++ b/config/crd/bases/ossmplugin.internal.kubeflow.org_kfossmplugins.yaml @@ -65,6 +65,11 @@ spec: name: type: string type: object + installationMode: + description: InstallationMode defines how the plugin should handle + OpenShift Service Mesh installation. If not specified `use-existing` + is assumed. + type: string name: type: string namespace: diff --git a/controllers/kfdef.apps.kubeflow.org/kfdef_controller.go b/controllers/kfdef.apps.kubeflow.org/kfdef_controller.go index ab92f23b11a..e6399c2e719 100644 --- a/controllers/kfdef.apps.kubeflow.org/kfdef_controller.go +++ b/controllers/kfdef.apps.kubeflow.org/kfdef_controller.go @@ -247,12 +247,11 @@ func (r *KfDefReconciler) Reconcile(ctx context.Context, request ctrl.Request) ( } // set status of the KfDef resource - if err := r.reconcileStatus(instance); err != nil { - return ctrl.Result{}, err + if reconcileError := r.reconcileStatus(instance); reconcileError != nil { + return ctrl.Result{}, reconcileError } - // If deployment created successfully - don't requeue - return ctrl.Result{}, nil + return ctrl.Result{}, err } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/kfdef.apps.kubeflow.org/status.go b/controllers/kfdef.apps.kubeflow.org/status.go index 3aca3f9d2ca..23b07408df6 100644 --- a/controllers/kfdef.apps.kubeflow.org/status.go +++ b/controllers/kfdef.apps.kubeflow.org/status.go @@ -40,7 +40,11 @@ func (r *KfDefReconciler) reconcileStatus(cr *kfdefv1.KfDef) error { func getReconcileStatus(cr *kfdefv1.KfDef, err error) error { conditions := []kfdefv1.KfDefCondition{} + availabilityStatus := corev1.ConditionTrue + if err != nil { + availabilityStatus = corev1.ConditionFalse + conditions = append(conditions, kfdefv1.KfDefCondition{ LastUpdateTime: cr.CreationTimestamp, Status: corev1.ConditionTrue, @@ -51,7 +55,7 @@ func getReconcileStatus(cr *kfdefv1.KfDef, err error) error { conditions = append(conditions, kfdefv1.KfDefCondition{ LastUpdateTime: cr.CreationTimestamp, - Status: corev1.ConditionTrue, + Status: availabilityStatus, Reason: DeploymentCompleted, Type: kfdefv1.KfAvailable, }) diff --git a/pkg/kfapp/coordinator/coordinator.go b/pkg/kfapp/coordinator/coordinator.go index cb617866ae6..01d68611d50 100644 --- a/pkg/kfapp/coordinator/coordinator.go +++ b/pkg/kfapp/coordinator/coordinator.go @@ -404,6 +404,22 @@ func (kfapp *coordinator) Apply(resources kftypesv3.ResourceEnum) error { } } + serviceMeshConfig := func() error { + if kfapp.KfDef.Spec.Platform != kftypesv3.OSSM { + return nil + } + + if p, ok := kfapp.Platforms[kfapp.KfDef.Spec.Platform]; !ok { + return &kfapis.KfError{ + Code: int(kfapis.INTERNAL_ERROR), + Message: "Platform OSSM specified but not loaded.", + } + } else { + ossmInstaller := p.(*ossm.OssmInstaller) + return ossmInstaller.Apply(kftypesv3.K8S) + } + } + if err := kfapp.KfDef.SyncCache(); err != nil { return &kfapis.KfError{ Code: int(kfapis.INTERNAL_ERROR), @@ -423,6 +439,9 @@ func (kfapp *coordinator) Apply(resources kftypesv3.ResourceEnum) error { case kftypesv3.PLATFORM: return platform() case kftypesv3.K8S: + if err := serviceMeshConfig(); err != nil { + return err + } if err := k8s(); err != nil { return err } @@ -471,7 +490,7 @@ func (kfapp *coordinator) Delete(resources kftypesv3.ResourceEnum) error { return nil } - ossmCleanup := func() error { + serviceMeshCleanup := func() error { if kfapp.KfDef.Spec.Platform != kftypesv3.OSSM { return nil } @@ -483,7 +502,7 @@ func (kfapp *coordinator) Delete(resources kftypesv3.ResourceEnum) error { } } else { ossmInstaller := p.(*ossm.OssmInstaller) - return ossmInstaller.CleanupResources() + return ossmInstaller.Delete(kftypesv3.K8S) } } @@ -515,7 +534,7 @@ func (kfapp *coordinator) Delete(resources kftypesv3.ResourceEnum) error { if err := k8s(); err != nil { return err } - return ossmCleanup() + return serviceMeshCleanup() } return nil } diff --git a/pkg/kfapp/ossm/feature/builder.go b/pkg/kfapp/ossm/feature/builder.go index 2218b02a76a..adcb1b99b58 100644 --- a/pkg/kfapp/ossm/feature/builder.go +++ b/pkg/kfapp/ossm/feature/builder.go @@ -3,6 +3,7 @@ package feature import ( "github.com/opendatahub-io/opendatahub-operator/pkg/kfconfig/ossmplugin" "github.com/pkg/errors" + apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -53,6 +54,10 @@ func (fb *featureBuilder) UsingConfig(config *rest.Config) *featureBuilder { return errors.WithStack(err) } + if err := apiextv1.AddToScheme(f.client.Scheme()); err != nil { + return err + } + return nil }) @@ -99,6 +104,16 @@ func (fb *featureBuilder) Preconditions(preconditions ...action) *featureBuilder return fb } +func (fb *featureBuilder) Postconditions(postconditions ...action) *featureBuilder { + fb.builders = append(fb.builders, func(f *Feature) error { + f.postconditions = append(f.postconditions, postconditions...) + + return nil + }) + + return fb +} + func (fb *featureBuilder) OnDelete(cleanups ...action) *featureBuilder { fb.builders = append(fb.builders, func(f *Feature) error { f.addCleanup(cleanups...) @@ -119,9 +134,20 @@ func (fb *featureBuilder) WithResources(resources ...action) *featureBuilder { return fb } +func (fb *featureBuilder) EnabledIf(enabled func(f *Feature) bool) *featureBuilder { + fb.builders = append(fb.builders, func(f *Feature) error { + f.Enabled = enabled(f) + + return nil + + }) + return fb +} + func (fb *featureBuilder) Load() (*Feature, error) { feature := &Feature{ - Name: fb.name, + Name: fb.name, + Enabled: true, } for i := range fb.builders { @@ -130,8 +156,10 @@ func (fb *featureBuilder) Load() (*Feature, error) { } } - if err := feature.createResourceTracker(); err != nil { - return nil, err + if feature.Enabled { + if err := feature.createResourceTracker(); err != nil { + return feature, err + } } return feature, nil diff --git a/pkg/kfapp/ossm/feature/cleanup.go b/pkg/kfapp/ossm/feature/cleanup.go index 1928c13afc7..38d5315aaa7 100644 --- a/pkg/kfapp/ossm/feature/cleanup.go +++ b/pkg/kfapp/ossm/feature/cleanup.go @@ -9,19 +9,19 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" ) +var smcpGVR = schema.GroupVersionResource{ + Group: "maistra.io", + Version: "v2", + Resource: "servicemeshcontrolplanes", +} + func RemoveTokenVolumes(feature *Feature) error { tokenVolume := fmt.Sprintf("%s-oauth2-tokens", feature.Spec.AppNamespace) - gvr := schema.GroupVersionResource{ - Group: "maistra.io", - Version: "v2", - Resource: "servicemeshcontrolplanes", - } - meshNs := feature.Spec.Mesh.Namespace meshName := feature.Spec.Mesh.Name - smcp, err := feature.dynamicClient.Resource(gvr).Namespace(meshNs).Get(context.Background(), meshName, metav1.GetOptions{}) + smcp, err := feature.dynamicClient.Resource(smcpGVR).Namespace(meshNs).Get(context.Background(), meshName, metav1.GetOptions{}) if err != nil { return err } @@ -37,7 +37,7 @@ func RemoveTokenVolumes(feature *Feature) error { for i, v := range volumes { volume, ok := v.(map[string]interface{}) if !ok { - fmt.Println("Unexpected type for volume") + log.Info("unexpected type for volume", "type", fmt.Sprintf("%T", volume)) continue } @@ -46,7 +46,7 @@ func RemoveTokenVolumes(feature *Feature) error { return err } if !found { - fmt.Println("No volumeMount found in the volume") + log.Info("no volumeMount found in the volume") continue } @@ -60,12 +60,9 @@ func RemoveTokenVolumes(feature *Feature) error { } } - _, err = feature.dynamicClient.Resource(gvr).Namespace(meshNs).Update(context.Background(), smcp, metav1.UpdateOptions{}) - if err != nil { - return err - } + _, err = feature.dynamicClient.Resource(smcpGVR).Namespace(meshNs).Update(context.Background(), smcp, metav1.UpdateOptions{}) - return nil + return err } func RemoveOAuthClient(feature *Feature) error { @@ -86,6 +83,7 @@ func RemoveOAuthClient(feature *Feature) error { if err := feature.dynamicClient.Resource(gvr).Delete(context.Background(), oauthClientName, metav1.DeleteOptions{}); err != nil { log.Error(err, "failed deleting OAuthClient", "name", oauthClientName) + return err } @@ -95,15 +93,9 @@ func RemoveOAuthClient(feature *Feature) error { func RemoveExtensionProvider(feature *Feature) error { ossmAuthzProvider := fmt.Sprintf("%s-odh-auth-provider", feature.Spec.AppNamespace) - gvr := schema.GroupVersionResource{ - Group: "maistra.io", - Version: "v2", - Resource: "servicemeshcontrolplanes", - } - mesh := feature.Spec.Mesh - smcp, err := feature.dynamicClient.Resource(gvr). + smcp, err := feature.dynamicClient.Resource(smcpGVR). Namespace(mesh.Namespace). Get(context.Background(), mesh.Name, metav1.GetOptions{}) if err != nil { @@ -136,7 +128,7 @@ func RemoveExtensionProvider(feature *Feature) error { } } - _, err = feature.dynamicClient.Resource(gvr). + _, err = feature.dynamicClient.Resource(smcpGVR). Namespace(mesh.Namespace). Update(context.Background(), smcp, metav1.UpdateOptions{}) diff --git a/pkg/kfapp/ossm/feature/feature.go b/pkg/kfapp/ossm/feature/feature.go index b7419221244..6099c28d6cf 100644 --- a/pkg/kfapp/ossm/feature/feature.go +++ b/pkg/kfapp/ossm/feature/feature.go @@ -23,8 +23,9 @@ import ( var log = ctrlLog.Log.WithName("ossm-features") type Feature struct { - Name string - Spec *Spec + Name string + Spec *Spec + Enabled bool clientset *kubernetes.Clientset dynamicClient dynamic.Interface @@ -42,6 +43,13 @@ type Feature struct { type action func(feature *Feature) error func (f *Feature) Apply() error { + + if !f.Enabled { + log.Info("feature is disabled, skipping.", "name", f.Name) + + return nil + } + // Verify all precondition and collect errors var multiErr *multierror.Error for _, precondition := range f.preconditions { @@ -80,12 +88,24 @@ func (f *Feature) Apply() error { return err } - // TODO postconditions + for _, postcondition := range f.postconditions { + multiErr = multierror.Append(multiErr, postcondition(f)) + } + + if multiErr.ErrorOrNil() != nil { + return multiErr.ErrorOrNil() + } return nil } func (f *Feature) Cleanup() error { + if !f.Enabled { + log.Info("feature is disabled, skipping.", "name", f.Name) + + return nil + } + var cleanupErrors *multierror.Error for _, cleanupFunc := range f.cleanups { cleanupErrors = multierror.Append(cleanupErrors, cleanupFunc(f)) diff --git a/pkg/kfapp/ossm/feature/resources.go b/pkg/kfapp/ossm/feature/resources.go index 439b8a78b7e..8d3c97d1d91 100644 --- a/pkg/kfapp/ossm/feature/resources.go +++ b/pkg/kfapp/ossm/feature/resources.go @@ -113,15 +113,16 @@ func ServiceMeshEnabledInDashboard(feature *Feature) error { dashboardConfig := spec["dashboardConfig"].(map[string]interface{}) dashboardConfig["disableServiceMesh"] = false - _, err = feature.dynamicClient.Resource(gvr). + if _, err := feature.dynamicClient.Resource(gvr). Namespace(feature.Spec.AppNamespace). - Update(context.Background(), &config, metav1.UpdateOptions{}) - if err != nil { + Update(context.Background(), &config, metav1.UpdateOptions{}); err != nil { log.Error(err, "Failed to update odhdashboardconfig") + return err } log.Info("Successfully patched odhdashboardconfig") + return nil } diff --git a/pkg/kfapp/ossm/feature/verification.go b/pkg/kfapp/ossm/feature/verification.go index 752de668569..1dfff28b236 100644 --- a/pkg/kfapp/ossm/feature/verification.go +++ b/pkg/kfapp/ossm/feature/verification.go @@ -2,69 +2,77 @@ package feature import ( "context" + "github.com/hashicorp/go-multierror" "github.com/pkg/errors" + apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/dynamic" + "sigs.k8s.io/controller-runtime/pkg/client" + "time" ) -func EnsureCRDIsInstalled(group string, version string, resource string) action { +func EnsureCRDIsInstalled(name string) action { return func(f *Feature) error { - crdGVR := schema.GroupVersionResource{ - Group: group, - Version: version, - Resource: resource, - } - - _, err := f.dynamicClient.Resource(crdGVR).List(context.Background(), metav1.ListOptions{}) - - return err + return f.client.Get(context.Background(), client.ObjectKey{Name: name}, &apiextv1.CustomResourceDefinition{}) } } func EnsureServiceMeshInstalled(feature *Feature) error { - if err := EnsureCRDIsInstalled("maistra.io", "v2", "servicemeshcontrolplanes")(feature); err != nil { - log.Info("Failed to find the pre-requisite SMCP CRD, please ensure OSSM operator is installed.") + if err := EnsureCRDIsInstalled("servicemeshcontrolplanes.maistra.io")(feature); err != nil { + log.Info("Failed to find the pre-requisite Service Mesh Control Plane CRD, please ensure Service Mesh Operator is installed.") + return err } smcp := feature.Spec.Mesh.Name smcpNs := feature.Spec.Mesh.Namespace - status, err := checkSMCPStatus(feature.dynamicClient, smcp, smcpNs) - if err != nil { - log.Info("An error occurred while checking SMCP status - ensure the SMCP referenced exists.") - return err - } - if status != "Ready" { - log.Info("The referenced SMCP is not ready.", "name", smcp, "namespace", smcpNs) - return errors.New("SMCP status is not ready") + ready, err := CheckControlPlaneComponentReadiness(feature.dynamicClient, smcp, smcpNs) + if err != nil || !ready { + log.Error(err, "failed waiting for control plane being ready", "name", smcp, "namespace", smcpNs) + + return multierror.Append(err, errors.New("service mesh control plane is not ready")).ErrorOrNil() } - return nil + return nil } -func checkSMCPStatus(dynamicClient dynamic.Interface, name, namespace string) (string, error) { - gvr := schema.GroupVersionResource{ - Group: "maistra.io", - Version: "v1", - Resource: "servicemeshcontrolplanes", - } +const ( + interval = 5 * time.Second + duration = 5 * time.Minute +) + +func WaitForControlPlaneToBeReady(feature *Feature) error { + return wait.PollImmediate(interval, duration, func() (done bool, err error) { + smcp := feature.Spec.Mesh.Name + smcpNs := feature.Spec.Mesh.Namespace - unstructObj, err := dynamicClient.Resource(gvr).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{}) + log.Info("waiting for control plane components to be ready", "name", smcp, "namespace", smcpNs, "duration (s)", duration.Seconds()) + + return CheckControlPlaneComponentReadiness(feature.dynamicClient, smcp, smcpNs) + }) +} + +func CheckControlPlaneComponentReadiness(dynamicClient dynamic.Interface, smcp, smcpNs string) (bool, error) { + unstructObj, err := dynamicClient.Resource(smcpGVR).Namespace(smcpNs).Get(context.Background(), smcp, metav1.GetOptions{}) if err != nil { - log.Info("Failed to find SMCP") - return "", err + log.Info("failed to find Service Mesh Control Plane", "name", smcp, "namespace", smcpNs) + + return false, err } - conditions, found, err := unstructured.NestedSlice(unstructObj.Object, "status", "conditions") + components, found, err := unstructured.NestedMap(unstructObj.Object, "status", "readiness", "components") if err != nil || !found { - log.Info("status conditions not found or error in parsing of SMCP") - return "", err + log.Info("status conditions not found or error in parsing of Service Mesh Control Plane") + + return false, err } - lastCondition := conditions[len(conditions)-1].(map[string]interface{}) - status := lastCondition["type"].(string) - return status, nil + readyComponents := len(components["ready"].([]interface{})) + pendingComponents := len(components["pending"].([]interface{})) + unreadyComponents := len(components["unready"].([]interface{})) + + return pendingComponents == 0 && unreadyComponents == 0 && readyComponents > 0, nil } diff --git a/pkg/kfapp/ossm/ossm_installer.go b/pkg/kfapp/ossm/ossm_installer.go index 4acfe52e2f5..e191f74e5f8 100644 --- a/pkg/kfapp/ossm/ossm_installer.go +++ b/pkg/kfapp/ossm/ossm_installer.go @@ -58,6 +58,8 @@ func (o *OssmInstaller) GetPluginSpec() (*ossmplugin.OssmPluginSpec, error) { return o.PluginSpec, nil } +// Init performs validation of the plugin spec and ensures all resources, +// such as features and their templates, are processed and initialized. func (o *OssmInstaller) Init(_ kftypesv3.ResourceEnum) error { if o.KfConfig.Spec.SkipInitProject { log.Info("Skipping init phase", "plugin", PluginName) @@ -69,7 +71,9 @@ func (o *OssmInstaller) Init(_ kftypesv3.ResourceEnum) error { return internalError(errors.WithStack(err)) } - pluginSpec.SetDefaults() + if err := pluginSpec.SetDefaults(); err != nil { + return internalError(errors.WithStack(err)) + } if valid, reason := pluginSpec.IsValid(); !valid { return internalError(errors.New(reason)) @@ -89,6 +93,27 @@ func (o *OssmInstaller) enableFeatures() error { return internalError(errors.WithStack(err)) } + if smcpInstallation, err := feature.CreateFeature("control-plane-install-managed"). + For(o.PluginSpec). + UsingConfig(o.config). + Manifests( + path.Join(rootDir, feature.ControlPlaneDir, "control-plane-minimal.tmpl"), + ). + EnabledIf(func(f *feature.Feature) bool { + return f.Spec.Mesh.InstallationMode != ossmplugin.PreInstalled + }). + Preconditions( + feature.EnsureCRDIsInstalled("servicemeshcontrolplanes.maistra.io"), + ). + Postconditions( + feature.WaitForControlPlaneToBeReady, + ). + Load(); err != nil { + return err + } else { + o.features = append(o.features, smcpInstallation) + } + if oauth, err := feature.CreateFeature("control-plane-configure-oauth"). For(o.PluginSpec). UsingConfig(o.config). @@ -103,14 +128,13 @@ func (o *OssmInstaller) enableFeatures() error { ). WithData(feature.ClusterDetails, feature.OAuthConfig). Preconditions( - feature.EnsureCRDIsInstalled("operator.authorino.kuadrant.io", "v1beta1", "authorinos"), feature.EnsureServiceMeshInstalled, ). OnDelete( feature.RemoveOAuthClient, feature.RemoveTokenVolumes, ).Load(); err != nil { - return nil + return err } else { o.features = append(o.features, oauth) } @@ -170,6 +194,10 @@ func (o *OssmInstaller) enableFeatures() error { path.Join(rootDir, feature.AuthDir, "mesh-authz-ext-provider.patch.tmpl"), ). WithData(feature.ClusterDetails). + Preconditions( + feature.EnsureCRDIsInstalled("authconfigs.authorino.kuadrant.io"), + feature.EnsureServiceMeshInstalled, + ). OnDelete(feature.RemoveExtensionProvider). Load(); err != nil { return err @@ -180,7 +208,12 @@ func (o *OssmInstaller) enableFeatures() error { return nil } -func (o *OssmInstaller) Generate(_ kftypesv3.ResourceEnum) error { +// Apply is implemented as a patched logic in coordinator.go similar to already existing GCP one. +// Plugins called from within the Kubernetes are not invoked in this particular phase by default. +// In order to prevent the accumulation of unmanaged resources in the event of a failure, +// the use of Generate() alone is not viable. This is due to its consequence of prematurely halting +// the Delete() chain without invoking our associated Delete hook. +func (o *OssmInstaller) Apply(_ kftypesv3.ResourceEnum) error { var applyErrors *multierror.Error for _, f := range o.features { @@ -191,15 +224,35 @@ func (o *OssmInstaller) Generate(_ kftypesv3.ResourceEnum) error { return applyErrors.ErrorOrNil() } -func (o *OssmInstaller) CleanupResources() error { +// Delete is implemented as a patched logic in coordinator.go similar to the Apply(). +// Plugins called from within the Kubernetes are not invoked in this particular phase by default. +// Because we create resources we need to clean them up on deletion of KfDef though. +func (o *OssmInstaller) Delete(_ kftypesv3.ResourceEnum) error { var cleanupErrors *multierror.Error - for _, f := range o.features { - cleanupErrors = multierror.Append(cleanupErrors, f.Cleanup()) + // Performs cleanups in reverse order (stack), this way e.g. we can unpatch SMCP before deleting it (if managed) + // Though it sounds unnecessary it keeps features isolated and there is no need to rely on the InstallationMode + // between the features when it comes to clean-up. This is based on the assumption, that features + // are created in the correct order or are self-contained. + for i := len(o.features) - 1; i >= 0; i-- { + log.Info("cleanup", "name", o.features[i].Name) + cleanupErrors = multierror.Append(cleanupErrors, o.features[i].Cleanup()) } return cleanupErrors.ErrorOrNil() } +// Generate function is not utilized by the plugin. This is because, in the event of an error, using Generate() +// would lead to the complete omission of the Delete() call. +// This, in turn, would leave resources created by it dangling in the cluster. +func (o *OssmInstaller) Generate(_ kftypesv3.ResourceEnum) error { + return nil +} + +// Dump is unused. Plugins called from within the Kubernetes are not invoked in this particular phase by default. +func (o *OssmInstaller) Dump(_ kftypesv3.ResourceEnum) error { + return nil +} + func internalError(err error) error { return &kfapisv3.KfError{ Code: int(kfapisv3.INTERNAL_ERROR), diff --git a/pkg/kfapp/ossm/ossm_installer_noop.go b/pkg/kfapp/ossm/ossm_installer_noop.go deleted file mode 100644 index b4a515e601c..00000000000 --- a/pkg/kfapp/ossm/ossm_installer_noop.go +++ /dev/null @@ -1,24 +0,0 @@ -package ossm - -import kftypesv3 "github.com/opendatahub-io/opendatahub-operator/apis/apps" - -// Below are the functions which are not used/executed at this point. -// They're here to satisfy the Plugin interface. - -func (o *OssmInstaller) Apply(resources kftypesv3.ResourceEnum) error { - // Plugins invoked within k8s (as a platform) won't be participating in Apply - // This is responsibility of PackageManagers - in this case kustomize - return nil -} - -func (o *OssmInstaller) Delete(resources kftypesv3.ResourceEnum) error { - // Plugins invoked within k8s (as a platform) won't be participating in Delete - // This is responsibility of PackageManagers - in this case kustomize - return nil -} - -func (o *OssmInstaller) Dump(resources kftypesv3.ResourceEnum) error { - // Plugins invoked within k8s (as a platform) won't be participating in Dump - // This is responsibility of PackageManagers - in this case kustomize - return nil -} diff --git a/pkg/kfapp/ossm/templates/control-plane/control-plane-minimal.tmpl b/pkg/kfapp/ossm/templates/control-plane/control-plane-minimal.tmpl new file mode 100644 index 00000000000..d0719fef5f0 --- /dev/null +++ b/pkg/kfapp/ossm/templates/control-plane/control-plane-minimal.tmpl @@ -0,0 +1,19 @@ +apiVersion: maistra.io/v2 +kind: ServiceMeshControlPlane +metadata: + name: {{ .Mesh.Name }} + namespace: {{ .Mesh.Namespace }} +spec: + version: v2.4 + tracing: + type: None + addons: + prometheus: + enabled: false + grafana: + enabled: false + jaeger: + name: jaeger + kiali: + name: kiali + enabled: false diff --git a/pkg/kfconfig/ossmplugin/types.go b/pkg/kfconfig/ossmplugin/types.go index d4d4f9ceed3..d501bcb3a41 100644 --- a/pkg/kfconfig/ossmplugin/types.go +++ b/pkg/kfconfig/ossmplugin/types.go @@ -1,6 +1,7 @@ package ossmplugin import ( + "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "reflect" ) @@ -25,10 +26,25 @@ type OssmPluginSpec struct { AppNamespace string `json:"appNamespace,omitempty"` } +// InstallationMode defines how the plugin should handle OpenShift Service Mesh installation. +// If not specified `use-existing` is assumed. +type InstallationMode string + +var ( + // PreInstalled indicates that KfDef plugin for Openshift Service Mesh will use existing + // installation and patch Service Mesh Control Plane. + PreInstalled InstallationMode = "pre-installed" + + // Minimal results in installing Openshift Service Mesh Control Plane + // in defined namespace with minimal required configuration. + Minimal InstallationMode = "minimal" +) + type MeshSpec struct { - Name string `json:"name,omitempty" default:"basic"` - Namespace string `json:"namespace,omitempty" default:"istio-system"` - Certificate CertSpec `json:"certificate,omitempty"` + Name string `json:"name,omitempty" default:"basic"` + Namespace string `json:"namespace,omitempty" default:"istio-system"` + InstallationMode InstallationMode `json:"installationMode,omitempty" default:"pre-installed"` + Certificate CertSpec `json:"certificate,omitempty"` } type CertSpec struct { @@ -59,11 +75,11 @@ func (plugin *OssmPluginSpec) IsValid() (bool, string) { return true, "" } -func (plugin *OssmPluginSpec) SetDefaults() { - setDefaults(plugin) +func (plugin *OssmPluginSpec) SetDefaults() error { + return setDefaults(plugin) } -func setDefaults(obj interface{}) { +func setDefaults(obj interface{}) error { value := reflect.ValueOf(obj).Elem() typ := value.Type() @@ -71,15 +87,26 @@ func setDefaults(obj interface{}) { field := value.Field(i) tag := typ.Field(i).Tag.Get("default") - if tag != "" && field.CanSet() && isEmptyValue(field) { - defaultValue := reflect.ValueOf(tag) - field.Set(defaultValue) + if field.Kind() == reflect.Struct { + if err := setDefaults(field.Addr().Interface()); err != nil { + return err + } } - if field.Kind() == reflect.Struct { - setDefaults(field.Addr().Interface()) + if tag != "" && field.IsValid() && field.CanSet() && isEmptyValue(field) { + defaultValue := reflect.ValueOf(tag) + targetType := field.Type() + if defaultValue.Type().ConvertibleTo(targetType) { + convertedValue := defaultValue.Convert(targetType) + field.Set(convertedValue) + } else { + return errors.Errorf("unable to convert \"%s\" to %s\n", defaultValue, targetType.Name()) + } } + } + + return nil } func isEmptyValue(value reflect.Value) bool { diff --git a/tests/data/test-data.tar.gz b/tests/data/test-data.tar.gz index 5e3383f05d58a758e83aaa3ca33c545428afed70..1727c8fe39e3bb0b374f88287f166cc6044916fc 100644 GIT binary patch literal 681 zcmV;a0#^MWiwFP!000001MQbxZ<{a_hPmcf9HrNgm=LJAtF~$vYo$q@c3lHbu!3!b zO{MhL&khNNG!^N`(oOSoW7$6b;+*%`_)|rslG7Y3JY|^E<+kd4Br{FCSeO#Ui&?+i zR%HFM-B|xWihtby60@o#9$|(FZ`TP- zeil?h7b%co4qPZm2{4h~l(|uCO_n9e6v=^dP=&U`wZD3VB`pc08^{IaEB6BK8D5j! zT%HLl&qUSzAgAo3iTG`D@Yk2pr(*hmQk`m$+vcP1{g`uWB1KbYM4A-i!N`TQwHfQr zNmYssBV4s@nP5!t8=R7IyNaj?H=8e-)a*`oSAuJ6o!L~~kcl?w+WZ6KHcvh&=&#Om zgO#jgg9q3<|bF2Sz(ATE+w$wu0Y4B_3_am=>UUUPLoov9&G3b>Q$xKhJ zXQLUG2_RjQ#3U3^w__MqS#i^w%T^4kyrUfZW;dS-I>`kiPh)UF|H+#E6x9$(!o51Dlvy;QP{@?fZ zH;m~&2&2RIKML&qe-_^K{vW*khvIv7aXwjgcjrle3-(XJVzF2(7K_DVu~;k?i^XEG PSl-2Nnpc$%04M+eykS_6 literal 681 zcmV;a0#^MWiwFP!000001MQdJZ<{a>$MeizacZ9-kO8T9tF~$nYo$q@_FMx_u!3!b zO{Mg&-#H`{(nMr!mu{Lrc|gYJU%C694{xfe(rTI$O{Sa(w%9Z$?}?AhqsTmidF1z> zJ0?LGev0tthiDgsQ9PeO^sc)&LZdXP0VXM_HC-LI8|(i^@ecc65nh$lB~|qfq44|u zkKa*U|MAmL z{(Jwgn8*|Oym9FrsT@+VpR`s?iUb}5hf_^VXe17RAe<&3^TOQ^31??=cXNM#{q-h+ ziysA3&_xPlM1YVQQVLw=Y{6)L;e7PjEvM*zg$j|JJrRvTajN78jOjlL_w_%+hSmQ$ zxP)u5hNeRQQ_vE+5CB!krp%3EYq}_DrfCjLfG*IE)ZX$57ObR@t|6C9ES*bu;ABO& zbGat0ToY9fgPgL9ChE1x!CPG^kICs{kYcI{oHieQ>&Ki^Qze@^qspWh7e+3mjm=np zPODO`ITgBnmr2g0xFsnqH>=2s3bXm5NzLxCyOIK}F|+Bqp%-Y