Skip to content

Commit

Permalink
feat: adds finalizers to DSC and DSCI (opendatahub-io#661)
Browse files Browse the repository at this point in the history
- rename k8serrors to apierrs for consistency
- Use controllerutil methods add and remove finalizer

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
Co-authored-by: Wen Zhou <wenzhou@redhat.com>
  • Loading branch information
bartoszmajsak and zdtsw authored Oct 25, 2023
1 parent 14e2a04 commit 4869de8
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 73 deletions.
1 change: 1 addition & 0 deletions components/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ can be found [here](https://github.com/opendatahub-io/opendatahub-operator/tree/
```go
type ComponentInterface interface {
ReconcileComponent(cli client.Client, owner metav1.Object, DSCISpec *dsci.DSCInitializationSpec) error
Cleanup(cli client.Client, DSCISpec *dsci.DSCInitializationSpec) error
GetComponentName() string
GetManagementState() operatorv1.ManagementState
SetImageParamsMap(imageMap map[string]string) map[string]string
Expand Down
6 changes: 6 additions & 0 deletions components/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ func (c *Component) GetManagementState() operatorv1.ManagementState {
return c.ManagementState
}

func (c *Component) Cleanup(_ client.Client, _ *dsci.DSCInitializationSpec) error {
// noop
return nil
}

// DevFlags defines list of fields that can be used by developers to test customizations. This is not recommended
// to be used in production environment.
type DevFlags struct {
Expand Down Expand Up @@ -60,6 +65,7 @@ type ManifestsConfig struct {

type ComponentInterface interface {
ReconcileComponent(cli client.Client, owner metav1.Object, DSCISpec *dsci.DSCInitializationSpec) error
Cleanup(cli client.Client, DSCISpec *dsci.DSCInitializationSpec) error
GetComponentName() string
GetManagementState() operatorv1.ManagementState
SetImageParamsMap(imageMap map[string]string) map[string]string
Expand Down
117 changes: 69 additions & 48 deletions controllers/datasciencecluster/datasciencecluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"errors"
"fmt"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade"
apierrs "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/client-go/rest"
"reflect"
"time"
Expand Down Expand Up @@ -52,6 +51,8 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"

"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"sigs.k8s.io/controller-runtime/pkg/predicate"
)

Expand All @@ -71,6 +72,10 @@ type DataScienceClusterConfig struct {
DSCISpec *dsci.DSCInitializationSpec
}

const (
finalizerName = "datasciencecluster.opendatahub.io/finalizer"
)

// Reconcile is part of the main kubernetes reconciliation loop which aims to
// move the current state of the cluster closer to the desired state.
func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
Expand Down Expand Up @@ -112,29 +117,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
}

var err error
// Start reconciling
if instance.Status.Conditions == nil {
reason := status.ReconcileInit
message := "Initializing DataScienceCluster resource"
instance, err = r.updateStatus(ctx, instance, func(saved *dsc.DataScienceCluster) {
status.SetProgressingCondition(&saved.Status.Conditions, reason, message)
saved.Status.Phase = status.PhaseProgressing
})
if err != nil {
_ = r.reportError(err, instance, fmt.Sprintf("failed to add conditions to status of DataScienceCluster resource on namespace %s and name %s", req.Namespace, req.Name))
return ctrl.Result{}, err
}
}

// If Deletion configmap is found, reconcile to trigger operatorUninstall
if upgrade.HasDeleteConfigMap(r.Client) {
if err := r.Client.Delete(context.TODO(), instance); err != nil {
if !apierrs.IsNotFound(err) {
return reconcile.Result{}, err
}
}
return reconcile.Result{Requeue: true}, nil
}
// Verify a valid DSCInitialization instance is created
dsciInstances := &dsci.DSCInitializationList{}
err = r.Client.List(ctx, dsciInstances)
Expand Down Expand Up @@ -166,6 +149,50 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
return ctrl.Result{}, errors.New("only one instance of DSCInitialization object is allowed")
}

allComponents, err := getAllComponents(&instance.Spec.Components)
if err != nil {
return ctrl.Result{}, err
}

if instance.ObjectMeta.DeletionTimestamp.IsZero() {
if !controllerutil.ContainsFinalizer(instance, finalizerName) {
r.Log.Info("Adding finalizer for DataScienceCluster", "name", instance.Name, "namespace", instance.Namespace, "finalizer", finalizerName)
controllerutil.AddFinalizer(instance, finalizerName)
if err := r.Update(ctx, instance); err != nil {
return ctrl.Result{}, err
}
}
} else {
r.Log.Info("Finalization DataScienceCluster start deleting instance", "name", instance.Name, "namespace", instance.Namespace, "finalizer", finalizerName)
for _, component := range allComponents {
if err := component.Cleanup(r.Client, r.DataScienceCluster.DSCISpec); err != nil {
return ctrl.Result{}, err
}
}
if controllerutil.ContainsFinalizer(instance, finalizerName) {
controllerutil.RemoveFinalizer(instance, finalizerName)
if err := r.Update(ctx, instance); err != nil {
return ctrl.Result{}, err
}
}

return ctrl.Result{}, nil
}

// Start reconciling
if instance.Status.Conditions == nil {
reason := status.ReconcileInit
message := "Initializing DataScienceCluster resource"
instance, err = r.updateStatus(ctx, instance, func(saved *dsc.DataScienceCluster) {
status.SetProgressingCondition(&saved.Status.Conditions, reason, message)
saved.Status.Phase = status.PhaseProgressing
})
if err != nil {
_ = r.reportError(err, instance, fmt.Sprintf("failed to add conditions to status of DataScienceCluster resource on namespace %s and name %s", req.Namespace, req.Name))
return ctrl.Result{}, err
}
}

// Ensure all omitted components show up as explicitly disabled
instance, err = r.updateComponents(ctx, instance)
if err != nil {
Expand All @@ -176,14 +203,8 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
// Initialize error list, instead of returning errors after every component is deployed
var componentErrors *multierror.Error

allComponents, err := getAllComponents(&instance.Spec.Components)
if err != nil {
return ctrl.Result{}, err
}

for _, component := range allComponents {
if instance, err = r.reconcileSubComponent(ctx, instance, component); err != nil {
// no need to log any errors as this is done in the reconcileSubComponent method
componentErrors = multierror.Append(componentErrors, err)
}
}
Expand Down Expand Up @@ -222,25 +243,6 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
return ctrl.Result{}, nil
}

func getAllComponents(c *dsc.Components) ([]components.ComponentInterface, error) {
var allComponents []components.ComponentInterface

definedComponents := reflect.ValueOf(c).Elem()
for i := 0; i < definedComponents.NumField(); i++ {
c := definedComponents.Field(i)
if c.CanAddr() {
component, ok := c.Addr().Interface().(components.ComponentInterface)
if !ok {
return allComponents, errors.New("this is not a pointer to ComponentInterface")
}

allComponents = append(allComponents, component)
}
}

return allComponents, nil
}

func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context, instance *dsc.DataScienceCluster,
component components.ComponentInterface,
) (*dsc.DataScienceCluster, error) {
Expand Down Expand Up @@ -397,3 +399,22 @@ func (r *DataScienceClusterReconciler) watchDataScienceClusterResources(a client
}
return nil
}

func getAllComponents(c *dsc.Components) ([]components.ComponentInterface, error) {
var allComponents []components.ComponentInterface

definedComponents := reflect.ValueOf(c).Elem()
for i := 0; i < definedComponents.NumField(); i++ {
c := definedComponents.Field(i)
if c.CanAddr() {
component, ok := c.Addr().Interface().(components.ComponentInterface)
if !ok {
return allComponents, errors.New("this is not a pointer to ComponentInterface")
}

allComponents = append(allComponents, component)
}
}

return allComponents, nil
}
65 changes: 40 additions & 25 deletions controllers/dscinitialization/dscinitialization_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,34 @@ import (
"context"
"errors"
"fmt"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade"
"path/filepath"

"github.com/go-logr/logr"
operatorv1 "github.com/openshift/api/operator/v1"

"github.com/go-logr/logr"
dsci "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/controllers/status"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
netv1 "k8s.io/api/networking/v1"
authv1 "k8s.io/api/rbac/v1"
apierrs "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/record"
"k8s.io/client-go/util/retry"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

const (
finalizerName = "dscinitialization.opendatahub.io/finalizer"
)

// DSCInitializationReconciler reconciles a DSCInitialization object.
type DSCInitializationReconciler struct {
client.Client
Expand All @@ -63,36 +67,47 @@ type DSCInitializationReconciler struct {
func (r *DSCInitializationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
r.Log.Info("Reconciling DSCInitialization.", "DSCInitialization", req.Namespace, "Request.Name", req.Name)

instance := &dsci.DSCInitialization{}
// First check if instance is being deleted, return
if instance.GetDeletionTimestamp() != nil {
return ctrl.Result{}, nil
}

// Second check if instance exists, return
err := r.Client.Get(ctx, req.NamespacedName, instance)
if err != nil {
if apierrs.IsNotFound(err) {
// DSCInitialization instance not found
return ctrl.Result{}, nil
}
instances := &dsci.DSCInitializationList{}
if err := r.Client.List(ctx, instances); err != nil {
r.Log.Error(err, "Failed to retrieve DSCInitialization resource.", "DSCInitialization", req.Namespace, "Request.Name", req.Name)
r.Recorder.Eventf(instance, corev1.EventTypeWarning, "DSCInitializationReconcileError", "Failed to retrieve DSCInitialization instance")
return ctrl.Result{}, err
}

// Last check if multiple instances of DSCInitialization exist
instanceList := &dsci.DSCInitializationList{}
err = r.Client.List(ctx, instanceList)
if err != nil {
r.Recorder.Eventf(instances, corev1.EventTypeWarning, "DSCInitializationReconcileError", "Failed to retrieve DSCInitialization instance")
return ctrl.Result{}, err
}

if len(instanceList.Items) > 1 {
if len(instances.Items) > 1 {
message := fmt.Sprintf("only one instance of DSCInitialization object is allowed. Update existing instance on namespace %s and name %s", req.Namespace, req.Name)

return ctrl.Result{}, errors.New(message)
}

if len(instances.Items) == 0 {
return ctrl.Result{}, nil
}

instance := &instances.Items[0]

if instance.ObjectMeta.DeletionTimestamp.IsZero() {
if !controllerutil.ContainsFinalizer(instance, finalizerName) {
r.Log.Info("Adding finalizer for DSCInitialization", "name", instance.Name, "namespace", instance.Namespace, "finalizer", finalizerName)
controllerutil.AddFinalizer(instance, finalizerName)
if err := r.Update(ctx, instance); err != nil {
return ctrl.Result{}, err
}
}
} else {
r.Log.Info("Finalization DSCInitialization start deleting instance", "name", instance.Name, "namespace", instance.Namespace, "finalizer", finalizerName)
// Add cleanup logic here
if controllerutil.ContainsFinalizer(instance, finalizerName) {
controllerutil.RemoveFinalizer(instance, finalizerName)
if err := r.Update(ctx, instance); err != nil {
return ctrl.Result{}, err
}
}

return ctrl.Result{}, nil
}

var err error
// Start reconciling
if instance.Status.Conditions == nil {
reason := status.ReconcileInit
Expand Down

0 comments on commit 4869de8

Please sign in to comment.