mirrors/ceph-csi
1
0
Fork 0
mirror of https://github.com/ceph/ceph-csi.git synced 2024-11-27 16:50:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

util: fix unit-test for GetClusterMappingInfo()

Browse Source 
Unit-testing often fails due to a race condition while writing the
clusterMappingConfigFile from multiple go-routines at the same time.
Failures from `make containerized-test` look like this:

    === CONT  TestGetClusterMappingInfo/site2-storage_cluster-id_mapping
        cluster_mapping_test.go:153: GetClusterMappingInfo() = <nil>, expected data &[{map[site1-storage:site2-storage] [map[1:3]] [map[11:5]]} {map[site3-storage:site2-storage] [map[8:3]] [map[10:5]]}]
    === CONT  TestGetClusterMappingInfo/site3-storage_cluster-id_mapping
        cluster_mapping_test.go:153: GetClusterMappingInfo() = <nil>, expected data &[{map[site3-storage:site2-storage] [map[8:3]] [map[10:5]]}]
    --- FAIL: TestGetClusterMappingInfo (0.01s)
        --- PASS: TestGetClusterMappingInfo/mapping_file_not_found (0.00s)
        --- PASS: TestGetClusterMappingInfo/mapping_file_found_with_empty_data (0.00s)
        --- PASS: TestGetClusterMappingInfo/cluster-id_mapping_not_found (0.00s)
        --- FAIL: TestGetClusterMappingInfo/site2-storage_cluster-id_mapping (0.00s)
        --- FAIL: TestGetClusterMappingInfo/site3-storage_cluster-id_mapping (0.00s)
        --- PASS: TestGetClusterMappingInfo/site1-storage_cluster-id_mapping (0.00s)

By splitting the public GetClusterMappingInfo() function into an
internal getClusterMappingInfo() that takes a filename, unit-testing can
use different files for each go-routine, and testing becomes more
predictable.

Signed-off-by: Niels de Vos <ndevos@redhat.com>
(cherry picked from commit 8b71671b42665789fac4a4aa1453b0b107f475c6)
This commit is contained in:
Niels de Vos 2021-08-24 17:23:51 +02:00 committed by mergify[bot]
parent 82b6857688
commit d42814dfb9
2 changed files with 25 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
internal/util/cluster_mapping.go
View File

@ -65,9 +65,9 @@ type ClusterMappingInfo struct {
// ... // ...
// }] // }]
func readClusterMappingInfo() (*[]ClusterMappingInfo, error) { func readClusterMappingInfo(filename string) (*[]ClusterMappingInfo, error) {
var info []ClusterMappingInfo var info []ClusterMappingInfo
content, err := ioutil.ReadFile(clusterMappingConfigFile) content, err := ioutil.ReadFile(filename) // #nosec:G304, file inclusion via variable.
if err != nil { if err != nil {
err = fmt.Errorf("error fetching clusterID mapping %w", err) err = fmt.Errorf("error fetching clusterID mapping %w", err)
@ -83,11 +83,11 @@ func readClusterMappingInfo() (*[]ClusterMappingInfo, error) {
return &info, nil return &info, nil
} }
// GetClusterMappingInfo returns corresponding cluster details like clusterID's // getClusterMappingInfo returns corresponding cluster details like clusterID's
// poolID,fscID lists read from configfile. // poolID,fscID lists read from 'filename'.
func GetClusterMappingInfo(clusterID string) (*[]ClusterMappingInfo, error) { func getClusterMappingInfo(clusterID, filename string) (*[]ClusterMappingInfo, error) {
var mappingInfo []ClusterMappingInfo var mappingInfo []ClusterMappingInfo
info, err := readClusterMappingInfo() info, err := readClusterMappingInfo(filename)
if err != nil { if err != nil {
// discard not found error as this file is expected to be created by // discard not found error as this file is expected to be created by
// the admin in case of failover. // the admin in case of failover.
@ -114,3 +114,9 @@ func GetClusterMappingInfo(clusterID string) (*[]ClusterMappingInfo, error) {
return &mappingInfo, nil return &mappingInfo, nil
} }
// GetClusterMappingInfo returns corresponding cluster details like clusterID's
// poolID,fscID lists read from configfile.
func GetClusterMappingInfo(clusterID string) (*[]ClusterMappingInfo, error) {
return getClusterMappingInfo(clusterID, clusterMappingConfigFile)
}

27
internal/util/cluster_mapping_test.go
View File

@ -140,25 +140,24 @@ func TestGetClusterMappingInfo(t *testing.T) {
expectErr: false, expectErr: false,
}, },
} }
for _, tt := range tests { for i, tt := range tests {
tt := tt currentI := i
t.Run(tt.name, func(t *testing.T) { currentTT := tt
t.Run(currentTT.name, func(t *testing.T) {
t.Parallel() t.Parallel()
if tt.createMappingConfigFile { mappingConfigFile := fmt.Sprintf("%s/mapping-%d.json", mappingBasePath, currentI)
clusterMappingConfigFile = fmt.Sprintf("%s/mapping.json", mappingBasePath) if len(currentTT.mappingFilecontent) != 0 {
} err = ioutil.WriteFile(mappingConfigFile, currentTT.mappingFilecontent, 0o600)
if len(tt.mappingFilecontent) != 0 {
err = ioutil.WriteFile(clusterMappingConfigFile, tt.mappingFilecontent, 0o600)
if err != nil { if err != nil {
t.Errorf("GetClusterMappingInfo() error = %v", err) t.Errorf("failed to write to %q, error = %v", mappingConfigFile, err)
} }
} }
data, mErr := GetClusterMappingInfo(tt.clusterID) data, mErr := getClusterMappingInfo(currentTT.clusterID, mappingConfigFile)
if (mErr != nil) != tt.expectErr { if (mErr != nil) != currentTT.expectErr {
t.Errorf("GetClusterMappingInfo() error = %v, expected Error %v", mErr, tt.expectErr) t.Errorf("getClusterMappingInfo() error = %v, expected Error %v", mErr, currentTT.expectErr)
} }
if !reflect.DeepEqual(data, tt.expectedData) { if !reflect.DeepEqual(data, currentTT.expectedData) {
t.Errorf("GetClusterMappingInfo() = %v, expected data %v", data, tt.expectedData) t.Errorf("getClusterMappingInfo() = %v, expected data %v", data, currentTT.expectedData)
} }
}) })
} }