Skip to content

Commit

Permalink
Fixes
Browse files Browse the repository at this point in the history 
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
  • Loading branch information
@tolusha
tolusha committed Oct 13, 2021
1 parent d9ccde4 commit 3064209
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 93 deletions.
46 changes: 22 additions & 24 deletions controllers/che/checluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,12 @@ type CheClusterReconciler struct {
nonCachedClient client.Client
// A discovery client to check for the existence of certain APIs registered
// in the API Server
discoveryClient discovery.DiscoveryInterface
tests bool
userHandler OpenShiftOAuthUserHandler
discoveryClient discovery.DiscoveryInterface
tests bool
userHandler OpenShiftOAuthUserHandler
reconcileManager *deploy.ReconcileManager
// the namespace to which to limit the reconciliation. If empty, all namespaces are considered
namespace string
syncManager *deploy.SyncManager
namespace string
}

// NewReconciler returns a new CheClusterReconciler
Expand All @@ -96,22 +96,22 @@ func NewReconciler(
scheme *k8sruntime.Scheme,
namespace string) *CheClusterReconciler {

syncManager := deploy.NewSyncManager()
reconcileManager := deploy.NewReconcileManager()

// order does matter
imagePuller := deploy.NewImagePuller()
imagePuller.Register(syncManager)
imagePuller.Register(reconcileManager)

return &CheClusterReconciler{
Scheme: scheme,
Log: ctrl.Log.WithName("controllers").WithName("CheCluster"),

client: k8sclient,
nonCachedClient: noncachedClient,
discoveryClient: discoveryClient,
userHandler: NewOpenShiftOAuthUserHandler(noncachedClient),
namespace: namespace,
syncManager: syncManager,
client: k8sclient,
nonCachedClient: noncachedClient,
discoveryClient: discoveryClient,
userHandler: NewOpenShiftOAuthUserHandler(noncachedClient),
namespace: namespace,
reconcileManager: reconcileManager,
}
}

Expand Down Expand Up @@ -279,18 +279,16 @@ func (r *CheClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
}

if deployContext.CheCluster.ObjectMeta.DeletionTimestamp.IsZero() {
result, err := r.syncManager.SyncAll(deployContext)
if result != nil {
return *result, err
}
// if result == nil {
// logrus.Info("Successfully reconciled.")
// return ctrl.Result{}, nil
// } else {
// return *result, err
// }
result, done, err := r.reconcileManager.ReconcileAll(deployContext)
if !done {
return result, err
// TODO: uncomment when all items added to ReconcilerManager
// } else {
// logrus.Info("Successfully reconciled.")
// return ctrl.Result{}, nil
}
} else {
r.syncManager.FinalizeAll(deployContext)
r.reconcileManager.FinalizeAll(deployContext)
}

// Reconcile finalizers before CR is deleted
Expand Down
69 changes: 37 additions & 32 deletions pkg/deploy/kubernetes_image_puller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,32 +52,32 @@ func NewImagePuller() *ImagePuller {
return &ImagePuller{}
}

