Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCPBUGS-27956: Revert back to file-based lvmd config to support different configs for each node #566

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
31 changes: 0 additions & 31 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
55 changes: 1 addition & 54 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 @@ -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)
})
}
}
87 changes: 37 additions & 50 deletions internal/controllers/vgmanager/lvmd/lvmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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},
}
}

Expand Down Expand Up @@ -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
}