Skip to content

Commit

Permalink
Revert "feat: save lvmd config to a configmap instead of a file"
Browse files Browse the repository at this point in the history
This reverts commit faed224.
  • Loading branch information
suleymanakbas91 committed Jan 25, 2024
1 parent a312ffe commit 35ae7a5
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 254 deletions.
12 changes: 0 additions & 12 deletions bundle/manifests/lvms-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -872,18 +872,6 @@ spec:
- create
- patch
- update
- apiGroups:
- ""
resources:
- configmaps
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
serviceAccountName: vg-manager
strategy: deployment
installModes:
Expand Down
15 changes: 5 additions & 10 deletions cmd/vgmanager/vgmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"os"

"github.com/go-logr/logr"
"github.com/openshift/lvm-operator/internal/cluster"
"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"
Expand Down Expand Up @@ -85,18 +84,14 @@ func NewCmd(opts *Options) *cobra.Command {
func run(cmd *cobra.Command, _ []string, opts *Options) error {
lvm := lvm.NewDefaultHostLVM()
nodeName := os.Getenv("NODE_NAME")

operatorNamespace, err := cluster.GetOperatorNamespace()
if err != nil {
return fmt.Errorf("unable to get operatorNamespace: %w", err)
}
namespace := os.Getenv("POD_NAMESPACE")

setupClient, err := client.New(ctrl.GetConfigOrDie(), client.Options{Scheme: opts.Scheme})
if err != nil {
return fmt.Errorf("unable to initialize setup client for pre-manager startup checks: %w", err)
}

if err := tagging.AddTagToVGs(cmd.Context(), setupClient, lvm, nodeName, operatorNamespace); err != nil {
if err := tagging.AddTagToVGs(cmd.Context(), setupClient, lvm, nodeName, namespace); err != nil {
opts.SetupLog.Error(err, "failed to tag existing volume groups")
}

Expand All @@ -114,7 +109,7 @@ func run(cmd *cobra.Command, _ []string, opts *Options) error {
LeaderElection: false,
Cache: cache.Options{
DefaultNamespaces: map[string]cache.Config{
operatorNamespace: {},
os.Getenv("POD_NAMESPACE"): {},
},
},
})
Expand All @@ -125,14 +120,14 @@ 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(),
Dmsetup: dmsetup.NewDefaultHostDmsetup(),
LVM: lvm,
NodeName: nodeName,
Namespace: operatorNamespace,
Namespace: namespace,
Filters: filter.DefaultFilters,
}).SetupWithManager(mgr); err != nil {
return fmt.Errorf("unable to create controller VGManager: %w", err)
Expand Down
12 changes: 0 additions & 12 deletions config/rbac/vg_manager_role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,3 @@ rules:
- create
- patch
- update
- apiGroups:
- ""
resources:
- configmaps
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
7 changes: 3 additions & 4 deletions internal/controllers/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ const (
LivenessProbeMemRequest = "15Mi"
LivenessProbeCPURequest = "1m"

FileCheckerMemRequest = "10Mi"
FileCheckerCPURequest = "1m"

// topoLVM Node
TopolvmNodeServiceAccount = "topolvm-node"
TopolvmNodeDaemonsetName = "topolvm-node"
Expand All @@ -71,10 +74,6 @@ const (
DefaultCSISocket = "/run/topolvm/csi-topolvm.sock"
DeviceClassKey = "topolvm.io/device-class"

LVMDConfigMapName = "lvmd-config"
LVMDDefaultConfigDir = "/etc/topolvm"
LVMDDefaultFileConfigPath = "/etc/topolvm/lvmd.yaml"

// name of the lvm-operator container
LVMOperatorContainerName = "manager"

Expand Down
14 changes: 6 additions & 8 deletions internal/controllers/lvmcluster/resource/topolvm_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,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"
Expand Down Expand Up @@ -156,12 +157,9 @@ func getNodeDaemonSet(lvmCluster *lvmv1alpha1.LVMCluster, namespace string, args
Type: &hostPathDirectoryOrCreateType}}},
{Name: "lvmd-config-dir",
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: constants.LVMDConfigMapName,
},
},
}},
HostPath: &corev1.HostPathVolumeSource{
Path: filepath.Dir(lvmd.DefaultFileConfigPath),
Type: &hostPathDirectory}}},
{Name: "metrics-cert",
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
Expand Down Expand Up @@ -233,7 +231,7 @@ func getNodeContainer(args []string) *corev1.Container {
command := []string{
"/topolvm-node",
"--embed-lvmd",
fmt.Sprintf("--config=%s", constants.LVMDDefaultFileConfigPath),
fmt.Sprintf("--config=%s", lvmd.DefaultFileConfigPath),
}

command = append(command, args...)
Expand All @@ -249,7 +247,7 @@ func getNodeContainer(args []string) *corev1.Container {

volumeMounts := []corev1.VolumeMount{
{Name: "node-plugin-dir", MountPath: filepath.Dir(constants.DefaultCSISocket)},
{Name: "lvmd-config-dir", MountPath: constants.LVMDDefaultConfigDir},
{Name: "lvmd-config-dir", MountPath: filepath.Dir(lvmd.DefaultFileConfigPath)},
{Name: "pod-volumes-dir",
MountPath: fmt.Sprintf("%spods", getAbsoluteKubeletPath(constants.CSIKubeletRootDir)),
MountPropagation: &mountPropagationMode},
Expand Down
42 changes: 5 additions & 37 deletions internal/controllers/vgmanager/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"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"

corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -44,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 (
Expand Down Expand Up @@ -83,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
Expand Down Expand Up @@ -326,7 +294,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, nil, err); err != nil {
Expand Down Expand Up @@ -392,7 +360,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"
Expand All @@ -407,7 +375,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)
Expand Down Expand Up @@ -475,14 +443,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"
Expand Down
66 changes: 6 additions & 60 deletions internal/controllers/vgmanager/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,11 @@ 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"

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"
Expand Down Expand Up @@ -111,6 +107,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"
Expand All @@ -126,8 +123,6 @@ func setupInstances() testInstances {
fakeRecorder := record.NewFakeRecorder(100)
fakeRecorder.IncludeObject = true

testLVMD := lvmd.NewFileConfigurator(fakeClient, namespace.GetName())

return testInstances{
LVM: mockLVM,
LSBLK: mockLSBLK,
Expand Down Expand Up @@ -307,7 +302,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())
Expand Down Expand Up @@ -397,7 +392,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(BeNil())
})
Expand Down Expand Up @@ -552,13 +547,13 @@ 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"))
mockLVM.EXPECT().ListVGs().Once().Return(nil, fmt.Errorf("mock list failure"))
err := r.applyLVMDConfig(ctx, vg, devices)
Expect(err).To(HaveOccurred(), "should error if lvmd config cannot be loaded and/or status cannot be set")

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"))
mockLVM.EXPECT().ListVGs().Once().Return(nil, fmt.Errorf("mock list failure"))
err = r.applyLVMDConfig(ctx, vg, devices)
Expect(err).To(HaveOccurred(), "should error if lvmd config cannot be saved and/or status cannot be set")
Expand Down Expand Up @@ -854,52 +849,3 @@ func testReconcileFailure(ctx context.Context) {
Expect(err).To(HaveOccurred())
})
}

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)
})
}
}
Loading

0 comments on commit 35ae7a5

Please sign in to comment.