Skip to content

Commit

Permalink
fix: revert back to file-based lvmd config to support different confi…
Browse files Browse the repository at this point in the history
…gs for each node

Signed-off-by: Suleyman Akbas <[email protected]>
  • Loading branch information
suleymanakbas91 committed Jan 31, 2024
1 parent 382b4ae commit f194827
Show file tree
Hide file tree
Showing 9 changed files with 92 additions and 241 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 @@ -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:
Expand Down
8 changes: 4 additions & 4 deletions cmd/vgmanager/vgmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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)
Expand All @@ -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")
}
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
4 changes: 0 additions & 4 deletions internal/controllers/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
14 changes: 7 additions & 7 deletions internal/controllers/lvmcluster/resource/vgmanager_daemonset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
}
)

Expand Down
41 changes: 5 additions & 36 deletions internal/controllers/vgmanager/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.."
Expand All @@ -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)
Expand Down Expand Up @@ -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"
Expand Down
65 changes: 6 additions & 59 deletions internal/controllers/vgmanager/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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,
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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")
})
Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -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)
})
}
}
Loading

0 comments on commit f194827

Please sign in to comment.