func (ip *ImagePuller) Register(sm *SyncManager) {
sm.RegisterSyncer(ip)
func (ip *ImagePuller) Register(sm *ReconcileManager) {
sm.RegisterReconciler(ip)
}

func (ip *ImagePuller) Sync(ctx *DeployContext) (*reconcile.Result, error) {
func (ip *ImagePuller) Reconcile(ctx *DeployContext) (reconcile.Result, bool, error) {
return ReconcileImagePuller(ctx)
}

func (ip *ImagePuller) Finalize(ctx *DeployContext) (*reconcile.Result, error) {
func (ip *ImagePuller) Finalize(ctx *DeployContext) (bool, error) {
return DeleteImagePullerOperatorAndFinalizer(ctx)
}

// Reconcile the imagePuller section of the CheCluster CR. If imagePuller.enable is set to true, install the Kubernetes Image Puller operator and create
// a KubernetesImagePuller CR. Add a finalizer to the CheCluster CR. If false, remove the KubernetesImagePuller CR, uninstall the operator, and remove the finalizer.
func ReconcileImagePuller(ctx *DeployContext) (*reconcile.Result, error) {
func ReconcileImagePuller(ctx *DeployContext) (reconcile.Result, bool, error) {
// Determine what server groups the API Server knows about
foundPackagesAPI, foundOperatorsAPI, _, err := CheckNeededImagePullerApis(ctx)
if err != nil {
logrus.Errorf("Error discovering image puller APIs: %v", err)
return &reconcile.Result{}, err
return reconcile.Result{}, false, err
}

// If the image puller should be installed but the APIServer doesn't know about PackageManifests/Subscriptions, log a warning and requeue
if ctx.CheCluster.Spec.ImagePuller.Enable && (!foundPackagesAPI || !foundOperatorsAPI) {
logrus.Infof("Couldn't find Operator Lifecycle Manager types to install the Kubernetes Image Puller Operator. Please install Operator Lifecycle Manager to install the operator or disable the image puller by setting spec.imagePuller.enable to false.")
return &reconcile.Result{RequeueAfter: time.Second}, nil
return reconcile.Result{RequeueAfter: time.Second}, false, nil
}

if ctx.CheCluster.Spec.ImagePuller.Enable {
Expand All @@ -86,42 +86,42 @@ func ReconcileImagePuller(ctx *DeployContext) (*reconcile.Result, error) {
if err != nil {
if errors.IsNotFound(err) {
logrus.Infof("There is no PackageManifest for the Kubernetes Image Puller Operator. Install the Operator Lifecycle Manager and the Community Operators Catalog")
return &reconcile.Result{RequeueAfter: time.Second}, nil
return reconcile.Result{RequeueAfter: time.Second}, false, nil
}
logrus.Errorf("Error getting packagemanifest: %v", err)
return &reconcile.Result{}, err
return reconcile.Result{}, false, err
}

createdOperatorGroup, err := CreateOperatorGroupIfNotFound(ctx)
if err != nil {
logrus.Infof("Error creating OperatorGroup: %v", err)
return &reconcile.Result{}, err
return reconcile.Result{}, false, err
}
if createdOperatorGroup {
return &reconcile.Result{RequeueAfter: time.Second}, nil
return reconcile.Result{RequeueAfter: time.Second}, false, nil
}
createdOperatorSubscription, err := CreateImagePullerSubscription(ctx, packageManifest)
if err != nil {
logrus.Infof("Error creating Subscription: %v", err)
return &reconcile.Result{}, err
return reconcile.Result{}, false, err
}
if createdOperatorSubscription {
return &reconcile.Result{RequeueAfter: time.Second}, nil
return reconcile.Result{RequeueAfter: time.Second}, false, nil
}

// Add the image puller finalizer
if !HasImagePullerFinalizer(ctx.CheCluster) {
if err := ReconcileImagePullerFinalizer(ctx); err != nil {
return &reconcile.Result{}, err
return reconcile.Result{}, false, err
}
return &reconcile.Result{RequeueAfter: time.Second}, nil
return reconcile.Result{RequeueAfter: time.Second}, false, nil
}
}

_, _, foundKubernetesImagePullerAPI, err := CheckNeededImagePullerApis(ctx)
if err != nil {
logrus.Errorf("Error discovering image puller APIs: %v", err)
return &reconcile.Result{}, err
return reconcile.Result{}, false, err
}
// If the KubernetesImagePuller API service exists, attempt to reconcile creation/update
if foundKubernetesImagePullerAPI {
Expand All @@ -139,35 +139,35 @@ func ReconcileImagePuller(ctx *DeployContext) (*reconcile.Result, error) {
_, err := UpdateImagePullerSpecIfEmpty(ctx)
if err != nil {
logrus.Errorf("Error updating CheCluster: %v", err)
return &reconcile.Result{}, err
return reconcile.Result{}, false, err
}
return &reconcile.Result{RequeueAfter: time.Second}, nil
return reconcile.Result{RequeueAfter: time.Second}, false, nil
}

if ctx.CheCluster.IsImagePullerImagesEmpty() {
if err = SetDefaultImages(ctx); err != nil {
logrus.Error(err)
return &reconcile.Result{}, err
return reconcile.Result{}, false, err
}
}

logrus.Infof("Creating KubernetesImagePuller for CheCluster %v", ctx.CheCluster.Name)
createdImagePuller, err := CreateKubernetesImagePuller(ctx)
if err != nil {
logrus.Error("Error creating KubernetesImagePuller: ", err)
return &reconcile.Result{}, err
return reconcile.Result{}, false, err
}
if createdImagePuller {
return &reconcile.Result{}, nil
return reconcile.Result{}, false, nil
}
}
logrus.Errorf("Error getting KubernetesImagePuller: %v", err)
return &reconcile.Result{}, err
return reconcile.Result{}, false, err
}

if err = UpdateDefaultImagesIfNeeded(ctx); err != nil {
logrus.Error(err)
return &reconcile.Result{}, err
return reconcile.Result{}, false, err
}

if ctx.CheCluster.Spec.ImagePuller.Spec.DeploymentName == "" {
Expand All @@ -186,38 +186,43 @@ func ReconcileImagePuller(ctx *DeployContext) (*reconcile.Result, error) {
logrus.Infof("Updating KubernetesImagePuller %v", imagePuller.Name)
if err = ctx.ClusterAPI.Client.Update(context.TODO(), imagePuller, &client.UpdateOptions{}); err != nil {
logrus.Errorf("Error updating KubernetesImagePuller: %v", err)
return &reconcile.Result{}, err
return reconcile.Result{}, false, err
}
return &reconcile.Result{RequeueAfter: time.Second}, nil
return reconcile.Result{RequeueAfter: time.Second}, false, nil
}
} else {
logrus.Infof("Waiting 15 seconds for kubernetesimagepullers.che.eclipse.org API")
return &reconcile.Result{RequeueAfter: 15 * time.Second}, nil
return reconcile.Result{RequeueAfter: 15 * time.Second}, false, nil
}

} else {
if foundOperatorsAPI && foundPackagesAPI {
return DeleteImagePullerOperatorAndFinalizer(ctx)
done, err := DeleteImagePullerOperatorAndFinalizer(ctx)
if !done {
return reconcile.Result{}, false, err
} else {
return reconcile.Result{}, true, nil
}
}
}
return nil, nil
return reconcile.Result{}, true, nil
}

func DeleteImagePullerOperatorAndFinalizer(ctx *DeployContext) (*reconcile.Result, error) {
func DeleteImagePullerOperatorAndFinalizer(ctx *DeployContext) (bool, error) {
if _, err := GetImagePullerOperator(ctx); err == nil {
if _, err := UninstallImagePullerOperator(ctx); err != nil {
logrus.Errorf("Error uninstalling Image Puller: %v", err)
return &reconcile.Result{}, err
return false, err
}
}
if HasImagePullerFinalizer(ctx.CheCluster) {
if err := DeleteImagePullerFinalizer(ctx); err != nil {
logrus.Errorf("Error deleting finalizer: %v", err)
return &reconcile.Result{}, err
return false, err
}
}

return nil, nil
return true, nil
}

func HasImagePullerFinalizer(instance *orgv1.CheCluster) bool {
Expand Down
2 changes: 1 addition & 1 deletion pkg/deploy/kubernetes_image_puller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ func TestImagePullerConfiguration(t *testing.T) {
t.Fatalf("Error reconciling: %v", err)
}
} else {
_, err = ReconcileImagePuller(deployContext)
_, _, err = ReconcileImagePuller(deployContext)
if err != nil {
t.Fatalf("Error reconciling: %v", err)
}
Expand Down
74 changes: 38 additions & 36 deletions pkg/deploy/syncmanager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,67 +12,69 @@
package deploy

import (
"reflect"
"runtime"

"github.com/sirupsen/logrus"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

type Syncable interface {
// Syncs object to a cluster.
// Returns nil,nil if object successfully reconciled.
Sync(ctx *DeployContext) (*reconcile.Result, error)
type Reconcilable interface {
// Reconcile object.
Reconcile(ctx *DeployContext) (result reconcile.Result, done bool, err error)
// Does finalization (removes cluster scope objects, etc)
Finalize(ctx *DeployContext) (*reconcile.Result, error)
Finalize(ctx *DeployContext) (done bool, err error)
// Registration
Register(sm *SyncManager)
Register(reconcileManager *ReconcileManager)
}

type SyncManager struct {
syncers []Syncable
failedSyncer Syncable
type ReconcileManager struct {
reconcilers []Reconcilable
failedReconciler *Reconcilable
}

func NewSyncManager() *SyncManager {
return &SyncManager{
syncers: make([]Syncable, 0),
failedSyncer: nil,
func NewReconcileManager() *ReconcileManager {
return &ReconcileManager{
reconcilers: make([]Reconcilable, 0),
failedReconciler: nil,
}
}

func (sm *SyncManager) RegisterSyncer(syncer Syncable) {
sm.syncers = append(sm.syncers, syncer)
func (rm *ReconcileManager) RegisterReconciler(reconciler Reconcilable) {
rm.reconcilers = append(rm.reconcilers, reconciler)
}

// Sync all objects consequently.
func (sm *SyncManager) SyncAll(ctx *DeployContext) (*reconcile.Result, error) {
for _, syncer := range sm.syncers {
result, err := syncer.Sync(ctx)
// Reconcile all objects in a order they have been added
// If reconciliation failed then CheCluster status will be updated accordingly.
func (rm *ReconcileManager) ReconcileAll(ctx *DeployContext) (reconcile.Result, bool, error) {
for _, reconciler := range rm.reconcilers {
result, done, err := reconciler.Reconcile(ctx)
if err != nil {
sm.failedSyncer = syncer

err = SetStatusDetails(ctx, InstallOrUpdateFailed, err.Error(), "")
logrus.Errorf("Failed to update checluster status, cause: %v", err)
} else if sm.failedSyncer == syncer {
sm.failedSyncer = nil

err = SetStatusDetails(ctx, "", "", "")
if err != nil {
return &reconcile.Result{Requeue: true}, err
rm.failedReconciler = &reconciler
if err := SetStatusDetails(ctx, InstallOrUpdateFailed, err.Error(), ""); err != nil {
logrus.Errorf("Failed to update checluster status, cause: %v", err)
}
} else if rm.failedReconciler == &reconciler {
rm.failedReconciler = nil
if err := SetStatusDetails(ctx, "", "", ""); err != nil {
logrus.Errorf("Failed to update checluster status, cause: %v", err)
}
}

if result != nil {
return result, err
if !done {
return result, done, err
}
}

return nil, nil
return reconcile.Result{}, true, nil
}

func (sm *SyncManager) FinalizeAll(ctx *DeployContext) {
for _, syncer := range sm.syncers {
_, err := syncer.Finalize(ctx)
func (sm *ReconcileManager) FinalizeAll(ctx *DeployContext) {
for _, reconciler := range sm.reconcilers {
_, err := reconciler.Finalize(ctx)
if err != nil {
logrus.Errorf("Failed to finalize, cause: %v", err)
reconcilerName := runtime.FuncForPC(reflect.ValueOf(reconciler).Pointer()).Name()
logrus.Errorf("Finalization failed for reconciler: `%s`, cause: %v", reconcilerName, err)
}
}
}

0 comments on commit 3064209

Please sign in to comment.