Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCPVE-309: Remove commented code #337

Merged
merged 4 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
9 changes: 2 additions & 7 deletions api/v1alpha1/webhook_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"testing"
"time"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

admissionv1beta1 "k8s.io/api/admission/v1beta1"
Expand All @@ -35,7 +35,6 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"
"sigs.k8s.io/controller-runtime/pkg/envtest/printer"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
)
Expand All @@ -51,10 +50,6 @@ var cancel context.CancelFunc

func TestAPIs(t *testing.T) {
RegisterFailHandler(Fail)

RunSpecsWithDefaultAndCustomReporters(t,
suleymanakbas91 marked this conversation as resolved.
Show resolved Hide resolved
"Webhook Suite",
[]Reporter{printer.NewlineReporter{}})
}

var _ = BeforeSuite(func() {
Expand Down Expand Up @@ -125,7 +120,7 @@ var _ = BeforeSuite(func() {
return nil
}).Should(Succeed())

}, 60)
})

var _ = AfterSuite(func() {
cancel()
Expand Down
5 changes: 0 additions & 5 deletions controllers/lvm_volumegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
49 changes: 13 additions & 36 deletions controllers/lvmcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,19 @@ const (
openshiftSCCPrivilegedName = "privileged"
)

// NOTE: when updating this, please also update docs/design/lvm-operator-manager.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 (
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -335,25 +331,6 @@ func (r *LVMClusterReconciler) getExpectedVgCount(ctx context.Context, instance
return vgCount, nil
}

// NOTE: when updating this, please also update docs/design/lvm-operator-manager.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 {
Expand Down
2 changes: 1 addition & 1 deletion controllers/lvmcluster_controller_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"context"
"time"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
lvmv1alpha1 "github.com/openshift/lvm-operator/api/v1alpha1"
appsv1 "k8s.io/api/apps/v1"
Expand Down
5 changes: 0 additions & 5 deletions controllers/scc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
9 changes: 2 additions & 7 deletions controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,14 @@ import (
"path/filepath"
"testing"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"
"sigs.k8s.io/controller-runtime/pkg/envtest/printer"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"

Expand All @@ -53,10 +52,6 @@ var (

func TestAPIs(t *testing.T) {
RegisterFailHandler(Fail)

RunSpecsWithDefaultAndCustomReporters(t,
"Controller Suite",
[]Reporter{printer.NewlineReporter{}})
}

const (
Expand Down Expand Up @@ -128,7 +123,7 @@ var _ = BeforeSuite(func() {
Expect(err).ToNot(HaveOccurred(), "failed to run manager")
}()

}, 60)
})

var _ = AfterSuite(func() {
cancel()
Expand Down
5 changes: 0 additions & 5 deletions controllers/topolvm_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions controllers/topolvm_csi_driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions controllers/topolvm_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions controllers/topolvm_snapshotclass.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}

Expand Down
5 changes: 0 additions & 5 deletions controllers/topolvm_storageclass.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 0 additions & 4 deletions controllers/vgmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 0 additions & 6 deletions docs/design/lvm-operator-manager.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,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
}
```
Loading