[release-4.13] OCPBUGS-10242: make lvms configuration more robust

docs/default_csi_plugin.md

@@ -3,7 +3,7 @@
 > **IMPORTANT!** The default LVMS configuration is intended to match the developer environment described in [MicroShift Development Environment](./devenv_setup.md). See section **[Configuring LVMS](#Configuring-LVMS)** for guidance on configuring LVMS for your environment.
 
 MicroShift enables dynamic storage provisioning out of the box with the LVMS CSI plugin. This plugin is a downstream
-Red Hat fork of TopoLVM. This provisioner will create a new LVM logical volume in the `rhel` volume group for each
+Red Hat build of TopoLVM. This provisioner will create a new LVM logical volume in the `rhel` volume group for each
 PersistenVolumeClaim(PVC), and make these volumes available to pods. For more information on LVMS, visit the repo's
 [README](https://github.com/red-hat-storage/topolvm).
 
@@ -31,20 +31,22 @@ Full documentation of the config spec can be found at [github.com/red-hat-storag
 
 #### Path
 
-The user provided lvmd config should be written to the same directory as the MicroShift config.  If a MicroShift config
-doesn't exist, MicroShift will assume default lvmd values. These paths will be checked for the config, depending on the user MicroShift
-is run as.
-
-1. User config dir: `~/.microshift/lvmd.yaml`
-2. Global config dir: `/etc/microshift/lvmd.yaml`
+The user provided lvmd config should be written to the same directory as the MicroShift config.  If an lvmd configuration file
+does not exist in `/etc/microshift/lvmd.yaml`, MicroShift will use default values.
 
 ## System Requirements
 
-### Volume Group Name
+### Default Volume Group
+
+If there is only one volume group on the system, LVMS uses it by
+default. If there are multiple volume groups, and no configuration
+file, LVMS looks for a volume group named `microshift`. If there is no
+volume group named `microshift`, LVMS is disabled.
 
-The default integration of LVMS assumes a volume-group named `rhel`. LVMS's node-controller expects that volume
-group to exist prior to launching the service. If the volume group does not exist, the node-controller will fail to
-start and enter a CrashLoopBackoff state.
+LVMS expects all volume groups to exist prior to launching the
+service. If LVMS is configured to use a volume group that does not
+exist, the node-controller Pod will fail and enter a CrashLoopBackoff
+state.
 
 ### Volume Size Increments
 

pkg/components/render.go

@@ -54,7 +54,11 @@ func renderLvmdParams(l *lvmd.Lvmd) (assets.RenderParams, error) {
 	if err != nil {
 		return nil, err
 	}
-	r["lvmd"] = string(b)
+	content := string(b)
+	if l.Message != "" {
+		content = fmt.Sprintf("# %s\n%s", l.Message, content)
+	}
+	r["lvmd"] = content
 	r["SocketName"] = l.SocketName
 	return r, nil
 }

pkg/components/storage.go

@@ -21,7 +21,7 @@ func getCSIPluginConfig() (*lvmd.Lvmd, error) {
 	if _, err := os.Stat(lvmdConfig); !errors.Is(err, os.ErrNotExist) {
 		return lvmd.NewLvmdConfigFromFile(lvmdConfig)
 	}
-	return (&lvmd.Lvmd{}).WithDefaults(), nil
+	return lvmd.DefaultLvmdConfig()
 }
 
 func startCSIPlugin(cfg *config.MicroshiftConfig, kubeconfigPath string) error {
@@ -81,6 +81,10 @@ func startCSIPlugin(cfg *config.MicroshiftConfig, kubeconfigPath string) error {
 	if err != nil {
 		return err
 	}
+	if !lvmdCfg.IsEnabled() {
+		klog.V(2).Info("CSI is disabled. %s", lvmdCfg.Message)
+		return nil
+	}
 	lvmdRenderParams, err := renderLvmdParams(lvmdCfg)
 	if err != nil {
 		return fmt.Errorf("rendering lvmd params: %v", err)

pkg/config/lvmd/lvmd.go

@@ -3,34 +3,124 @@ package lvmd
 import (
 	"fmt"
 	"os"
+	"os/exec"
+	"strings"
 
 	"github.com/ghodss/yaml"
+	"k8s.io/klog/v2"
 )
 
 const (
 	LvmdConfigFileName          = "lvmd.yaml"
 	defaultSockName             = "/run/lvmd/lvmd.socket"
-	defaultRHEL4EdgeVolumeGroup = "rhel"
+	defaultRHEL4EdgeVolumeGroup = "microshift"
+
+	errorMessageNoVolumeGroups       = "No volume groups found"
+	errorMessageMultipleVolumeGroups = "Multiple volume groups are available, but no configuration file was provided."
+	statusMessageFoundDefault        = "Found default volume group \"microshift\""
+	statusMessageDefaultAvailable    = "Defaulting to the only available volume group"
 )
 
 // Lvmd stores the read-in or defaulted values of the lvmd configuration and provides the topolvm-node process information
 // about its host's storage environment.
 type Lvmd struct {
 	DeviceClasses []*DeviceClass `json:"device-classes"`
 	SocketName    string         `json:"socket-name"`
+	Message       string         `json:"-"`
+}
+
+// IsEnabled returns a boolean indicating whether the CSI driver
+// should be enabled for this host.
+func (l *Lvmd) IsEnabled() bool {
+	return len(l.DeviceClasses) > 0
 }
 
-func (l *Lvmd) WithDefaults() *Lvmd {
-	l.SocketName = defaultSockName
-	l.DeviceClasses = []*DeviceClass{
+func uint64Ptr(val uint64) *uint64 {
+	return &val
+}
+
+func getLvmdConfigForVGs(vgNames []string) (*Lvmd, error) {
+	response := &Lvmd{
+		SocketName: defaultSockName,
+	}
+
+	vgName := ""
+	if len(vgNames) == 0 {
+
+		response.Message = errorMessageNoVolumeGroups
+		klog.V(2).Info(errorMessageNoVolumeGroups)
+		return response, nil
+
+	} else if len(vgNames) == 1 {
+
+		vgName = vgNames[0]
+		klog.V(2).Infof("Using volume group %q", vgName)
+		response.Message = statusMessageDefaultAvailable
+
+	} else {
+
+		for _, name := range vgNames {
+			if name == defaultRHEL4EdgeVolumeGroup {
+				klog.V(2).Infof("Using default volume group %q", defaultRHEL4EdgeVolumeGroup)
+				vgName = name
+				response.Message = statusMessageFoundDefault
+				break
+			}
+		}
+
+		// If the default volume group is not found and there are
+		// multiple volume groups, disable the CSI driver.
+		if vgName == "" {
+			klog.V(2).Infof("Multiple volume groups available but no configuration file is present, disabling CSI. %v", vgNames)
+			response.Message = errorMessageMultipleVolumeGroups
+			return response, nil
+		}
+	}
+
+	// Fill in the default device class using the selected volume
+	// group.
+	response.DeviceClasses = []*DeviceClass{
 		{
 			Name:        "default",
-			VolumeGroup: defaultRHEL4EdgeVolumeGroup,
+			VolumeGroup: vgName,
 			Default:     true,
-			SpareGB:     func() *uint64 { s := uint64(defaultSpareGB); return &s }(),
+			SpareGB:     uint64Ptr(defaultSpareGB),
 		},
 	}
-	return l
+	return response, nil
+}
+
+// DefaultLvmdConfig returns a configuration struct for Lvmd with
+// default settings based on the current host. If a single volume
+// group is found, that value is used. If multiple volume groups are
+// available and one is named "rhel", that group is used. Otherwise,
+// the configuration returned will report that it is not enabled (see
+// IsEnabled()).
+func DefaultLvmdConfig() (*Lvmd, error) {
+	vgNames, err := getVolumeGroups()
+	if err != nil {
+		return nil, fmt.Errorf("Failed to discover local volume groups: %s", err)
+	}
+
+	return getLvmdConfigForVGs(vgNames)
+
+}
+
+// getVolumeGroups returns a slice of volume group names.
+func getVolumeGroups() ([]string, error) {
+	cmd := exec.Command("vgs", "--readonly", "--options=name", "--noheadings")
+	output, err := cmd.Output()
+	if err != nil {
+		return nil, fmt.Errorf("error running vgs: %s", err)
+	}
+	names := []string{}
+	for _, line := range strings.Split(string(output), "\n") {
+		newName := strings.Trim(line, " \t\n")
+		if newName != "" {
+			names = append(names, newName)
+		}
+	}
+	return names, nil
 }
 
 func NewLvmdConfigFromFile(p string) (*Lvmd, error) {
@@ -47,5 +137,6 @@ func NewLvmdConfigFromFile(p string) (*Lvmd, error) {
 	if l.SocketName == "" {
 		l.SocketName = defaultSockName
 	}
+	l.Message = fmt.Sprintf("Read from %s", p)
 	return l, nil
 }

pkg/config/lvmd/lvmd_test.go

@@ -4,8 +4,111 @@ import (
 	"encoding/json"
 	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
+func TestGetLvmdConfigForVGs(t *testing.T) {
+	tests := []struct {
+		name        string
+		vgNames     []string
+		expected    *Lvmd
+		expectedErr error
+	}{
+		{
+			name: "no groups",
+			expected: &Lvmd{
+				SocketName: defaultSockName,
+				Message:    errorMessageNoVolumeGroups,
+			},
+		},
+		{
+			name:    "one group",
+			vgNames: []string{"choose-me"},
+			expected: &Lvmd{
+				SocketName: defaultSockName,
+				DeviceClasses: []*DeviceClass{
+					{
+						Name:        "default",
+						VolumeGroup: "choose-me",
+						Default:     true,
+						SpareGB:     func() *uint64 { s := uint64(defaultSpareGB); return &s }(),
+					},
+				},
+				Message: statusMessageDefaultAvailable,
+			},
+		},
+		{
+			name:    "one group default",
+			vgNames: []string{defaultRHEL4EdgeVolumeGroup},
+			expected: &Lvmd{
+				SocketName: defaultSockName,
+				DeviceClasses: []*DeviceClass{
+					{
+						Name:        "default",
+						VolumeGroup: defaultRHEL4EdgeVolumeGroup,
+						Default:     true,
+						SpareGB:     func() *uint64 { s := uint64(defaultSpareGB); return &s }(),
+					},
+				},
+				Message: statusMessageDefaultAvailable,
+			},
+		},
+		{
+			name:    "default first",
+			vgNames: []string{defaultRHEL4EdgeVolumeGroup, "other"},
+			expected: &Lvmd{
+				SocketName: defaultSockName,
+				DeviceClasses: []*DeviceClass{
+					{
+						Name:        "default",
+						VolumeGroup: defaultRHEL4EdgeVolumeGroup,
+						Default:     true,
+						SpareGB:     func() *uint64 { s := uint64(defaultSpareGB); return &s }(),
+					},
+				},
+				Message: statusMessageFoundDefault,
+			},
+		},
+		{
+			name:    "default last",
+			vgNames: []string{"other", defaultRHEL4EdgeVolumeGroup},
+			expected: &Lvmd{
+				SocketName: defaultSockName,
+				DeviceClasses: []*DeviceClass{
+					{
+						Name:        "default",
+						VolumeGroup: defaultRHEL4EdgeVolumeGroup,
+						Default:     true,
+						SpareGB:     uint64Ptr(defaultSpareGB),
+					},
+				},
+				Message: statusMessageFoundDefault,
+			},
+		},
+		{
+			name:    "no default",
+			vgNames: []string{"other", "choose-me"},
+			expected: &Lvmd{
+				SocketName: defaultSockName,
+				Message:    errorMessageMultipleVolumeGroups,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			actual, err := getLvmdConfigForVGs(tt.vgNames)
+			assert.Equal(t, tt.expected, actual, "names: %v", tt.vgNames)
+			if tt.expectedErr != nil {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
 func Test_newLvmdConfigFromFile(t *testing.T) {
 
 	iToP := func(i int) *uint64 {
@@ -51,6 +154,7 @@ func Test_newLvmdConfigFromFile(t *testing.T) {
 						SpareGB:     iToP(5),
 					},
 				},
+				Message: "Read from ./test/lvmd.yaml",
 			},
 			wantErr: false,
 		},