From 3776f3e795906b2f7fe706ec26ca1bb12311b5f1 Mon Sep 17 00:00:00 2001 From: Suleyman Akbas Date: Wed, 31 Jan 2024 12:18:12 +0100 Subject: [PATCH] fix: revert back to file-based lvmd config to support different configs for each node Signed-off-by: Suleyman Akbas --- .../lvms-operator.clusterserviceversion.yaml | 12 --- cmd/vgmanager/vgmanager.go | 8 +- config/rbac/vg_manager_role.yaml | 12 --- internal/controllers/constants/constants.go | 4 - .../resource/vgmanager_daemonset.go | 14 +-- internal/controllers/vgmanager/controller.go | 31 ------- .../controllers/vgmanager/controller_test.go | 55 +----------- internal/controllers/vgmanager/lvmd/lvmd.go | 87 ++++++++----------- 8 files changed, 49 insertions(+), 174 deletions(-) diff --git a/bundle/manifests/lvms-operator.clusterserviceversion.yaml b/bundle/manifests/lvms-operator.clusterserviceversion.yaml index 7121d1e92..ef2b2bad4 100644 --- a/bundle/manifests/lvms-operator.clusterserviceversion.yaml +++ b/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: diff --git a/cmd/vgmanager/vgmanager.go b/cmd/vgmanager/vgmanager.go index d5da46a20..06e90348d 100644 --- a/cmd/vgmanager/vgmanager.go +++ b/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") } diff --git a/config/rbac/vg_manager_role.yaml b/config/rbac/vg_manager_role.yaml index 4e8cde82f..8cc42ecdf 100644 --- a/config/rbac/vg_manager_role.yaml +++ b/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 diff --git a/internal/controllers/constants/constants.go b/internal/controllers/constants/constants.go index e4a0df337..ab6f6230e 100644 --- a/internal/controllers/constants/constants.go +++ b/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" diff --git a/internal/controllers/lvmcluster/resource/vgmanager_daemonset.go b/internal/controllers/lvmcluster/resource/vgmanager_daemonset.go index 1b4a66394..c365884ba 100644 --- a/internal/controllers/lvmcluster/resource/vgmanager_daemonset.go +++ b/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, } ) diff --git a/internal/controllers/vgmanager/controller.go b/internal/controllers/vgmanager/controller.go index ebd911c95..1b457fd22 100644 --- a/internal/controllers/vgmanager/controller.go +++ b/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 diff --git a/internal/controllers/vgmanager/controller_test.go b/internal/controllers/vgmanager/controller_test.go index 1ba6e3182..29f35d94e 100644 --- a/internal/controllers/vgmanager/controller_test.go +++ b/internal/controllers/vgmanager/controller_test.go @@ -24,7 +24,6 @@ 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" @@ -32,8 +31,6 @@ import ( 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) - }) - } -} diff --git a/internal/controllers/vgmanager/lvmd/lvmd.go b/internal/controllers/vgmanager/lvmd/lvmd.go index 0456ac24e..d0233823a 100644 --- a/internal/controllers/vgmanager/lvmd/lvmd.go +++ b/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 }