OCPVE-346: chore: move all controllers under internal/controllers

.mockery.yaml

@@ -4,15 +4,15 @@ dir: "{{.InterfaceDir}}/mocks"
 mockname: "{{.Mock}}{{.InterfaceName}}"
 outpkg: "{{.PackageName}}"
 packages:
-  github.com/openshift/lvm-operator/pkg/lsblk:
-      interfaces:
-        LSBLK:
-  github.com/openshift/lvm-operator/pkg/lvm:
-      interfaces:
-        LVM:
-  github.com/openshift/lvm-operator/pkg/wipefs:
-      interfaces:
-        Wipefs:
-  github.com/openshift/lvm-operator/pkg/dmsetup:
+  github.com/openshift/lvm-operator/internal/controllers/vgmanager/lsblk:
+    interfaces:
+      LSBLK:
+  github.com/openshift/lvm-operator/internal/controllers/vgmanager/lvm:
+    interfaces:
+      LVM:
+  github.com/openshift/lvm-operator/internal/controllers/vgmanager/wipefs:
+    interfaces:
+      Wipefs:
+  github.com/openshift/lvm-operator/internal/controllers/vgmanager/dmsetup:
     interfaces:
       Dmsetup:

Dockerfile

@@ -17,12 +17,12 @@ RUN go mod verify
 
 # Copy the go source
 COPY api/ api/
-COPY controllers/ controllers/
 COPY cmd/ cmd/
-COPY pkg/ pkg/
+COPY internal/ internal/
 
 # Build
-RUN CGO_ENABLED=0 GOOS=$TARGETOS GOARCH=$TARGETARCH go build -mod=vendor --ldflags "-s -w" -a -o lvms cmd/main.go
+RUN --mount=type=cache,target=/root/.cache/go-build \
+    GOOS=$TARGETOS GOARCH=$TARGETARCH go build -mod=vendor --ldflags "-s -w" -a -o lvms cmd/main.go
 
 # vgmanager needs 'nsenter' and other basic linux utils to correctly function
 FROM --platform=$TARGETPLATFORM registry.access.redhat.com/ubi9/ubi-minimal:9.2

api/v1alpha1/lvmcluster_test.go

@@ -7,7 +7,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"github.com/openshift/lvm-operator/pkg/cluster"
+	"github.com/openshift/lvm-operator/internal/cluster"
 
 	v1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"

api/v1alpha1/lvmcluster_webhook.go

@@ -22,7 +22,7 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/openshift/lvm-operator/pkg/cluster"
+	"github.com/openshift/lvm-operator/internal/cluster"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"

cmd/operator/operator.go

