From c584fa20da9f024ccde6f40bc48a5fa46d13d13d Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Fri, 8 Oct 2021 13:58:04 +0530 Subject: [PATCH] rbd: use clusterID from volumeContext at nodestage previously we were retriving clusterID using the monitors field in the volume context at node stage code path. however it is possible to retrieve or use clusterID directly from the volume context. This commit also remove the getClusterIDFromMigrationVolume() function which was used previously and its tests Signed-off-by: Humble Chirammal --- internal/rbd/nodeserver.go | 28 --------------------- internal/util/csiconfig.go | 44 --------------------------------- internal/util/csiconfig_test.go | 32 ------------------------ 3 files changed, 104 deletions(-) diff --git a/internal/rbd/nodeserver.go b/internal/rbd/nodeserver.go index 96d94de52..846668e6f 100644 --- a/internal/rbd/nodeserver.go +++ b/internal/rbd/nodeserver.go @@ -150,24 +150,6 @@ func healerStageTransaction(ctx context.Context, cr *util.Credentials, volOps *r return nil } -// getClusterIDFromMigrationVolume fills the clusterID for the passed in monitors. -func getClusterIDFromMigrationVolume(monitors string) (string, error) { - var err error - var rclusterID string - for _, m := range strings.Split(monitors, ",") { - rclusterID, err = util.GetClusterIDFromMon(m) - if err != nil && !errors.Is(err, util.ErrMissingConfigForMonitor) { - return "", err - } - - if rclusterID != "" { - return rclusterID, nil - } - } - - return "", err -} - // populateRbdVol update the fields in rbdVolume struct based on the request it received. func populateRbdVol( ctx context.Context, @@ -291,16 +273,6 @@ func (ns *NodeServer) NodeStageVolume( } defer ns.VolumeLocks.Release(volID) - // Check this is a migration request because in that case, unlike other node stage requests - // it will be missing the clusterID, so fill it by fetching it from config file using mon. - if req.GetVolumeContext()[intreeMigrationKey] == intreeMigrationLabel && req.VolumeContext[util.ClusterIDKey] == "" { - cID, cErr := getClusterIDFromMigrationVolume(req.GetVolumeContext()["monitors"]) - if cErr != nil { - return nil, status.Error(codes.Internal, cErr.Error()) - } - req.VolumeContext[util.ClusterIDKey] = cID - } - stagingParentPath := req.GetStagingTargetPath() stagingTargetPath := stagingParentPath + "/" + volID diff --git a/internal/util/csiconfig.go b/internal/util/csiconfig.go index 6e45d04a6..874ff5269 100644 --- a/internal/util/csiconfig.go +++ b/internal/util/csiconfig.go @@ -161,47 +161,3 @@ func GetClusterID(options map[string]string) (string, error) { return clusterID, nil } - -// GetClusterIDFromMon will be called with a mon string to fetch -// clusterID based on the passed in mon string. If passed in 'mon' -// string has been found in the config the clusterID is returned, -// else error. -func GetClusterIDFromMon(mon string) (string, error) { - clusterID, err := readClusterInfoWithMon(CsiConfigFile, mon) - - return clusterID, err -} - -func readClusterInfoWithMon(pathToConfig, mon string) (string, error) { - var config []ClusterInfo - - // #nosec - content, err := ioutil.ReadFile(pathToConfig) - if err != nil { - err = fmt.Errorf("error fetching configuration file %q: %w", pathToConfig, err) - - return "", err - } - - err = json.Unmarshal(content, &config) - if err != nil { - return "", fmt.Errorf("unmarshal failed (%w), raw buffer response: %s", - err, string(content)) - } - - for _, cluster := range config { - // as the same mons can fall into different clusterIDs with - // different radosnamespace configurations, we are bailing out - // if radosnamespace configuration is found for this cluster - if cluster.RadosNamespace != "" { - continue - } - for _, m := range cluster.Monitors { - if m == mon { - return cluster.ClusterID, nil - } - } - } - - return "", ErrMissingConfigForMonitor -} diff --git a/internal/util/csiconfig_test.go b/internal/util/csiconfig_test.go index cdd199cdd..a946dd76c 100644 --- a/internal/util/csiconfig_test.go +++ b/internal/util/csiconfig_test.go @@ -34,7 +34,6 @@ func cleanupTestData() { os.RemoveAll(basePath) } -// nolint:gocyclo,cyclop // TODO: make this function less complex. func TestCSIConfig(t *testing.T) { t.Parallel() var err error @@ -139,35 +138,4 @@ func TestCSIConfig(t *testing.T) { if err != nil { t.Errorf("Test setup error %s", err) } - - // TEST: Should pass as clusterID is present in config - content, err = readClusterInfoWithMon(pathToConfig, "mon1") - if err != nil || content != "test2" { - t.Errorf("Failed: want (%s), got (%s) (%v)", "test2", content, err) - } - - // TEST: Should pass as clusterID is present in config - content, err = readClusterInfoWithMon(pathToConfig, "mon5") - if err != nil || content != "test1" { - t.Errorf("Failed: want (%s), got (%s) (%v)", "test1", content, err) - } - - // TEST: Should fail as clusterID is not present in config - content, err = readClusterInfoWithMon(pathToConfig, "mon8") - if err == nil { - t.Errorf("Failed: got (%s)", content) - } - - data = "[{\"clusterID\":\"" + clusterID2 + "\", \"radosNamespace\": \"ns1\", \"monitors\":[\"mon1\"]}," + - "{\"clusterID\":\"" + clusterID1 + "\",\"monitors\":[\"mon1\"]}]" - err = ioutil.WriteFile(basePath+"/"+csiClusters, []byte(data), 0o600) - if err != nil { - t.Errorf("Test setup error %s", err) - } - - // TEST: Should pass as clusterID is present in config - content, err = readClusterInfoWithMon(pathToConfig, "mon1") - if err != nil || content != clusterID1 { - t.Errorf("Failed: want (%s), got (%s) (%v)", "test2", content, err) - } }