Skip to content

Commit

Permalink
Merge pull request #441 from jakobmoellerdev/OCPVE-347-naming-alignment
Browse files Browse the repository at this point in the history
OCPVE-347: chore: unify controller naming
  • Loading branch information
openshift-ci[bot] authored Oct 9, 2023
2 parents 58e7c6f + ad34f71 commit 4d94695
Show file tree
Hide file tree
Showing 20 changed files with 85 additions and 83 deletions.
1 change: 1 addition & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ repos:
entry: golangci-lint run --fix
args:
- "--modules-download-mode=vendor"
- "--timeout=10m"
types: [go]
language: golang
require_serial: true
Expand Down
11 changes: 6 additions & 5 deletions cmd/operator/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,14 @@ import (
"github.com/go-logr/logr"
"github.com/openshift/lvm-operator/internal/controllers/lvmcluster"
"github.com/openshift/lvm-operator/internal/controllers/lvmcluster/logpassthrough"
"github.com/openshift/lvm-operator/internal/controllers/node"
"github.com/openshift/lvm-operator/internal/controllers/node/removal"
"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"
"sigs.k8s.io/controller-runtime/pkg/cache"

// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
// to ensure that exec-entrypoint and run can make use of them.
_ "k8s.io/client-go/plugin/pkg/client/auth"
Expand Down Expand Up @@ -171,7 +172,7 @@ func run(cmd *cobra.Command, _ []string, opts *Options) error {
}

// register controllers
if err = (&lvmcluster.LVMClusterReconciler{
if err = (&lvmcluster.Reconciler{
Client: mgr.GetClient(),
EventRecorder: mgr.GetEventRecorderFor("LVMClusterReconciler"),
ClusterType: clusterType,
Expand All @@ -186,7 +187,7 @@ func run(cmd *cobra.Command, _ []string, opts *Options) error {

if !snoCheck.IsSNO(cmd.Context()) {
opts.SetupLog.Info("starting node-removal controller to observe node removal in MultiNode")
if err = (&node.RemovalController{Client: mgr.GetClient()}).SetupWithManager(mgr); err != nil {
if err = (&removal.Reconciler{Client: mgr.GetClient()}).SetupWithManager(mgr); err != nil {
return fmt.Errorf("unable to create NodeRemovalController controller: %w", err)
}
}
Expand All @@ -201,12 +202,12 @@ func run(cmd *cobra.Command, _ []string, opts *Options) error {
return fmt.Errorf("unable to create LVMCluster webhook: %w", err)
}

pvController := persistent_volume.NewPersistentVolumeReconciler(mgr.GetClient(), mgr.GetAPIReader(), mgr.GetEventRecorderFor("lvms-pv-controller"))
pvController := persistent_volume.NewReconciler(mgr.GetClient(), mgr.GetAPIReader(), mgr.GetEventRecorderFor("lvms-pv-controller"))
if err := pvController.SetupWithManager(mgr); err != nil {
return fmt.Errorf("unable to create PersistentVolume controller: %w", err)
}

pvcController := persistent_volume_claim.NewPersistentVolumeClaimReconciler(mgr.GetClient(), mgr.GetEventRecorderFor("lvms-pvc-controller"))
pvcController := persistent_volume_claim.NewReconciler(mgr.GetClient(), mgr.GetEventRecorderFor("lvms-pvc-controller"))
if err := pvcController.SetupWithManager(mgr); err != nil {
return fmt.Errorf("unable to create PersistentVolumeClaim controller: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/vgmanager/vgmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func run(_ *cobra.Command, _ []string, opts *Options) error {
return fmt.Errorf("unable to start manager: %w", err)
}

if err = (&vgmanager.VGReconciler{
if err = (&vgmanager.Reconciler{
Client: mgr.GetClient(),
EventRecorder: mgr.GetEventRecorderFor(vgmanager.ControllerName),
LVMD: lvmd.DefaultConfigurator(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ const (
lvmClusterFinalizer = "lvmcluster.topolvm.io"
)

// LVMClusterReconciler reconciles a LVMCluster object
type LVMClusterReconciler struct {
// Reconciler reconciles a LVMCluster object
type Reconciler struct {
client.Client
record.EventRecorder
ClusterType cluster.Type
Expand All @@ -76,31 +76,31 @@ type LVMClusterReconciler struct {
LogPassthroughOptions *logpassthrough.Options
}

func (r *LVMClusterReconciler) GetNamespace() string {
func (r *Reconciler) GetNamespace() string {
return r.Namespace
}

func (r *LVMClusterReconciler) GetImageName() string {
func (r *Reconciler) GetImageName() string {
return r.ImageName
}

func (r *LVMClusterReconciler) GetClient() client.Client {
func (r *Reconciler) GetClient() client.Client {
return r.Client
}

func (r *LVMClusterReconciler) SnapshotsEnabled() bool {
func (r *Reconciler) SnapshotsEnabled() bool {
return r.EnableSnapshotting
}

func (r *LVMClusterReconciler) GetVGManagerCommand() []string {
func (r *Reconciler) GetVGManagerCommand() []string {
return r.VGManagerCommand
}

func (r *LVMClusterReconciler) GetTopoLVMLeaderElectionPassthrough() configv1.LeaderElection {
func (r *Reconciler) GetTopoLVMLeaderElectionPassthrough() configv1.LeaderElection {
return r.TopoLVMLeaderElectionPassthrough
}

func (r *LVMClusterReconciler) GetLogPassthroughOptions() *logpassthrough.Options {
func (r *Reconciler) GetLogPassthroughOptions() *logpassthrough.Options {
return r.LogPassthroughOptions
}

Expand All @@ -125,7 +125,7 @@ func (r *LVMClusterReconciler) GetLogPassthroughOptions() *logpassthrough.Option
//
// For more details, check Reconcile and its Result here:
// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
func (r *LVMClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
logger := log.FromContext(ctx)
logger.Info("reconciling")

Expand Down Expand Up @@ -164,7 +164,7 @@ func (r *LVMClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
}

// errors returned by this will be updated in the reconcileSucceeded condition of the LVMCluster
func (r *LVMClusterReconciler) reconcile(ctx context.Context, instance *lvmv1alpha1.LVMCluster) (ctrl.Result, error) {
func (r *Reconciler) reconcile(ctx context.Context, instance *lvmv1alpha1.LVMCluster) (ctrl.Result, error) {
logger := log.FromContext(ctx)

// The resource was deleted
Expand Down Expand Up @@ -246,7 +246,7 @@ func (r *LVMClusterReconciler) reconcile(ctx context.Context, instance *lvmv1alp
return ctrl.Result{}, nil
}

func (r *LVMClusterReconciler) updateLVMClusterStatus(ctx context.Context, instance *lvmv1alpha1.LVMCluster) error {
func (r *Reconciler) updateLVMClusterStatus(ctx context.Context, instance *lvmv1alpha1.LVMCluster) error {
logger := log.FromContext(ctx)

vgNodeMap := make(map[string][]lvmv1alpha1.NodeStatus)
Expand Down Expand Up @@ -317,7 +317,7 @@ func (r *LVMClusterReconciler) updateLVMClusterStatus(ctx context.Context, insta
return nil
}

func (r *LVMClusterReconciler) getExpectedVGCount(ctx context.Context, instance *lvmv1alpha1.LVMCluster) (int, error) {
func (r *Reconciler) getExpectedVGCount(ctx context.Context, instance *lvmv1alpha1.LVMCluster) (int, error) {
logger := log.FromContext(ctx)
var vgCount int

Expand Down Expand Up @@ -361,7 +361,7 @@ func (r *LVMClusterReconciler) getExpectedVGCount(ctx context.Context, instance
}

// getRunningPodImage gets the operator image and set it in reconciler struct
func (r *LVMClusterReconciler) setRunningPodImage(ctx context.Context) error {
func (r *Reconciler) 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
Expand All @@ -388,7 +388,7 @@ func (r *LVMClusterReconciler) setRunningPodImage(ctx context.Context) error {
return nil
}

func (r *LVMClusterReconciler) logicalVolumesExist(ctx context.Context) (bool, error) {
func (r *Reconciler) logicalVolumesExist(ctx context.Context) (bool, error) {
logicalVolumeList := &topolvmv1.LogicalVolumeList{}
if err := r.Client.List(ctx, logicalVolumeList); err != nil {
return false, fmt.Errorf("failed to get TopoLVM LogicalVolume list: %w", err)
Expand All @@ -399,7 +399,7 @@ func (r *LVMClusterReconciler) logicalVolumesExist(ctx context.Context) (bool, e
return false, nil
}

func (r *LVMClusterReconciler) processDelete(ctx context.Context, instance *lvmv1alpha1.LVMCluster) error {
func (r *Reconciler) processDelete(ctx context.Context, instance *lvmv1alpha1.LVMCluster) error {
if controllerutil.ContainsFinalizer(instance, lvmClusterFinalizer) {
resourceDeletionList := []resource.Manager{
resource.TopoLVMStorageClass(),
Expand Down Expand Up @@ -436,11 +436,11 @@ func (r *LVMClusterReconciler) processDelete(ctx context.Context, instance *lvmv
return nil
}

func (r *LVMClusterReconciler) WarningEvent(_ context.Context, obj client.Object, reason EventReasonError, err error) {
func (r *Reconciler) WarningEvent(_ context.Context, obj client.Object, reason EventReasonError, err error) {
r.Event(obj, corev1.EventTypeWarning, string(reason), err.Error())
}

func (r *LVMClusterReconciler) NormalEvent(ctx context.Context, obj client.Object, reason EventReasonInfo, message string) {
func (r *Reconciler) NormalEvent(ctx context.Context, obj client.Object, reason EventReasonInfo, message string) {
if !log.FromContext(ctx).V(1).Enabled() {
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import (
)

// SetupWithManager sets up the controller with the Manager.
func (r *LVMClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
builder := ctrl.NewControllerManagedBy(mgr).
For(&lvmv1alpha1.LVMCluster{}).
Owns(&appsv1.DaemonSet{}).
Expand Down Expand Up @@ -76,7 +76,7 @@ func (r *LVMClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
// the namespaced LVMCluster needs to "own" a cluster-scoped resource, in which case owner references are invalid).
// This should generally only be used for cluster-scoped resources. Also it should be noted that deletion logic must
// be handled manually as garbage collection is not handled automatically like for owner references.
func (r *LVMClusterReconciler) getManagedLabelObjsForReconcile(ctx context.Context, obj client.Object) []reconcile.Request {
func (r *Reconciler) getManagedLabelObjsForReconcile(ctx context.Context, obj client.Object) []reconcile.Request {
foundLVMClusterList := &lvmv1alpha1.LVMClusterList{}
listOps := &client.ListOptions{
Namespace: obj.GetNamespace(),
Expand Down Expand Up @@ -106,7 +106,7 @@ func (r *LVMClusterReconciler) getManagedLabelObjsForReconcile(ctx context.Conte
// this means that as if the obj name fits to a given node on the cluster and that node is part of the node selector,
// then the lvm cluster will get updated as well. Should only be used in conjunction with LVMVolumeGroupNodeStatus
// as other objects do not use the node name as resource name.
func (r *LVMClusterReconciler) getLVMClusterObjsByNameFittingNodeSelector(ctx context.Context, obj client.Object) []reconcile.Request {
func (r *Reconciler) getLVMClusterObjsByNameFittingNodeSelector(ctx context.Context, obj client.Object) []reconcile.Request {
foundLVMClusterList := &lvmv1alpha1.LVMClusterList{}
listOps := &client.ListOptions{
Namespace: obj.GetNamespace(),
Expand Down
6 changes: 3 additions & 3 deletions internal/controllers/lvmcluster/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
secv1 "github.com/openshift/api/security/v1"
"github.com/openshift/lvm-operator/internal/cluster"
"github.com/openshift/lvm-operator/internal/controllers/lvmcluster/logpassthrough"
"github.com/openshift/lvm-operator/internal/controllers/node"
"github.com/openshift/lvm-operator/internal/controllers/node/removal"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/client-go/kubernetes/scheme"
Expand Down Expand Up @@ -131,7 +131,7 @@ var _ = BeforeSuite(func() {
}
}

err = (&LVMClusterReconciler{
err = (&Reconciler{
Client: k8sManager.GetClient(),
EventRecorder: k8sManager.GetEventRecorderFor("LVMClusterReconciler"),
EnableSnapshotting: enableSnapshotting,
Expand All @@ -142,7 +142,7 @@ var _ = BeforeSuite(func() {
}).SetupWithManager(k8sManager)
Expect(err).ToNot(HaveOccurred())

err = (&node.RemovalController{
err = (&removal.Reconciler{
Client: k8sManager.GetClient(),
}).SetupWithManager(k8sManager)
Expect(err).ToNot(HaveOccurred())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const (
testNamespace = "default"
)

func newFakeLVMClusterReconciler(t *testing.T, objs ...client.Object) *LVMClusterReconciler {
func newFakeReconciler(t *testing.T, objs ...client.Object) *Reconciler {
scheme, err := lvmv1alpha1.SchemeBuilder.Build()
assert.NilError(t, err, "creating scheme")

Expand All @@ -57,7 +57,7 @@ func newFakeLVMClusterReconciler(t *testing.T, objs ...client.Object) *LVMCluste
err = snapapi.AddToScheme(scheme)
assert.NilError(t, err, "adding snapshot api to scheme")

return &LVMClusterReconciler{
return &Reconciler{
Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build(),
Namespace: "default",
LogPassthroughOptions: logpassthrough.NewOptions(),
Expand Down Expand Up @@ -94,7 +94,7 @@ func TestVGManagerEnsureCreated(t *testing.T) {
},
Spec: testCase.lvmclusterSpec,
}
r := newFakeLVMClusterReconciler(t, lvmcluster)
r := newFakeReconciler(t, lvmcluster)
var unit = resource.VGManager()
err := unit.EnsureCreated(r, log.IntoContext(context.Background(), testr.New(t)), lvmcluster)
assert.NilError(t, err, "running EnsureCreated")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package node
package removal

import (
"context"
Expand All @@ -20,7 +20,7 @@ import (
const cleanupFinalizer = "lvm.topolvm.io/node-removal-hook"
const fieldOwner = "lvms"

type RemovalController struct {
type Reconciler struct {
client.Client
}

Expand All @@ -33,7 +33,7 @@ type RemovalController struct {
// the unwanted LVMVolumeGroupNodeStatus that was associated with the node, before removing the finalizer.
// It does nothing on active Nodes. If it can be assumed that there will always be only one node (SNO),
// this controller should not be started.
func (r *RemovalController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
logger := log.FromContext(ctx)

node := &v1.Node{}
Expand Down Expand Up @@ -85,12 +85,12 @@ func (r *RemovalController) Reconcile(ctx context.Context, req ctrl.Request) (ct
}

// SetupWithManager sets up the controller with the Manager.
func (r *RemovalController) SetupWithManager(mgr ctrl.Manager) error {
func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).For(&v1.Node{}).Watches(&lvmv1alpha1.LVMVolumeGroupNodeStatus{},
handler.EnqueueRequestsFromMapFunc(r.getNodeForLVMVolumeGroupNodeStatus)).Complete(r)
}

func (r *RemovalController) getNodeForLVMVolumeGroupNodeStatus(ctx context.Context, object client.Object) []reconcile.Request {
func (r *Reconciler) getNodeForLVMVolumeGroupNodeStatus(ctx context.Context, object client.Object) []reconcile.Request {
node := &v1.Node{}
node.SetName(object.GetName())

Expand Down
14 changes: 7 additions & 7 deletions internal/controllers/persistent-volume-claim/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ const (
CapacityAnnotation = "capacity.topolvm.io/"
)

// PersistentVolumeClaimReconciler reconciles a PersistentVolumeClaim object
type PersistentVolumeClaimReconciler struct {
// Reconciler reconciles a PersistentVolumeClaim object
type Reconciler struct {
Client client.Client
Recorder record.EventRecorder
}

// NewPersistentVolumeClaimReconciler returns PersistentVolumeClaimReconciler.
func NewPersistentVolumeClaimReconciler(client client.Client, eventRecorder record.EventRecorder) *PersistentVolumeClaimReconciler {
return &PersistentVolumeClaimReconciler{
// NewReconciler returns Reconciler.
func NewReconciler(client client.Client, eventRecorder record.EventRecorder) *Reconciler {
return &Reconciler{
Client: client,
Recorder: eventRecorder,
}
Expand All @@ -43,7 +43,7 @@ func NewPersistentVolumeClaimReconciler(client client.Client, eventRecorder reco
//+kubebuilder:rbac:groups=core,resources=events,verbs=create;update;patch

// Reconcile PVC
func (r *PersistentVolumeClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
logger := log.Log.WithName("pvc-controller").WithValues("Request.Name", req.Name, "Request.Namespace", req.Namespace)

pvc := &corev1.PersistentVolumeClaim{}
Expand Down Expand Up @@ -120,7 +120,7 @@ func (r *PersistentVolumeClaimReconciler) Reconcile(ctx context.Context, req ctr
}

// SetupWithManager sets up the controller with the Manager.
func (r *PersistentVolumeClaimReconciler) SetupWithManager(mgr ctrl.Manager) error {
func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
pred := predicate.Funcs{
CreateFunc: func(event.CreateEvent) bool { return true },
DeleteFunc: func(event.DeleteEvent) bool { return false },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func TestPersistentVolumeClaimReconciler_Reconcile(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
recorder := record.NewFakeRecorder(1)
r := &persistentvolumeclaim.PersistentVolumeClaimReconciler{
r := &persistentvolumeclaim.Reconciler{
Client: fake.NewClientBuilder().WithObjects(tt.objs...).Build(),
Recorder: recorder,
}
Expand Down
Loading

0 comments on commit 4d94695

Please sign in to comment.