fix: revert back to file-based lvmd config to support different configs for each node

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

Author: suleymanakbas91

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,15 @@ 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: &HostPathDirectoryOrCreate},
 		},
 	}
 	LVMDConfMapVolMount = corev1.VolumeMount{
-		Name: LVMDConfMapVolName, MountPath: constants.LVMDDefaultConfigDir,
+		Name:             LVMDConfMapVolName,
+		MountPath:        filepath.Dir(lvmd.DefaultFileConfigPath),
+		MountPropagation: &hostContainerPropagation,
 	}
 )
 

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

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,
@@ -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)
-		})
-	}
-}

internal/controllers/vgmanager/lvmd/lvmd.go

@@ -3,16 +3,12 @@ package lvmd
 import (
 	"context"
 	"fmt"
+	"io"
+	"os"
 
-	"github.com/openshift/lvm-operator/internal/controllers/constants"
 	"github.com/topolvm/topolvm/lvmd"
 	lvmdCMD "github.com/topolvm/topolvm/pkg/lvmd/cmd"
 
-	corev1 "k8s.io/api/core/v1"
-	k8serrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/yaml"
 )
@@ -24,9 +20,18 @@ type ThinPoolConfig = lvmd.ThinPoolConfig
 
 var TypeThin = lvmd.TypeThin
 
-func NewFileConfigurator(client client.Client, namespace string) *CachedFileConfig {
+const (
+	DefaultFileConfigPath = "/etc/topolvm/lvmd.yaml"
+	maxReadLength         = 2 * 1 << 20 // 2MB
+)
+
+func DefaultConfigurator() *CachedFileConfig {
+	return NewFileConfigurator(DefaultFileConfigPath)
+}
+
+func NewFileConfigurator(path string) *CachedFileConfig {
 	return &CachedFileConfig{
-		FileConfig: FileConfig{Client: client, Namespace: namespace},
+		FileConfig: FileConfig{path: path},
 	}
 }
 
@@ -61,69 +66,51 @@ func (c *CachedFileConfig) Save(ctx context.Context, config *Config) error {
 }
 
 type FileConfig struct {
-	client.Client
-	Namespace string
+	path string
 }
 
 func (c FileConfig) Load(ctx context.Context) (*Config, error) {
-	cm := &corev1.ConfigMap{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      constants.LVMDConfigMapName,
-			Namespace: c.Namespace,
-		},
-	}
-	err := c.Client.Get(ctx, client.ObjectKeyFromObject(cm), cm)
-	if k8serrors.IsNotFound(err) {
+	file, err := os.Open(c.path)
+	if os.IsNotExist(err) {
 		// If the file does not exist, return nil for both
 		return nil, nil
 	} else if err != nil {
-		return nil, fmt.Errorf("failed to get ConfigMap %s: %w", cm.Name, err)
+		return nil, fmt.Errorf("failed to open config file %s: %w", c.path, err)
+	}
+	defer file.Close()
+
+	limitedReader := &io.LimitedReader{R: file, N: maxReadLength}
+	cfgBytes, err := io.ReadAll(limitedReader)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read config file %s: %w", c.path, err)
+	}
+
+	if limitedReader.N <= 0 {
+		return nil, fmt.Errorf("the read limit is reached for config file %s", c.path)
 	}
 
 	config := &Config{}
-	if err = yaml.Unmarshal([]byte(cm.Data["lvmd.yaml"]), config); err != nil {
-		return nil, fmt.Errorf("failed to unmarshal config file: %w", err)
+	if err = yaml.Unmarshal(cfgBytes, config); err != nil {
+		return nil, fmt.Errorf("failed to unmarshal config file %s: %w", c.path, err)
 	}
 	return config, nil
 }
 
 func (c FileConfig) Save(ctx context.Context, config *Config) error {
 	out, err := yaml.Marshal(config)
-	if err != nil {
-		return fmt.Errorf("failed to marshal config file: %w", err)
+	if err == nil {
+		err = os.WriteFile(c.path, out, 0600)
 	}
-
-	cm := &corev1.ConfigMap{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      constants.LVMDConfigMapName,
-			Namespace: c.Namespace,
-		},
-	}
-	_, err = ctrl.CreateOrUpdate(ctx, c.Client, cm, func() error {
-		cm.Data = map[string]string{"lvmd.yaml": string(out)}
-		return nil
-	})
 	if err != nil {
-		return fmt.Errorf("failed to apply ConfigMap %s: %w", cm.GetName(), err)
+		return fmt.Errorf("failed to save config file %s: %w", c.path, err)
 	}
-
 	return nil
 }
 
 func (c FileConfig) Delete(ctx context.Context) error {
-	cm := &corev1.ConfigMap{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      constants.LVMDConfigMapName,
-			Namespace: c.Namespace,
-		},
-	}
-	if err := c.Client.Delete(ctx, cm); err != nil {
-		if k8serrors.IsNotFound(err) {
-			// If the file does not exist, return nil
-			return nil
-		}
-		return fmt.Errorf("failed to delete ConfigMap: %w", err)
+	err := os.Remove(c.path)
+	if err != nil {
+		return fmt.Errorf("failed to delete config file %s: %w", c.path, err)
 	}
-
-	return nil
+	return err
 }