fix: multiple fixes for LVM activation

Two fixes were in pkgs/lvm2: * siderolabs/pkgs#1041 * siderolabs/pkgs#1042 Other fixes in this PR: * adjust the controller a bit for some interactions * make Rook test use more complicated, encrypted setup which uses LVM * adjust LVM test to handle a case when there's more than one worker Signed-off-by: Andrey Smirnov <[email protected]>
siderolabs · Oct 3, 2024 · 7486157 · 7486157
1 parent 74c12c2
commit 7486157
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 42 deletions.
diff --git a/Makefile b/Makefile
@@ -20,7 +20,7 @@ ARTIFACTS := _out
 TOOLS ?= ghcr.io/siderolabs/tools:v1.9.0-alpha.0-2-g9f2189b
 
 PKGS_PREFIX ?= ghcr.io/siderolabs
-PKGS ?= v1.9.0-alpha.0-11-g232a153
+PKGS ?= v1.9.0-alpha.0-13-ge2a561f
 EXTRAS ?= v1.9.0-alpha.0
 
 KRES_IMAGE ?= ghcr.io/siderolabs/kres:latest

diff --git a/internal/app/machined/pkg/controllers/block/lvm.go b/internal/app/machined/pkg/controllers/block/lvm.go
@@ -11,16 +11,11 @@ import (
 
 	"github.com/cosi-project/runtime/pkg/controller"
 	"github.com/cosi-project/runtime/pkg/safe"
-	"github.com/cosi-project/runtime/pkg/state"
 	"github.com/hashicorp/go-multierror"
-	"github.com/siderolabs/gen/optional"
 	"github.com/siderolabs/go-cmd/pkg/cmd"
 	"go.uber.org/zap"
 
-	"github.com/siderolabs/talos/pkg/machinery/constants"
 	"github.com/siderolabs/talos/pkg/machinery/resources/block"
-	runtimeres "github.com/siderolabs/talos/pkg/machinery/resources/runtime"
-	"github.com/siderolabs/talos/pkg/machinery/resources/v1alpha1"
 )
 
 // LVMActivationController activates LVM volumes when they are discovered by the block.DiscoveryController.
