From 6f3220b02191dbd8aaeaa0c5931a196ca8ed5b3d Mon Sep 17 00:00:00 2001 From: qJkee Date: Wed, 3 May 2023 12:53:24 -0400 Subject: [PATCH] OCPVE-309: remove updateStatus function from resourceManager interface --- controllers/lvm_volumegroup.go | 5 --- controllers/lvmcluster_controller.go | 49 ++++++++-------------------- controllers/scc.go | 5 --- controllers/topolvm_controller.go | 5 --- controllers/topolvm_csi_driver.go | 5 --- controllers/topolvm_node.go | 8 ----- controllers/topolvm_snapshotclass.go | 5 --- controllers/topolvm_storageclass.go | 5 --- controllers/vgmanager.go | 4 --- docs/design/operator.md | 6 ---- 10 files changed, 13 insertions(+), 84 deletions(-) diff --git a/controllers/lvm_volumegroup.go b/controllers/lvm_volumegroup.go index 0df4bf26d..3ae72928c 100644 --- a/controllers/lvm_volumegroup.go +++ b/controllers/lvm_volumegroup.go @@ -115,11 +115,6 @@ func (c lvmVG) ensureDeleted(r *LVMClusterReconciler, ctx context.Context, lvmCl return nil } -func (c lvmVG) updateStatus(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error { - // intentionally empty - return nil -} - func (c lvmVG) getLvmVolumeGroups(r *LVMClusterReconciler, instance *lvmv1alpha1.LVMCluster) []*lvmv1alpha1.LVMVolumeGroup { lvmVolumeGroups := []*lvmv1alpha1.LVMVolumeGroup{} diff --git a/controllers/lvmcluster_controller.go b/controllers/lvmcluster_controller.go index efe8dd7b0..1f3458353 100644 --- a/controllers/lvmcluster_controller.go +++ b/controllers/lvmcluster_controller.go @@ -48,6 +48,19 @@ const ( openshiftSCCPrivilegedName = "privileged" ) +// NOTE: when updating this, please also update doc/design/operator.md +type resourceManager interface { + + // getName should return a camelCase name of this unit of reconciliation + getName() string + + // ensureCreated should check the resources managed by this unit + ensureCreated(*LVMClusterReconciler, context.Context, *lvmv1alpha1.LVMCluster) error + + // ensureDeleted should wait for the resources to be cleaned up + ensureDeleted(*LVMClusterReconciler, context.Context, *lvmv1alpha1.LVMCluster) error +} + type ClusterType string const ( @@ -199,23 +212,6 @@ func (r *LVMClusterReconciler) reconcile(ctx context.Context, instance *lvmv1alp } } - /* // check and report deployment status - var failedStatusUpdates []string - var lastError error - for _, unit := range resourceList { - err := unit.updateStatus(r, ctx, instance) - if err != nil { - failedStatusUpdates = append(failedStatusUpdates, unit.getName()) - unitError := fmt.Errorf("failed updating status for: %s %w", unit.getName(), err) - r.Log.Error(unitError, "") - } - } */ - /* // return simple message that will fit in status reconcileSucceeded condition, don't put all the errors there - if len(failedStatusUpdates) > 0 { - return ctrl.Result{}, fmt.Errorf("status update failed for %s: %w", strings.Join(failedStatusUpdates, ","), lastError) - } - */ - r.Log.Info("successfully reconciled LvmCluster") return ctrl.Result{}, nil @@ -335,25 +331,6 @@ func (r *LVMClusterReconciler) getExpectedVgCount(ctx context.Context, instance return vgCount, nil } -// NOTE: when updating this, please also update docs/design/operator.md -type resourceManager interface { - - // getName should return a camelCase name of this unit of reconciliation - getName() string - - // ensureCreated should check the resources managed by this unit - ensureCreated(*LVMClusterReconciler, context.Context, *lvmv1alpha1.LVMCluster) error - - // ensureDeleted should wait for the resources to be cleaned up - ensureDeleted(*LVMClusterReconciler, context.Context, *lvmv1alpha1.LVMCluster) error - - // updateStatus should optionally update the CR's status about the health of the managed resource - // each unit will have updateStatus called individually so - // avoid status fields like lastHeartbeatTime and have a - // status that changes only when the operands change. - updateStatus(*LVMClusterReconciler, context.Context, *lvmv1alpha1.LVMCluster) error -} - // checkIfOpenshift checks to see if the operator is running on an OCP cluster. // It does this by querying for the "privileged" SCC which exists on all OCP clusters. func (r *LVMClusterReconciler) checkIfOpenshift(ctx context.Context) error { diff --git a/controllers/scc.go b/controllers/scc.go index 158777e00..a809f12c5 100644 --- a/controllers/scc.go +++ b/controllers/scc.go @@ -86,11 +86,6 @@ func (c openshiftSccs) ensureDeleted(r *LVMClusterReconciler, ctx context.Contex return nil } -func (c openshiftSccs) updateStatus(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error { - // intentionally empty - return nil -} - func getAllSCCs(namespace string) []*secv1.SecurityContextConstraints { return []*secv1.SecurityContextConstraints{ newTopolvmNodeScc(namespace), diff --git a/controllers/topolvm_controller.go b/controllers/topolvm_controller.go index 298811a98..e1b90a38c 100644 --- a/controllers/topolvm_controller.go +++ b/controllers/topolvm_controller.go @@ -101,11 +101,6 @@ func (c topolvmController) ensureDeleted(r *LVMClusterReconciler, ctx context.Co return err } -func (c topolvmController) updateStatus(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error { - // TODO: Verify the status of controller plugin deployment and set the same on CR - return nil -} - func (c topolvmController) setTopolvmControllerDesiredState(existing, desired *appsv1.Deployment) error { // at creation, deep copy desired deployment into existing diff --git a/controllers/topolvm_csi_driver.go b/controllers/topolvm_csi_driver.go index cd286746a..477785016 100644 --- a/controllers/topolvm_csi_driver.go +++ b/controllers/topolvm_csi_driver.go @@ -89,11 +89,6 @@ func (c csiDriver) ensureDeleted(r *LVMClusterReconciler, ctx context.Context, l return err } -func (c csiDriver) updateStatus(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error { - // intentionally empty as there'll be no status field on CSIDriver resource - return nil -} - func getCSIDriverResource() *storagev1.CSIDriver { // topolvm doesn't use/need attacher and reduce a round trip of the rpc by setting this to false attachRequired := false diff --git a/controllers/topolvm_node.go b/controllers/topolvm_node.go index 20b4681cb..363cd437d 100644 --- a/controllers/topolvm_node.go +++ b/controllers/topolvm_node.go @@ -123,14 +123,6 @@ func (n topolvmNode) ensureDeleted(r *LVMClusterReconciler, ctx context.Context, return nil } -// updateStatus should optionally update the CR's status about the health of the managed resource -// each unit will have updateStatus called individually so -// avoid status fields like lastHeartbeatTime and have a -// status that changes only when the operands change. -func (n topolvmNode) updateStatus(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error { - return nil -} - func getNodeDaemonSet(lvmCluster *lvmv1alpha1.LVMCluster, namespace string, initImage string) *appsv1.DaemonSet { hostPathDirectory := corev1.HostPathDirectory diff --git a/controllers/topolvm_snapshotclass.go b/controllers/topolvm_snapshotclass.go index f967ae743..aa0102da8 100644 --- a/controllers/topolvm_snapshotclass.go +++ b/controllers/topolvm_snapshotclass.go @@ -96,11 +96,6 @@ func (s topolvmVolumeSnapshotClass) ensureDeleted(r *LVMClusterReconciler, ctx c return nil } -func (s topolvmVolumeSnapshotClass) updateStatus(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error { - // intentionally empty as there'll be no status field on VolumeSnapshotClass resource - return nil -} - func getTopolvmSnapshotClasses(lvmCluster *lvmv1alpha1.LVMCluster) []*snapapi.VolumeSnapshotClass { vsc := []*snapapi.VolumeSnapshotClass{} diff --git a/controllers/topolvm_storageclass.go b/controllers/topolvm_storageclass.go index 8c2d47be8..bfc6ca19a 100644 --- a/controllers/topolvm_storageclass.go +++ b/controllers/topolvm_storageclass.go @@ -96,11 +96,6 @@ func (s topolvmStorageClass) ensureDeleted(r *LVMClusterReconciler, ctx context. return nil } -func (s topolvmStorageClass) updateStatus(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error { - // intentionally empty as there'll be no status field on StorageClass resource - return nil -} - func getTopolvmStorageClasses(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) []*storagev1.StorageClass { const defaultSCAnnotation string = "storageclass.kubernetes.io/is-default-class" diff --git a/controllers/vgmanager.go b/controllers/vgmanager.go index 492414c2d..49dcbe731 100644 --- a/controllers/vgmanager.go +++ b/controllers/vgmanager.go @@ -118,10 +118,6 @@ func (v vgManager) ensureDeleted(r *LVMClusterReconciler, ctx context.Context, l return nil } -func (v vgManager) updateStatus(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error { - return nil -} - func initMapIfNil(m *map[string]string) { if len(*m) > 1 { return diff --git a/docs/design/operator.md b/docs/design/operator.md index 0f4e0fc65..fe70dea6f 100644 --- a/docs/design/operator.md +++ b/docs/design/operator.md @@ -31,11 +31,5 @@ type resourceManager interface { // ensureDeleted should wait for the resources to be cleaned up ensureDeleted(*LVMClusterReconciler, context.Context, lvmv1alpha1.LVMCluster) error - - // updateStatus should optionally update the CR's status about the health of the managed resource - // each unit will have updateStatus called induvidually so - // avoid status fields like lastHeartbeatTime and have a - // status that changes only when the operands change. - updateStatus(*LVMClusterReconciler, context.Context, lvmv1alpha1.LVMCluster) error } ```