fix: revert back to file-based lvmd config to support different confi…

…gs for each node

Signed-off-by: Suleyman Akbas <[email protected]>

bundle/manifests/lvms-operator.clusterserviceversion.yaml

@@ -679,18 +679,6 @@ spec:
           - create
           - patch
           - update
-        - apiGroups:
-          - ""
-          resources:
-          - configmaps
-          verbs:
-          - create
-          - delete
-          - get
-          - list
-          - patch
-          - update
-          - watch
         serviceAccountName: vg-manager
     strategy: deployment
   installModes:

cmd/vgmanager/vgmanager.go

@@ -144,7 +144,7 @@ func run(cmd *cobra.Command, _ []string, opts *Options) error {
 	}
 
 	lvmdConfig := &lvmd.Config{}
-	if err := loadConfFile(ctx, lvmdConfig, constants.LVMDDefaultFileConfigPath); err != nil {
+	if err := loadConfFile(ctx, lvmdConfig, lvmd.DefaultFileConfigPath); err != nil {
 		opts.SetupLog.Error(err, "lvmd config could not be loaded, starting without topolvm components and attempting bootstrap")
 		if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil {
 			return fmt.Errorf("unable to set up ready check: %w", err)
@@ -199,7 +199,7 @@ func run(cmd *cobra.Command, _ []string, opts *Options) error {
 	if err = (&vgmanager.Reconciler{
 		Client:        mgr.GetClient(),
 		EventRecorder: mgr.GetEventRecorderFor(vgmanager.ControllerName),
-		LVMD:          lvmd.NewFileConfigurator(mgr.GetClient(), operatorNamespace),
+		LVMD:          lvmd.DefaultConfigurator(),
 		Scheme:        mgr.GetScheme(),
 		LSBLK:         lsblk.NewDefaultHostLSBLK(),
 		Wipefs:        wipefs.NewDefaultHostWipefs(),
@@ -238,7 +238,7 @@ func run(cmd *cobra.Command, _ []string, opts *Options) error {
 		fileNotExist := false
 		for {
 			// check if file exists, otherwise the watcher errors
-			_, err := os.Lstat(constants.LVMDDefaultFileConfigPath)
+			_, err := os.Lstat(lvmd.DefaultFileConfigPath)
 			if err != nil {
 				if os.IsNotExist(err) {
 					time.Sleep(100 * time.Millisecond)
@@ -251,7 +251,7 @@ func run(cmd *cobra.Command, _ []string, opts *Options) error {
 				if fileNotExist {
 					cancel()
 				}
-				err = watcher.Add(constants.LVMDDefaultFileConfigPath)
+				err = watcher.Add(lvmd.DefaultFileConfigPath)
 				if err != nil {
 					opts.SetupLog.Error(err, "unable to add file path to watcher")
 				}

config/rbac/vg_manager_role.yaml

@@ -66,15 +66,3 @@ rules:
     - create
     - patch
     - update
-- apiGroups:
-    - ""
-  resources:
-    - configmaps
-  verbs:
-    - create
-    - delete
-    - get
-    - list
-    - patch
-    - update
-    - watch

internal/controllers/constants/constants.go

@@ -32,10 +32,6 @@ const (
 	DeviceClassKey                = "topolvm.io/device-class"
 	DefaultPluginRegistrationPath = "/registration"
 
-	LVMDConfigMapName         = "lvmd-config"
-	LVMDDefaultConfigDir      = "/etc/topolvm"
-	LVMDDefaultFileConfigPath = "/etc/topolvm/lvmd.yaml"
-
 	// name of the lvm-operator container
 	LVMOperatorContainerName = "manager"
 

internal/controllers/lvmcluster/resource/vgmanager_daemonset.go

@@ -24,6 +24,7 @@ import (
 	lvmv1alpha1 "github.com/openshift/lvm-operator/api/v1alpha1"
 	"github.com/openshift/lvm-operator/internal/controllers/constants"
 	"github.com/openshift/lvm-operator/internal/controllers/lvmcluster/selector"
+	"github.com/openshift/lvm-operator/internal/controllers/vgmanager/lvmd"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -109,16 +110,13 @@ var (
 	LVMDConfMapVol     = corev1.Volume{
 		Name: LVMDConfMapVolName,
 		VolumeSource: corev1.VolumeSource{
-			ConfigMap: &corev1.ConfigMapVolumeSource{
-				Optional: ptr.To(true),
-				LocalObjectReference: corev1.LocalObjectReference{
-					Name: constants.LVMDConfigMapName,
-				},
-			},
+			HostPath: &corev1.HostPathVolumeSource{
+				Path: filepath.Dir(lvmd.DefaultFileConfigPath),
+				Type: &directoryHostPath},
 		},
 	}
 	LVMDConfMapVolMount = corev1.VolumeMount{
-		Name: LVMDConfMapVolName, MountPath: constants.LVMDDefaultConfigDir,
+		Name: LVMDConfMapVolName, MountPath: filepath.Dir(lvmd.DefaultFileConfigPath),
 	}
 )
 

internal/controllers/vgmanager/controller.go

@@ -43,10 +43,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
-	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
 const (
@@ -82,38 +80,9 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&lvmv1alpha1.LVMVolumeGroup{}).
 		Owns(&lvmv1alpha1.LVMVolumeGroupNodeStatus{}, builder.MatchEveryOwner, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
-		Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(r.getObjsInNamespaceForReconcile)).
 		Complete(r)
 }
 
-// getObjsInNamespaceForReconcile reconciles the object anytime the given object is in the same namespace
-// as the available LVMVolumeGroups.
-func (r *Reconciler) getObjsInNamespaceForReconcile(ctx context.Context, obj client.Object) []reconcile.Request {
-	foundLVMVolumeGroupList := &lvmv1alpha1.LVMVolumeGroupList{}
-	listOps := &client.ListOptions{
-		Namespace: obj.GetNamespace(),
-	}
-
-	if err := r.List(ctx, foundLVMVolumeGroupList, listOps); err != nil {
-		log.FromContext(ctx).Error(err, "getObjsInNamespaceForReconcile: Failed to get LVMVolumeGroup objs")
-		return []reconcile.Request{}
-	}
-	if len(foundLVMVolumeGroupList.Items) < 1 {
-		return []reconcile.Request{}
-	}
-
-	var requests []reconcile.Request
-	for _, lvmVG := range foundLVMVolumeGroupList.Items {
-		requests = append(requests, reconcile.Request{
-			NamespacedName: types.NamespacedName{
-				Name:      lvmVG.GetName(),
-				Namespace: lvmVG.GetNamespace(),
-			},
-		})
-	}
-	return requests
-}
-
 type Reconciler struct {
 	client.Client
 	Scheme *runtime.Scheme
@@ -348,7 +317,7 @@ func (r *Reconciler) applyLVMDConfig(ctx context.Context, volumeGroup *lvmv1alph
 	logger := log.FromContext(ctx).WithValues("VGName", volumeGroup.Name)
 
 	// Read the lvmd config file
-	lvmdConfig, err := r.LVMD.Load(ctx)
+	lvmdConfig, err := r.LVMD.Load()
 	if err != nil {
 		err = fmt.Errorf("failed to read the lvmd config file: %w", err)
 		if _, err := r.setVolumeGroupFailedStatus(ctx, volumeGroup, vgs, devices, err); err != nil {
@@ -414,7 +383,7 @@ func (r *Reconciler) updateLVMDConfigAfterReconcile(
 			r.NormalEvent(ctx, volumeGroup, EventReasonLVMDConfigMissing, msg)
 		}
 
-		if err := r.LVMD.Save(ctx, newCFG); err != nil {
+		if err := r.LVMD.Save(newCFG); err != nil {
 			return fmt.Errorf("failed to update lvmd config file to update volume group %s: %w", volumeGroup.GetName(), err)
 		}
 		msg := "updated lvmd config with new deviceClasses, now restarting.."
@@ -431,7 +400,7 @@ func (r *Reconciler) processDelete(ctx context.Context, volumeGroup *lvmv1alpha1
 	logger.Info("deleting")
 
 	// Read the lvmd config file
-	lvmdConfig, err := r.LVMD.Load(ctx)
+	lvmdConfig, err := r.LVMD.Load()
 	if err != nil {
 		// Failed to read lvmdconfig file. Reconcile again
 		return fmt.Errorf("failed to read the lvmd config file: %w", err)
@@ -509,14 +478,14 @@ func (r *Reconciler) processDelete(ctx context.Context, volumeGroup *lvmv1alpha1
 	// if there was no config file in the first place, nothing has to be removed.
 	if lvmdConfig != nil {
 		if len(lvmdConfig.DeviceClasses) > 0 {
-			if err = r.LVMD.Save(ctx, lvmdConfig); err != nil {
+			if err = r.LVMD.Save(lvmdConfig); err != nil {
 				return fmt.Errorf("failed to update lvmd.conf file for volume group %s: %w", volumeGroup.GetName(), err)
 			}
 			msg := "updated lvmd config after deviceClass was removed"
 			logger.Info(msg)
 			r.NormalEvent(ctx, volumeGroup, EventReasonLVMDConfigUpdated, msg)
 		} else {
-			if err = r.LVMD.Delete(ctx); err != nil {
+			if err = r.LVMD.Delete(); err != nil {
 				return fmt.Errorf("failed to delete lvmd.conf file for volume group %s: %w", volumeGroup.GetName(), err)
 			}
 			msg := "removed lvmd config after last deviceClass was removed"

internal/controllers/vgmanager/controller_test.go

@@ -24,16 +24,13 @@ import (
 	"github.com/openshift/lvm-operator/internal/controllers/vgmanager/lvmd"
 	lvmdmocks "github.com/openshift/lvm-operator/internal/controllers/vgmanager/lvmd/mocks"
 	wipefsmocks "github.com/openshift/lvm-operator/internal/controllers/vgmanager/wipefs/mocks"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	topolvmv1 "github.com/topolvm/topolvm/api/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -112,6 +109,7 @@ func setupInstances() testInstances {
 	mockLSBLK := lsblkmocks.NewMockLSBLK(t)
 	mockLVM := lvmmocks.NewMockLVM(t)
 	mockWipefs := wipefsmocks.NewMockWipefs(t)
+	testLVMD := lvmd.NewFileConfigurator(filepath.Join(t.TempDir(), "lvmd.yaml"))
 
 	hostname := "test-host.vgmanager.test.io"
 	hostnameLabelKey := "kubernetes.io/hostname"
@@ -127,8 +125,6 @@ func setupInstances() testInstances {
 	fakeRecorder := record.NewFakeRecorder(100)
 	fakeRecorder.IncludeObject = true
 
-	testLVMD := lvmd.NewFileConfigurator(fakeClient, namespace.GetName())
-
 	return testInstances{
 		LVM:       mockLVM,
 		LSBLK:     mockLSBLK,
@@ -319,7 +315,7 @@ func testMockedBlockDeviceOnHost(ctx context.Context) {
 	By("verifying the lvmd config generation", func() {
 		checkDistributedEvent(corev1.EventTypeNormal, "lvmd config file doesn't exist, will attempt to create a fresh config")
 		checkDistributedEvent(corev1.EventTypeNormal, "updated lvmd config with new deviceClasses")
-		lvmdConfig, err := instances.LVMD.Load(ctx)
+		lvmdConfig, err := instances.LVMD.Load()
 		Expect(err).ToNot(HaveOccurred())
 		Expect(lvmdConfig).ToNot(BeNil())
 		Expect(lvmdConfig.DeviceClasses).ToNot(BeNil())
@@ -409,7 +405,7 @@ func testMockedBlockDeviceOnHost(ctx context.Context) {
 		_, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)})
 		Expect(err).ToNot(HaveOccurred())
 		Expect(instances.client.Get(ctx, client.ObjectKeyFromObject(vg), vg)).ToNot(Succeed())
-		lvmdConfig, err := instances.LVMD.Load(ctx)
+		lvmdConfig, err := instances.LVMD.Load()
 		Expect(err).ToNot(HaveOccurred())
 		Expect(lvmdConfig).To(Not(BeNil()), "cached lvmd config is still present")
 	})
@@ -566,12 +562,12 @@ func testLVMD(ctx context.Context) {
 	mockLVM := lvmmocks.NewMockLVM(GinkgoT())
 	r.LVM = mockLVM
 
-	mockLVMD.EXPECT().Load(ctx).Once().Return(nil, fmt.Errorf("mock load failure"))
+	mockLVMD.EXPECT().Load().Once().Return(nil, fmt.Errorf("mock load failure"))
 	err := r.applyLVMDConfig(ctx, vg, []lvm.VolumeGroup{}, devices)
 	Expect(err).To(HaveOccurred(), "should error if lvmd config cannot be loaded")
 
-	mockLVMD.EXPECT().Load(ctx).Once().Return(&lvmd.Config{}, nil)
-	mockLVMD.EXPECT().Save(ctx, mock.Anything).Once().Return(fmt.Errorf("mock save failure"))
+	mockLVMD.EXPECT().Load().Once().Return(&lvmd.Config{}, nil)
+	mockLVMD.EXPECT().Save(mock.Anything).Once().Return(fmt.Errorf("mock save failure"))
 	err = r.applyLVMDConfig(ctx, vg, []lvm.VolumeGroup{}, devices)
 	Expect(err).To(HaveOccurred(), "should error if lvmd config cannot be saved")
 }
@@ -836,52 +832,3 @@ func testReconcileFailure(ctx context.Context) {
 		Expect(err).To(MatchError(expectedError))
 	})
 }
-
-func TestGetObjsInNamespaceForReconcile(t *testing.T) {
-	tests := []struct {
-		name   string
-		objs   []client.Object
-		list   error
-		expect []reconcile.Request
-	}{
-		{
-			name: "test lvmvolumegroup not fetch error",
-			list: assert.AnError,
-		},
-		{
-			name: "test lvmvolumegroup found in a different namespace",
-			objs: []client.Object{
-				&lvmv1alpha1.LVMVolumeGroup{ObjectMeta: metav1.ObjectMeta{Name: "test-vg", Namespace: "not-test"}},
-			},
-		},
-		{
-			name: "test lvmvolumegroup found in the same namespace",
-			objs: []client.Object{
-				&lvmv1alpha1.LVMVolumeGroup{ObjectMeta: metav1.ObjectMeta{Name: "test-vg", Namespace: "test"}},
-			},
-			expect: []reconcile.Request{{NamespacedName: types.NamespacedName{Name: "test-vg", Namespace: "test"}}},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			newScheme := runtime.NewScheme()
-			assert.NoError(t, lvmv1alpha1.AddToScheme(newScheme))
-			assert.NoError(t, corev1.AddToScheme(newScheme))
-			clnt := fake.NewClientBuilder().WithObjects(tt.objs...).
-				WithScheme(newScheme).WithInterceptorFuncs(interceptor.Funcs{
-				List: func(ctx context.Context, client client.WithWatch, list client.ObjectList, opts ...client.ListOption) error {
-					if tt.list != nil {
-						return tt.list
-					}
-					return client.List(ctx, list, opts...)
-				},
-			}).Build()
-
-			r := &Reconciler{Client: clnt}
-			requests := r.getObjsInNamespaceForReconcile(context.Background(),
-				&corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "test-vg", Namespace: "test"}})
-			assert.ElementsMatch(t, tt.expect, requests)
-		})
-	}
-}