@@ -37,12 +32,6 @@ func (ctrl *LVMActivationController) Name() string {
 // Inputs implements controller.Controller interface.
 func (ctrl *LVMActivationController) Inputs() []controller.Input {
 	return []controller.Input{
-		{
-			Namespace: v1alpha1.NamespaceName,
-			Type:      runtimeres.MountStatusType,
-			ID:        optional.Some(constants.EphemeralPartitionLabel),
-			Kind:      controller.InputWeak,
-		},
 		{
 			Namespace: block.NamespaceName,
 			Type:      block.DiscoveredVolumeType,
@@ -75,15 +64,6 @@ func (ctrl *LVMActivationController) Run(ctx context.Context, r controller.Runti
 		case <-r.EventCh():
 		}
 
-		if _, err := safe.ReaderGetByID[*runtimeres.MountStatus](ctx, r, constants.EphemeralPartitionLabel); err != nil {
-			if state.IsNotFoundError(err) {
-				// wait for the mount status to be available
-				continue
-			}
-
-			return fmt.Errorf("failed to get mount status: %w", err)
-		}
-
 		discoveredVolumes, err := safe.ReaderListAll[*block.DiscoveredVolume](ctx, r)
 		if err != nil {
 			return fmt.Errorf("failed to list discovered volumes: %w", err)
@@ -92,17 +72,19 @@ func (ctrl *LVMActivationController) Run(ctx context.Context, r controller.Runti
 		var multiErr error
 
 		for iterator := discoveredVolumes.Iterator(); iterator.Next(); {
-			if _, ok := ctrl.seenVolumes[iterator.Value().Metadata().ID()]; ok {
-				continue
-			}
-
 			if iterator.Value().TypedSpec().Name != "lvm2-pv" {
+				// if the volume is not an LVM volume the moment we saw it, we can skip it
+				// we need to activate the volumes only on reboot, not when they are first formatted
 				ctrl.seenVolumes[iterator.Value().Metadata().ID()] = struct{}{}
 
 				continue
 			}
 
-			logger.Info("checking device for LVM volume activation", zap.String("device", iterator.Value().TypedSpec().DevPath))
+			if _, ok := ctrl.seenVolumes[iterator.Value().Metadata().ID()]; ok {
+				continue
+			}
+
+			logger.Debug("checking device for LVM volume activation", zap.String("device", iterator.Value().TypedSpec().DevPath))
 
 			vgName, err := ctrl.checkVGNeedsActivation(ctx, iterator.Value().TypedSpec().DevPath)
 			if err != nil {
@@ -112,8 +94,6 @@ func (ctrl *LVMActivationController) Run(ctx context.Context, r controller.Runti
 			}
 
 			if vgName == "" {
-				ctrl.seenVolumes[iterator.Value().Metadata().ID()] = struct{}{}
-
 				continue
 			}
 
@@ -132,10 +112,10 @@ func (ctrl *LVMActivationController) Run(ctx context.Context, r controller.Runti
 				"event",
 				vgName,
 			); err != nil {
-				return fmt.Errorf("failed to activate LVM volume %s: %w", vgName, err)
+				multiErr = multierror.Append(multiErr, fmt.Errorf("failed to activate LVM volume %s: %w", vgName, err))
+			} else {
+				ctrl.activatedVGs[vgName] = struct{}{}
 			}
-
-			ctrl.activatedVGs[vgName] = struct{}{}
 		}
 
 		if multiErr != nil {
@@ -153,7 +133,6 @@ func (ctrl *LVMActivationController) checkVGNeedsActivation(ctx context.Context,
 		"/sbin/lvm",
 		"pvscan",
 		"--cache",
-		"--verbose",
 		"--listvg",
 		"--checkcomplete",
 		"--vgonline",
@@ -166,11 +145,19 @@ func (ctrl *LVMActivationController) checkVGNeedsActivation(ctx context.Context,
 		return "", fmt.Errorf("failed to check if LVM volume backed by device %s needs activation: %w", devicePath, err)
 	}
 
-	if strings.HasPrefix(stdOut, "LVM_VG_NAME_INCOMPLETE") {
-		return "", nil
-	}
+	// parse the key-value pairs from the udev output
+	for _, line := range strings.Split(stdOut, "\n") {
+		key, value, ok := strings.Cut(line, "=")
+		if !ok {
+			continue
+		}
 
-	vgName := strings.TrimSuffix(strings.TrimPrefix(strings.TrimSuffix(stdOut, "\n"), "LVM_VG_NAME_COMPLETE='"), "'")
+		value = strings.Trim(value, "'\"")
+
+		if key == "LVM_VG_NAME_COMPLETE" {
+			return value, nil
+		}
+	}
 
-	return vgName, nil
+	return "", nil
 }
diff --git a/internal/integration/api/volumes.go b/internal/integration/api/volumes.go
@@ -191,6 +191,13 @@ func (suite *VolumesSuite) TestLVMActivation() {
 
 	node := suite.RandomDiscoveredNodeInternalIP(machine.TypeWorker)
 
+	k8sNode, err := suite.GetK8sNodeByInternalIP(suite.ctx, node)
+	suite.Require().NoError(err)
+
+	nodeName := k8sNode.Name
+
+	suite.T().Logf("creating LVM volume group on node %s/%s", node, nodeName)
+
 	userDisks, err := suite.UserDisks(suite.ctx, node)
 	suite.Require().NoError(err)
 
@@ -201,6 +208,8 @@ func (suite *VolumesSuite) TestLVMActivation() {
 	podDef, err := suite.NewPrivilegedPod("pv-create")
 	suite.Require().NoError(err)
 
+	podDef = podDef.WithNodeName(nodeName)
+
 	suite.Require().NoError(podDef.Create(suite.ctx, 5*time.Minute))
 
 	defer podDef.Delete(suite.ctx) //nolint:errcheck
@@ -230,9 +239,13 @@ func (suite *VolumesSuite) TestLVMActivation() {
 	suite.Require().Contains(stdout, "Logical volume \"lv1\" created.")
 
 	defer func() {
+		suite.T().Logf("removing LVM volumes %s/%s", node, nodeName)
+
 		deletePodDef, err := suite.NewPrivilegedPod("pv-destroy")
 		suite.Require().NoError(err)
 
+		deletePodDef = deletePodDef.WithNodeName(nodeName)
+
 		suite.Require().NoError(deletePodDef.Create(suite.ctx, 5*time.Minute))
 
 		defer deletePodDef.Delete(suite.ctx) //nolint:errcheck
@@ -252,21 +265,23 @@ func (suite *VolumesSuite) TestLVMActivation() {
 		}
 	}()
 
+	suite.T().Logf("rebooting node %s/%s", node, nodeName)
+
 	// now we want to reboot the node and make sure the array is still mounted
 	suite.AssertRebooted(
 		suite.ctx, node, func(nodeCtx context.Context) error {
 			return base.IgnoreGRPCUnavailable(suite.Client.Reboot(nodeCtx))
 		}, 5*time.Minute,
 	)
 
+	suite.T().Logf("verifying LVM activation %s/%s", node, nodeName)
+
 	suite.Require().Eventually(func() bool {
-		return suite.lvmVolumeExists()
+		return suite.lvmVolumeExists(node)
 	}, 5*time.Second, 1*time.Second, "LVM volume group was not activated after reboot")
 }
 
-func (suite *VolumesSuite) lvmVolumeExists() bool {
-	node := suite.RandomDiscoveredNodeInternalIP(machine.TypeWorker)
-
+func (suite *VolumesSuite) lvmVolumeExists(node string) bool {
 	ctx := client.WithNode(suite.ctx, node)
 
 	disks, err := safe.StateListAll[*block.Disk](ctx, suite.Client.COSI)

diff --git a/internal/integration/base/k8s.go b/internal/integration/base/k8s.go
@@ -200,6 +200,7 @@ func (k8sSuite *K8sSuite) WaitForEventExists(ctx context.Context, ns string, che
 
 type podInfo interface {
 	Name() string
+	WithNodeName(nodeName string) podInfo
 	Create(ctx context.Context, waitTimeout time.Duration) error
 	Delete(ctx context.Context) error
 	Exec(ctx context.Context, command string) (string, string, error)
@@ -217,6 +218,12 @@ func (p *pod) Name() string {
 	return p.name
 }
 
+func (p *pod) WithNodeName(nodeName string) podInfo {
+	p.pod.Spec.NodeName = nodeName
+
+	return p
+}
+
 func (p *pod) Create(ctx context.Context, waitTimeout time.Duration) error {
 	_, err := p.suite.Clientset.CoreV1().Pods(p.namespace).Create(ctx, p.pod, metav1.CreateOptions{})
 	if err != nil {

diff --git a/internal/integration/k8s/testdata/rook-ceph-cluster-values.yaml b/internal/integration/k8s/testdata/rook-ceph-cluster-values.yaml
@@ -15,3 +15,8 @@ cephClusterSpec:
       requests:
         cpu: "500m"
         memory: "1Gi"
+  storage:
+    useAllNodes: true
+    useAllDevices: true
+    config:
+      encryptedDevice: "true"
diff --git a/pkg/machinery/gendata/data/pkgs b/pkg/machinery/gendata/data/pkgs
@@ -1 +1 @@
-v1.9.0-alpha.0-11-g232a153
+v1.9.0-alpha.0-13-ge2a561f