Add concurrent zapi SVM rename unit test

Author: jharrod

storage_drivers/ontap/api/azgo/common.go

@@ -35,6 +35,55 @@ type ZAPIResponseIterable interface {
 	NextTag() string
 }
 
+type httpClientGetter interface {
+	getHttpClient() (*http.Client, error)
+}
+
+func (o *ZapiRunner) getHttpClient() (*http.Client, error) {
+	// Check to use cert/key and load the cert pair
+	var cert tls.Certificate
+	caCertPool := x509.NewCertPool()
+	skipVerify := true
+	if o.ClientCertificate != "" && o.ClientPrivateKey != "" {
+		certDecode, err := base64.StdEncoding.DecodeString(o.ClientCertificate)
+		if err != nil {
+			return nil, errors.New("failed to decode client certificate from base64")
+		}
+		keyDecode, err := base64.StdEncoding.DecodeString(o.ClientPrivateKey)
+		if err != nil {
+			return nil, errors.New("failed to decode private key from base64")
+		}
+		cert, err = tls.X509KeyPair(certDecode, keyDecode)
+		if err != nil {
+			log.Debugf("error: %v", err)
+			return nil, errors.New("cannot load certificate and key")
+		}
+	}
+
+	// Check to use trustedCACertificate to use InsecureSkipVerify or not
+	if o.TrustedCACertificate != "" {
+		trustedCACert, err := base64.StdEncoding.DecodeString(o.TrustedCACertificate)
+		if err != nil {
+			return nil, errors.New("failed to decode trusted CA certificate from base64")
+		}
+		skipVerify = false
+		caCertPool.AppendCertsFromPEM(trustedCACert)
+	}
+
+	tr := &http.Transport{
+		TLSClientConfig: &tls.Config{
+			InsecureSkipVerify: skipVerify, MinVersion: tridentconfig.MinClientTLSVersion,
+			Certificates: []tls.Certificate{cert}, RootCAs: caCertPool,
+		},
+	}
+
+	client := &http.Client{
+		Transport: tr,
+		Timeout:   time.Duration(tridentconfig.StorageAPITimeoutSeconds * time.Second),
+	}
+	return client, nil
+}
+
 type ZapiRunner struct {
 	ManagementLIF        string
 	svm                  string
@@ -47,12 +96,13 @@ type ZapiRunner struct {
 	ontapApiVersion      string
 	DebugTraceFlags      map[string]bool // Example: {"api":false, "method":true}
 	m                    *sync.RWMutex
+	clientGetter         httpClientGetter
 }
 
 func NewZapiRunner(managementLIF, svm, username, password, clientPrivateKey, clientCertificate,
 	clientCACert string, secure bool, ontapApiVersion string, debugTraceFlags map[string]bool,
 ) *ZapiRunner {
-	return &ZapiRunner{
+	zr := &ZapiRunner{
 		ManagementLIF:        managementLIF,
 		svm:                  svm,
 		Username:             username,
@@ -65,6 +115,8 @@ func NewZapiRunner(managementLIF, svm, username, password, clientPrivateKey, cli
 		DebugTraceFlags:      debugTraceFlags,
 		m:                    &sync.RWMutex{},
 	}
+	zr.clientGetter = zr // Use ZapiRunners own getter for HTTP clients
+	return zr
 }
 
 // CopyForNontunneledZapiRunner returns a clone of the ZapiRunner configured on this driver with the SVM field cleared
@@ -182,50 +234,14 @@ func (o *ZapiRunner) SendZapi(r ZAPIRequest) (*http.Response, error) {
 		return nil, err
 	}
 	req.Header.Set("Content-Type", "application/xml")
-
-	// Check to use cert/key and load the cert pair
-	var cert tls.Certificate
-	caCertPool := x509.NewCertPool()
-	skipVerify := true
-	if o.ClientCertificate != "" && o.ClientPrivateKey != "" {
-		certDecode, err := base64.StdEncoding.DecodeString(o.ClientCertificate)
-		if err != nil {
-			return nil, errors.New("failed to decode client certificate from base64")
-		}
-		keyDecode, err := base64.StdEncoding.DecodeString(o.ClientPrivateKey)
-		if err != nil {
-			return nil, errors.New("failed to decode private key from base64")
-		}
-		cert, err = tls.X509KeyPair(certDecode, keyDecode)
-		if err != nil {
-			log.Debugf("error: %v", err)
-			return nil, errors.New("cannot load certificate and key")
-		}
-	} else {
+	if o.ClientCertificate == "" || o.ClientPrivateKey == "" {
 		req.SetBasicAuth(o.Username, o.Password)
 	}
-
-	// Check to use trustedCACertificate to use InsecureSkipVerify or not
-	if o.TrustedCACertificate != "" {
-		trustedCACert, err := base64.StdEncoding.DecodeString(o.TrustedCACertificate)
-		if err != nil {
-			return nil, errors.New("failed to decode trusted CA certificate from base64")
-		}
-		skipVerify = false
-		caCertPool.AppendCertsFromPEM(trustedCACert)
-	}
-
-	tr := &http.Transport{
-		TLSClientConfig: &tls.Config{
-			InsecureSkipVerify: skipVerify, MinVersion: tridentconfig.MinClientTLSVersion,
-			Certificates: []tls.Certificate{cert}, RootCAs: caCertPool,
-		},
+	client, err := o.clientGetter.getHttpClient()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get HTTP client: %v", err)
 	}
 
-	client := &http.Client{
-		Transport: tr,
-		Timeout:   time.Duration(tridentconfig.StorageAPITimeoutSeconds * time.Second),
-	}
 	response, err := client.Do(req)
 
 	if err != nil {
@@ -276,7 +292,7 @@ func (o *ZapiRunner) ExecuteUsing(z ZAPIRequest, requestType string, v interface
 	if strings.HasSuffix(svm, "-mc") {
 		o.svm = strings.TrimSuffix(svm, "-mc")
 	} else {
-		o.svm += "-mc"
+		o.svm = fmt.Sprintf("%s-mc", svm)
 	}
 
 	return o.executeWithoutIteration(z, requestType, vCopy)

storage_drivers/ontap/api/azgo/common_test.go

@@ -3,10 +3,14 @@ package azgo
 import (
 	"bytes"
 	"encoding/xml"
+	"fmt"
 	"io"
+	"io/ioutil"
 	"net/http"
+	"strings"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -104,3 +108,84 @@ func TestCopyForNontunneledZapiRunner(t *testing.T) {
 	assert.NotSame(t, originalZRunner.m, copiedZRunner.m, "Mutexes should not be the same instance")
 	assert.NotNil(t, copiedZRunner.m, "Copied ZapiRunner should have a non-nil mutex")
 }
+
+func CreateTestZapiRunner(n int) *ZapiRunner {
+	managementLIF := "1.2.3.4"
+	svm := fmt.Sprintf("svm-%d", n)
+	username := fmt.Sprintf("user%d", n)
+	password := fmt.Sprintf("password%d", n)
+	clientPrivateKey := ""
+	clientCertificate := ""
+	clientCACert := ""
+	secure := false
+	ontapApiVersion := "9.8"
+	debugTraceFlags := map[string]bool{}
+	return &ZapiRunner{
+		ManagementLIF:        managementLIF,
+		svm:                  svm,
+		Username:             username,
+		Password:             password,
+		ClientPrivateKey:     clientPrivateKey,
+		ClientCertificate:    clientCertificate,
+		TrustedCACertificate: clientCACert,
+		Secure:               secure,
+		ontapApiVersion:      ontapApiVersion,
+		DebugTraceFlags:      debugTraceFlags,
+		m:                    &sync.RWMutex{},
+	}
+}
+
+type mockHttpClientGetter struct {
+	tr      http.RoundTripper
+	timeout time.Duration
+}
+
+func (m *mockHttpClientGetter) getHttpClient() (*http.Client, error) {
+	client := &http.Client{
+		Transport: m.tr,
+		Timeout:   m.timeout,
+	}
+	return client, nil
+}
+
+type RoundTripFunc func(*http.Request) (*http.Response, error)
+
+func (f RoundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) {
+	return f(r)
+}
+
+var mockZapiErrorVserverNotFound string = `<?xml version="1.0" encoding="UTF-8"?>
+<netapp version="1.21" xmlns="http://www.netapp.com/filer/admin">
+  <results status="failed" errno="15698" reason="Simulated error for testing"/>
+</netapp>`
+
+// TestZapiRunner_ExecuteUsing_Parallel_MC tests the parallel execution of ExecuteUsing where the SVM name changes
+func TestZapiRunner_ExecuteUsing_Parallel_MC(t *testing.T) {
+	// Test parallel calls to ExecuteUsing with MC name changes
+	zRunner := CreateTestZapiRunner(1)
+	mockTransport := RoundTripFunc(func(req *http.Request) (*http.Response, error) {
+		return &http.Response{
+			StatusCode: http.StatusOK,
+			Body:       ioutil.NopCloser(strings.NewReader(mockZapiErrorVserverNotFound)),
+			Header:     make(http.Header),
+		}, nil
+	})
+	mockGetter := &mockHttpClientGetter{
+		tr:      mockTransport,
+		timeout: 5 * time.Second,
+	}
+	zRunner.clientGetter = mockGetter
+
+	var wg sync.WaitGroup
+	for i := 0; i < 50; i++ {
+		wg.Add(1)
+		go func(n int) {
+			defer wg.Done()
+			zRunner.ExecuteUsing(&VolumeCreateRequest{}, "VolumeCreateRequest", NewVolumeCreateResponse())
+		}(i)
+	}
+	wg.Wait() // Wait for all goroutines to finish
+
+	// Check the SVM name, it should be one of the expected values
+	assert.True(t, zRunner.svm == "svm-1-mc" || zRunner.svm == "svm-1")
+}