From e874c9c11bda386dfa21c0edf699e90df681e640 Mon Sep 17 00:00:00 2001 From: Thibaut Blanchard Date: Tue, 8 Mar 2022 13:45:01 +0100 Subject: [PATCH] rbd: fix topology snapshot pool Restoring a snapshot with a new PVC results with a wrong dataPoolName in case of initial volume linked to a storageClass with topology constraints and erasure coding. Signed-off-by: Thibaut Blanchard --- internal/rbd/rbd_journal.go | 10 +++++++--- internal/util/topology.go | 14 ++++++-------- internal/util/topology_test.go | 8 ++++---- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go index e2a50354e..4e2d03c6f 100644 --- a/internal/rbd/rbd_journal.go +++ b/internal/rbd/rbd_journal.go @@ -267,7 +267,7 @@ func (rv *rbdVolume) Exists(ctx context.Context, parentVol *rbdVolume) (bool, er rv.RbdImageName = imageData.ImageAttributes.ImageName rv.ImageID = imageData.ImageAttributes.ImageID // check if topology constraints match what is found - rv.Topology, err = util.MatchTopologyForPool(rv.TopologyPools, rv.TopologyRequirement, + _, _, rv.Topology, err = util.MatchPoolAndTopology(rv.TopologyPools, rv.TopologyRequirement, imageData.ImagePool) if err != nil { // TODO check if need any undo operation here, or ErrVolNameConflict @@ -412,7 +412,10 @@ func updateTopologyConstraints(rbdVol *rbdVolume, rbdSnap *rbdSnapshot) error { var err error if rbdSnap != nil { // check if topology constraints matches snapshot pool - rbdVol.Topology, err = util.MatchTopologyForPool(rbdVol.TopologyPools, + var poolName string + var dataPoolName string + + poolName, dataPoolName, rbdVol.Topology, err = util.MatchPoolAndTopology(rbdVol.TopologyPools, rbdVol.TopologyRequirement, rbdSnap.Pool) if err != nil { return err @@ -420,7 +423,8 @@ func updateTopologyConstraints(rbdVol *rbdVolume, rbdSnap *rbdSnapshot) error { // update Pool, if it was topology constrained if rbdVol.Topology != nil { - rbdVol.Pool = rbdSnap.Pool + rbdVol.Pool = poolName + rbdVol.DataPool = dataPoolName } return nil diff --git a/internal/util/topology.go b/internal/util/topology.go index 51b61fbe4..73ae9728b 100644 --- a/internal/util/topology.go +++ b/internal/util/topology.go @@ -168,14 +168,14 @@ func GetTopologyFromRequest( return &topologyPools, accessibilityRequirements, nil } -// MatchTopologyForPool returns the topology map, if the passed in pool matches any +// MatchPoolAndTopology returns the topology map, if the passed in pool matches any // passed in accessibility constraints. -func MatchTopologyForPool(topologyPools *[]TopologyConstrainedPool, - accessibilityRequirements *csi.TopologyRequirement, poolName string) (map[string]string, error) { +func MatchPoolAndTopology(topologyPools *[]TopologyConstrainedPool, + accessibilityRequirements *csi.TopologyRequirement, poolName string) (string, string, map[string]string, error) { var topologyPool []TopologyConstrainedPool if topologyPools == nil || accessibilityRequirements == nil { - return nil, nil + return "", "", nil, nil } // find the pool in the list of topology based pools @@ -187,13 +187,11 @@ func MatchTopologyForPool(topologyPools *[]TopologyConstrainedPool, } } if len(topologyPool) == 0 { - return nil, fmt.Errorf("none of the configured topology pools (%+v) matched passed in pool name (%s)", + return "", "", nil, fmt.Errorf("none of the configured topology pools (%+v) matched passed in pool name (%s)", topologyPools, poolName) } - _, _, topology, err := FindPoolAndTopology(&topologyPool, accessibilityRequirements) - - return topology, err + return FindPoolAndTopology(&topologyPool, accessibilityRequirements) } // FindPoolAndTopology loops through passed in "topologyPools" and also related diff --git a/internal/util/topology_test.go b/internal/util/topology_test.go index 71c5416b6..32ead00f9 100644 --- a/internal/util/topology_test.go +++ b/internal/util/topology_test.go @@ -37,7 +37,7 @@ func checkAndReportError(t *testing.T, msg string, err error) { } } -// TestFindPoolAndTopology also tests MatchTopologyForPool. +// TestFindPoolAndTopology also tests MatchPoolAndTopology. func TestFindPoolAndTopology(t *testing.T) { t.Parallel() var err error @@ -319,15 +319,15 @@ func TestFindPoolAndTopology(t *testing.T) { t.Errorf("expected data pool to be named ec-%s, got %s", poolName, dataPoolName) } - // TEST: MatchTopologyForPool + // TEST: MatchPoolAndTopology // check for non-existent pool - _, err = MatchTopologyForPool(&validMultipleTopoPools, &validAccReq, pool1+"fuzz") + _, _, _, err = MatchPoolAndTopology(&validMultipleTopoPools, &validAccReq, pool1+"fuzz") if err == nil { t.Errorf("expected failure due to non-existent pool name (%s) got success", pool1+"fuzz") } // check for existing pool - topoSegment, err = MatchTopologyForPool(&validMultipleTopoPools, &validAccReq, pool1) + _, _, topoSegment, err = MatchPoolAndTopology(&validMultipleTopoPools, &validAccReq, pool1) err = checkOutput(err, pool1, topoSegment) checkAndReportError(t, "expected success got:", err) }