@@ -21,7 +21,10 @@ import (
 	"fmt"
 
 	"github.com/go-logr/logr"
-	"github.com/openshift/lvm-operator/controllers/node"
+	"github.com/openshift/lvm-operator/internal/controllers/lvmcluster"
+	"github.com/openshift/lvm-operator/internal/controllers/node"
+	"github.com/openshift/lvm-operator/internal/controllers/persistent-volume"
+	"github.com/openshift/lvm-operator/internal/controllers/persistent-volume-claim"
 	"github.com/spf13/cobra"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -42,10 +45,7 @@ import (
 	"github.com/openshift/library-go/pkg/config/leaderelection"
 
 	lvmv1alpha1 "github.com/openshift/lvm-operator/api/v1alpha1"
-	"github.com/openshift/lvm-operator/controllers"
-	persistent_volume "github.com/openshift/lvm-operator/controllers/persistent-volume"
-	persistent_volume_claim "github.com/openshift/lvm-operator/controllers/persistent-volume-claim"
-	"github.com/openshift/lvm-operator/pkg/cluster"
+	"github.com/openshift/lvm-operator/internal/cluster"
 	//+kubebuilder:scaffold:imports
 )
 
@@ -158,9 +158,8 @@ func run(cmd *cobra.Command, _ []string, opts *Options) error {
 	}
 
 	// register controllers
-	if err = (&controllers.LVMClusterReconciler{
+	if err = (&lvmcluster.LVMClusterReconciler{
 		Client:                           mgr.GetClient(),
-		Scheme:                           mgr.GetScheme(),
 		EventRecorder:                    mgr.GetEventRecorderFor("LVMClusterReconciler"),
 		ClusterType:                      clusterType,
 		Namespace:                        operatorNamespace,

cmd/vgmanager/vgmanager.go

@@ -21,13 +21,13 @@ import (
 	"os"
 
 	"github.com/go-logr/logr"
-	"github.com/openshift/lvm-operator/pkg/dmsetup"
-	"github.com/openshift/lvm-operator/pkg/filter"
-	"github.com/openshift/lvm-operator/pkg/lsblk"
-	"github.com/openshift/lvm-operator/pkg/lvm"
-	"github.com/openshift/lvm-operator/pkg/lvmd"
-	"github.com/openshift/lvm-operator/pkg/vgmanager"
-	"github.com/openshift/lvm-operator/pkg/wipefs"
+	"github.com/openshift/lvm-operator/internal/controllers/vgmanager"
+	"github.com/openshift/lvm-operator/internal/controllers/vgmanager/dmsetup"
+	"github.com/openshift/lvm-operator/internal/controllers/vgmanager/filter"
+	"github.com/openshift/lvm-operator/internal/controllers/vgmanager/lsblk"
+	"github.com/openshift/lvm-operator/internal/controllers/vgmanager/lvm"
+	"github.com/openshift/lvm-operator/internal/controllers/vgmanager/lvmd"
+	"github.com/openshift/lvm-operator/internal/controllers/vgmanager/wipefs"
 	"github.com/spf13/cobra"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"

docs/design/lvm-operator-manager.md

@@ -61,7 +61,7 @@ The Operator requires elevated permissions to interact with the host's LVM comma
 
 ## Implementation Notes
 
-Each unit of reconciliation should implement the `reconcileUnit` interface. This is run by the controller. Errors and success messages are propagated as Operator status and events. This interface is defined in [lvmcluster_controller.go](../../controllers/lvmcluster_controller.go)
+Each unit of reconciliation should implement the `reconcileUnit` interface. This is run by the controller. Errors and success messages are propagated as Operator status and events. This interface is defined in [lvmcluster_controller.go](../../internal/controllers/lvmcluster/lvmcluster_controller.go)
 
 ```go
 type resourceManager interface {

docs/troubleshooting.md

@@ -63,7 +63,7 @@ If you encounter a failure message such as `no available devices found` while in
 $ lsblk --paths --json -o NAME,ROTA,TYPE,SIZE,MODEL,VENDOR,RO,STATE,KNAME,SERIAL,PARTLABEL,FSTYPE
 ```
 
-This prints information about the disks on the host. Review this information to see why a device is not considered available for LVMS utilization. For example, if a device has partlabel `bios` or `reserved`, or if they are suspended or read-only, or if they have children disks or `fstype` set, LVMS considers them unavailable. Check [filter.go](../pkg/vgmanager/filter.go) for the complete list of filters LVMS makes use of.
+This prints information about the disks on the host. Review this information to see why a device is not considered available for LVMS utilization. For example, if a device has partlabel `bios` or `reserved`, or if they are suspended or read-only, or if they have children disks or `fstype` set, LVMS considers them unavailable. Check [filter.go](../internal/controllers/vgmanager/filter/filter.go) for the complete list of filters LVMS makes use of.
 
 > If you set a device path in the `LVMCluster` CR under `spec.storage.deviceClasses.deviceSelector.paths`, make sure the paths match with `kname` of the device from the `lsblk` output.
 

controllers/constants.go → internal/controllers/constants/constants.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package controllers
+package constants
 
 const (
 	TopolvmCSIDriverName = "topolvm.io"
@@ -25,7 +25,6 @@ const (
 
 	// CSI Controller container and deployment names
 	TopolvmControllerDeploymentName = "topolvm-controller"
-	TopolvmControllerContainerName  = "topolvm-controller"
 	CsiResizerContainerName         = "csi-resizer"
 	CsiProvisionerContainerName     = "csi-provisioner"
 	CsiSnapshotterContainerName     = "csi-snapshotter"
@@ -68,10 +67,6 @@ const (
 	FileCheckerMemRequest = "10Mi"
 	FileCheckerCPURequest = "1m"
 
-	// CSI Provisioner requires below environment values to make use of CSIStorageCapacity
-	PodNameEnv   = "POD_NAME"
-	NameSpaceEnv = "NAMESPACE"
-
 	// topoLVM Node
 	TopolvmNodeServiceAccount       = "topolvm-node"
 	TopolvmNodeDaemonsetName        = "topolvm-node"
@@ -89,9 +84,9 @@ const (
 	// annotations
 
 	// WorkloadPartitioningManagement contains the management workload annotation
-	workloadPartitioningManagementAnnotation = "target.workload.openshift.io/management"
+	WorkloadPartitioningManagementAnnotation = "target.workload.openshift.io/management"
 
-	managementAnnotationVal = `{"effect": "PreferredDuringScheduling"}`
+	ManagementAnnotationVal = `{"effect": "PreferredDuringScheduling"}`
 
 	// labels and values
 
@@ -112,6 +107,6 @@ const (
 	CsiDriverNameVal          = "topolvm-csi-driver"
 
 	StorageClassPrefix        = "lvms-"
-	volumeSnapshotClassPrefix = "lvms-"
-	sccPrefix                 = "lvms-"
+	VolumeSnapshotClassPrefix = "lvms-"
+	SCCPrefix                 = "lvms-"
 )

controllers/lvmcluster_controller.go → internal/controllers/lvmcluster/lvmcluster_controller.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package controllers
+package lvmcluster
 
 import (
 	"context"
@@ -25,13 +25,14 @@ import (
 
 	configv1 "github.com/openshift/api/config/v1"
 	lvmv1alpha1 "github.com/openshift/lvm-operator/api/v1alpha1"
-	"github.com/openshift/lvm-operator/pkg/cluster"
+	"github.com/openshift/lvm-operator/internal/cluster"
+	"github.com/openshift/lvm-operator/internal/controllers/constants"
+	"github.com/openshift/lvm-operator/internal/controllers/lvmcluster/resource"
 
 	topolvmv1 "github.com/topolvm/topolvm/api/v1"
 
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	corev1helper "k8s.io/component-helpers/scheduling/corev1"
@@ -54,24 +55,10 @@ const (
 	lvmClusterFinalizer = "lvmcluster.topolvm.io"
 )
 
-// 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
-}
-
 // LVMClusterReconciler reconciles a LVMCluster object
 type LVMClusterReconciler struct {
 	client.Client
 	record.EventRecorder
-	Scheme             *runtime.Scheme
 	ClusterType        cluster.Type
 	EnableSnapshotting bool
 	Namespace          string
@@ -82,6 +69,26 @@ type LVMClusterReconciler struct {
 	TopoLVMLeaderElectionPassthrough configv1.LeaderElection
 }
 
+func (r *LVMClusterReconciler) GetNamespace() string {
+	return r.Namespace
+}
+
+func (r *LVMClusterReconciler) GetImageName() string {
+	return r.ImageName
+}
+
+func (r *LVMClusterReconciler) GetClient() client.Client {
+	return r.Client
+}
+
+func (r *LVMClusterReconciler) SnapshotsEnabled() bool {
+	return r.EnableSnapshotting
+}
+
+func (r *LVMClusterReconciler) GetTopoLVMLeaderElectionPassthrough() configv1.LeaderElection {
+	return r.TopoLVMLeaderElectionPassthrough
+}
+
 //+kubebuilder:rbac:groups=lvm.topolvm.io,resources=lvmclusters,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=lvm.topolvm.io,resources=lvmclusters/status,verbs=get;update;patch
@@ -176,27 +183,27 @@ func (r *LVMClusterReconciler) reconcile(ctx context.Context, instance *lvmv1alp
 		logger.Info("successfully added finalizer")
 	}
 
-	resources := []resourceManager{
-		&csiDriver{},
-		&topolvmController{},
-		&topolvmNode{},
-		&vgManager{},
-		&lvmVG{},
-		&topolvmStorageClass{},
+	resources := []resource.Manager{
+		resource.CSIDriver(),
+		resource.TopoLVMController(),
+		resource.TopoLVMNode(),
+		resource.VGManager(),
+		resource.LVMVGs(),
+		resource.TopoLVMStorageClass(),
 	}
 
 	if r.ClusterType == cluster.TypeOCP {
-		resources = append(resources, openshiftSccs{})
+		resources = append(resources, resource.OpenShiftSCCs())
 	}
 
-	if r.EnableSnapshotting {
-		resources = append(resources, &topolvmVolumeSnapshotClass{})
+	if r.SnapshotsEnabled() {
+		resources = append(resources, resource.TopoLVMVolumeSnapshotClass())
 	}
 
 	resourceSyncStart := time.Now()
 	results := make(chan error, len(resources))
 	create := func(i int) {
-		results <- resources[i].ensureCreated(r, ctx, instance)
+		results <- resources[i].EnsureCreated(r, ctx, instance)
 	}
 
 	for i := range resources {
@@ -343,24 +350,24 @@ func (r *LVMClusterReconciler) setRunningPodImage(ctx context.Context) error {
 
 	if r.ImageName == "" {
 		// 'POD_NAME' and 'POD_NAMESPACE' are set in env of lvm-operator when running as a container
-		podName := os.Getenv(PodNameEnv)
+		podName := os.Getenv(resource.PodNameEnv)
 		if podName == "" {
-			return fmt.Errorf("failed to get pod name env variable, %s env variable is not set", PodNameEnv)
+			return fmt.Errorf("failed to get pod name env variable, %s env variable is not set", resource.PodNameEnv)
 		}
 
 		pod := &corev1.Pod{}
-		if err := r.Get(ctx, types.NamespacedName{Name: podName, Namespace: r.Namespace}, pod); err != nil {
+		if err := r.Get(ctx, types.NamespacedName{Name: podName, Namespace: r.GetNamespace()}, pod); err != nil {
 			return fmt.Errorf("failed to get pod %s: %w", podName, err)
 		}
 
 		for _, c := range pod.Spec.Containers {
-			if c.Name == LVMOperatorContainerName {
+			if c.Name == constants.LVMOperatorContainerName {
 				r.ImageName = c.Image
 				return nil
 			}
 		}
 
-		return fmt.Errorf("failed to get container image for %s in pod %s", LVMOperatorContainerName, podName)
+		return fmt.Errorf("failed to get container image for %s in pod %s", constants.LVMOperatorContainerName, podName)
 	}
 
 	return nil
@@ -379,26 +386,26 @@ func (r *LVMClusterReconciler) logicalVolumesExist(ctx context.Context) (bool, e
 
 func (r *LVMClusterReconciler) processDelete(ctx context.Context, instance *lvmv1alpha1.LVMCluster) error {
 	if controllerutil.ContainsFinalizer(instance, lvmClusterFinalizer) {
-		resourceDeletionList := []resourceManager{
-			&topolvmStorageClass{},
-			&lvmVG{},
-			&topolvmController{},
-			&csiDriver{},
-			&topolvmNode{},
-			&vgManager{},
+		resourceDeletionList := []resource.Manager{
+			resource.TopoLVMStorageClass(),
+			resource.LVMVGs(),
+			resource.TopoLVMController(),
+			resource.CSIDriver(),
+			resource.TopoLVMNode(),
+			resource.VGManager(),
 		}
 
 		if r.ClusterType == cluster.TypeOCP {
-			resourceDeletionList = append(resourceDeletionList, openshiftSccs{})
+			resourceDeletionList = append(resourceDeletionList, resource.OpenShiftSCCs())
 		}
 
-		if r.EnableSnapshotting {
-			resourceDeletionList = append(resourceDeletionList, &topolvmVolumeSnapshotClass{})
+		if r.SnapshotsEnabled() {
+			resourceDeletionList = append(resourceDeletionList, resource.TopoLVMVolumeSnapshotClass())
 		}
 
 		for _, unit := range resourceDeletionList {
-			if err := unit.ensureDeleted(r, ctx, instance); err != nil {
-				err := fmt.Errorf("failed cleaning up %s: %w", unit.getName(), err)
+			if err := unit.EnsureDeleted(r, ctx, instance); err != nil {
+				err := fmt.Errorf("failed cleaning up %s: %w", unit.GetName(), err)
 				r.WarningEvent(ctx, instance, EventReasonErrorDeletionPending, err)
 				return err
 			}