Skip to content

Commit

Permalink
Tune connection performance (#106)
Browse files Browse the repository at this point in the history
Add configuration to http.Transport

Move OneFS version creation to method `client.GetOnefsVersion()`
that can be invoked as needed, also it will not delay client
initialization.

The following bug is fixed in the PR:
Fix PowerScale token cannot be refreshed after expiration.
  • Loading branch information
baoy1 authored Dec 7, 2023
1 parent e20fc58 commit 2d7a235
Show file tree
Hide file tree
Showing 11 changed files with 231 additions and 73 deletions.
80 changes: 53 additions & 27 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,50 @@ type AuthContextKey string
// Client type is to hold powerscale client.
type Client struct {
PscaleOpenAPIClient *powerscale.APIClient
OnefsVersion OnefsVersion
onefsVersion *OnefsVersion
mu sync.Mutex
}

// GetOnefsVersion get OneFS version.
func (c *Client) GetOnefsVersion() (*OnefsVersion, error) {
c.mu.Lock()
defer c.mu.Unlock()
if c.onefsVersion == nil {
c.onefsVersion = &OnefsVersion{}
config, _, err := c.PscaleOpenAPIClient.ClusterApi.GetClusterv3ClusterConfig(context.Background()).Execute()
if err != nil {
return nil, err
}

parts := strings.Split(config.OnefsVersion.Release, ".")
if len(parts) > 2 {
major, err := strconv.Atoi(parts[0])
if err != nil {
return nil, err
}
minor, err := strconv.Atoi(parts[1])
if err != nil {
return nil, err
}
patch, err := strconv.Atoi(parts[2])
if err != nil {
return nil, err
}
c.onefsVersion = &OnefsVersion{major, minor, patch}
} else {
return nil, errors.New("Unable to parse OneFS version " + config.OnefsVersion.Release)
}
}
return c.onefsVersion, nil
}

// SetOnefsVersion sets the OneFS version of the client.
// Note: This function is not supposed to be called.
// It is only used for testing.
func (c *Client) SetOnefsVersion(major, minor, patch int) {
c.mu.Lock()
defer c.mu.Unlock()
c.onefsVersion = &OnefsVersion{major, minor, patch}
}

// OnefsVersion present OneFS release version.
Expand Down Expand Up @@ -142,31 +185,6 @@ func NewClient(endpoint string,
PscaleOpenAPIClient: openAPIClient,
}

// Inject OneFS version
config, _, err := openAPIClient.ClusterApi.GetClusterv3ClusterConfig(context.Background()).Execute()
if err != nil {
return nil, err
}

parts := strings.Split(config.OnefsVersion.Release, ".")
if len(parts) > 2 {
major, err := strconv.Atoi(parts[0])
if err != nil {
return nil, err
}
minor, err := strconv.Atoi(parts[1])
if err != nil {
return nil, err
}
patch, err := strconv.Atoi(parts[2])
if err != nil {
return nil, err
}
client.OnefsVersion = OnefsVersion{major, minor, patch}
} else {
return nil, errors.New("Unable to parse OneFS version " + config.OnefsVersion.Release)
}

return &client, nil
}

Expand Down Expand Up @@ -194,6 +212,10 @@ func NewOpenAPIClient(ctx context.Context, endpoint string, insecure bool, user
/* #nosec */
InsecureSkipVerify: true,
},
MaxIdleConns: 100,
MaxIdleConnsPerHost: 30,
MaxConnsPerHost: 10,
IdleConnTimeout: 90 * time.Second,
}
} else {
// Loading system certs by default if insecure is set to false
Expand All @@ -209,6 +231,10 @@ func NewOpenAPIClient(ctx context.Context, endpoint string, insecure bool, user
RootCAs: pool,
InsecureSkipVerify: false,
},
MaxIdleConns: 100,
MaxIdleConnsPerHost: 30,
MaxConnsPerHost: 10,
IdleConnTimeout: 90 * time.Second,
}
}

Expand All @@ -232,6 +258,7 @@ func NewOpenAPIClient(ctx context.Context, endpoint string, insecure bool, user
httpclient.Transport = transport
basicAuth(user, pass, &cfg)
} else if authType == SessionAuthType {
ctx = context.WithValue(ctx, AuthContextKey(AuthType), SessionAuthType)
httpclient.Transport = &TokenTransport{Ctx: ctx, Username: user, Password: pass, RoundTripper: transport}
err := sessionAuth(ctx, user, pass, &cfg)
if err != nil {
Expand Down Expand Up @@ -267,7 +294,6 @@ func getHeaders() map[string]string {
func sessionAuth(ctx context.Context, user string, pass string, cfg *powerscale.Configuration) error {
mutex.Lock()
defer mutex.Unlock()
ctx = context.WithValue(ctx, AuthContextKey(AuthType), SessionAuthType)
host, err := cfg.ServerURLWithContext(ctx, "")
if err != nil {
return err
Expand Down
17 changes: 0 additions & 17 deletions powerscale/helper/access_zone_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"context"
powerscale "dell/powerscale-go-client"
"fmt"
"strconv"
"strings"
"terraform-provider-powerscale/client"
"terraform-provider-powerscale/powerscale/constants"
Expand Down Expand Up @@ -163,19 +162,3 @@ func QueryZoneNameByID(ctx context.Context, client *client.Client, zoneID int32)

return "", fmt.Errorf("error finding zone name for zone ID %d", zoneID)
}

// QueryZoneIDByName returns a specific zone id by name.
func QueryZoneIDByName(ctx context.Context, client *client.Client, zoneName string) (string, error) {
zones, err := GetAllAccessZones(ctx, client)
if err != nil {
return "", err
}
for _, zone := range zones.Zones {
if *zone.Name == zoneName {

return strconv.Itoa(int(*zone.ZoneId)), nil
}
}

return "", fmt.Errorf("error finding zone ID for zone name %s", zoneName)
}
4 changes: 2 additions & 2 deletions powerscale/helper/file_system_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ const acl = "acl"
const mode = "mode"

// UpdateFileSystem Updates the file system attributes.
func UpdateFileSystem(ctx context.Context, client client.Client, dirPath string, plan *models.FileSystemResource, state *models.FileSystemResource) error {
func UpdateFileSystem(ctx context.Context, client *client.Client, dirPath string, plan *models.FileSystemResource, state *models.FileSystemResource) error {

// Update Owner / Group if modified
if plan.Owner.Name.ValueString() != state.Owner.Name.ValueString() || plan.Group.Name.ValueString() != state.Group.Name.ValueString() ||
Expand Down Expand Up @@ -416,7 +416,7 @@ func getNewAccessControlParams(accessControl string) (string, string) {
}

// ValidateUserAndGroup check if owner/group information is correct.
func ValidateUserAndGroup(ctx context.Context, client client.Client, owner models.MemberObject, group models.MemberObject, accessZone string) error {
func ValidateUserAndGroup(ctx context.Context, client *client.Client, owner models.MemberObject, group models.MemberObject, accessZone string) error {
// Validate owner information
userReq := client.PscaleOpenAPIClient.AuthApi.GetAuthv1AuthUser(ctx, owner.Name.ValueString())

Expand Down
16 changes: 13 additions & 3 deletions powerscale/helper/smart_pool_settting_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,12 @@ type SettingsSetter interface {

// GetSmartPoolSettings Get SmartPool settings based on Onefs version.
func GetSmartPoolSettings(ctx context.Context, powerscaleClient *client.Client) (any, error) {
if powerscaleClient.OnefsVersion.IsGreaterThan("9.4.0") {
onfsVersion, err := powerscaleClient.GetOnefsVersion()
if err != nil {
return nil, fmt.Errorf("failed to get OneFS version: %v", err)
}

if onfsVersion.IsGreaterThan("9.4.0") {
settings, _, err := powerscaleClient.PscaleOpenAPIClient.StoragepoolApi.GetStoragepoolv16StoragepoolSettings(ctx).Execute()
return settings, err
}
Expand Down Expand Up @@ -255,7 +260,12 @@ func BigFloatToInt32(x *big.Float) (*int32, error) {

// UpdateSmartPoolSettings apply SmartPool Settings changes on PowerScale.
func UpdateSmartPoolSettings(ctx context.Context, client *client.Client, model *models.SmartPoolSettingsResource) error {
if client.OnefsVersion.IsGreaterThan("9.4.0") {
onfsVersion, err := client.GetOnefsVersion()
if err != nil {
return fmt.Errorf("failed to get OneFS version: %v", err)
}

if onfsVersion.IsGreaterThan("9.4.0") {
updateParam := client.PscaleOpenAPIClient.StoragepoolApi.UpdateStoragepoolv16StoragepoolSettings(ctx)
settings := powerscale.V16StoragepoolSettingsExtended{}

Expand Down Expand Up @@ -289,7 +299,7 @@ func UpdateSmartPoolSettings(ctx context.Context, client *client.Client, model *
updateParam := client.PscaleOpenAPIClient.StoragepoolApi.UpdateStoragepoolv5StoragepoolSettings(ctx)
settings := powerscale.V5StoragepoolSettingsExtended{}

err := ReadFromState(ctx, model, &settings)
err = ReadFromState(ctx, model, &settings)
if err != nil {
return err
}
Expand Down
31 changes: 31 additions & 0 deletions powerscale/provider/access_zone_datasource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ limitations under the License.
package provider

import (
"context"
powerscale "dell/powerscale-go-client"
"fmt"
"github.com/hashicorp/terraform-plugin-testing/terraform"
"github.com/stretchr/testify/assert"
"regexp"
"terraform-provider-powerscale/powerscale/helper"
"testing"
Expand Down Expand Up @@ -106,8 +110,35 @@ func TestAccAccessZoneDataSourceGetErr(t *testing.T) {
ExpectError: regexp.MustCompile(`.*mock error*.`),
},
},
CheckDestroy: func(_ *terraform.State) error {
if FunctionMocker != nil {
FunctionMocker.UnPatch()
}
return nil
},
})
}
func TestQueryZoneNameByID(t *testing.T) {
var azs []powerscale.V3ZoneExtended
for i := 0; i < 10; i++ {
idInt := int32(i)
zoneName := fmt.Sprintf("Zone%d", i)
azs = append(azs, powerscale.V3ZoneExtended{
ZoneId: &idInt,
Name: &zoneName,
})
}

zones := &powerscale.V3Zones{Zones: azs}
if FunctionMocker != nil {
FunctionMocker.UnPatch()
}
FunctionMocker = mockey.Mock(helper.GetAllAccessZones).Return(zones, nil).Build()
defer FunctionMocker.UnPatch()
name, err := helper.QueryZoneNameByID(context.Background(), nil, 0)
assert.Nil(t, err)
assert.Equal(t, "Zone0", name)
}

var AzDataSourceConfig = `
data "powerscale_accesszone" "test" {
Expand Down
34 changes: 34 additions & 0 deletions powerscale/provider/access_zone_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package provider

import (
"fmt"
"github.com/hashicorp/terraform-plugin-testing/terraform"
"regexp"
"terraform-provider-powerscale/powerscale/helper"
"testing"
Expand Down Expand Up @@ -107,12 +108,21 @@ func TestAccAccessZoneResourceAfterCreateGetErr(t *testing.T) {
},
{
PreConfig: func() {
if mocker != nil {
mocker.UnPatch()
}
mocker = mockey.Mock(helper.GetAllAccessZones).Return(nil, fmt.Errorf("access zone read mock error")).Build()
},
Config: ProviderConfig + AccessZoneResourceConfig,
ExpectError: regexp.MustCompile(`.*access zone read mock error*.`),
},
},
CheckDestroy: func(_ *terraform.State) error {
if mocker != nil {
mocker.UnPatch()
}
return nil
},
})
}

Expand All @@ -127,13 +137,25 @@ func TestAccAccessZoneResourceGetErr(t *testing.T) {
if mocker != nil {
mocker.UnPatch()
}
if createMocker != nil {
createMocker.UnPatch()
}
createMocker = mockey.Mock(helper.CreateAccessZones).Return(nil).Build()
mocker = mockey.Mock(helper.GetAllAccessZones).Return(nil, fmt.Errorf("access zone read mock error")).Build()
},
Config: ProviderConfig + AccessZoneResourceConfig,
ExpectError: regexp.MustCompile(`.*access zone read mock error*.`),
},
},
CheckDestroy: func(_ *terraform.State) error {
if mocker != nil {
mocker.UnPatch()
}
if createMocker != nil {
createMocker.UnPatch()
}
return nil
},
})
}

Expand Down Expand Up @@ -163,6 +185,9 @@ func TestAccAccessZoneResourceGetImportErr(t *testing.T) {
},
{
PreConfig: func() {
if mocker != nil {
mocker.UnPatch()
}
mocker = mockey.Mock(helper.GetAllAccessZones).Return(nil, fmt.Errorf("access zone read mock error")).Build()
},
ResourceName: accessZoneResourceName,
Expand All @@ -171,6 +196,15 @@ func TestAccAccessZoneResourceGetImportErr(t *testing.T) {
ExpectError: regexp.MustCompile(`.*access zone read mock error*.`),
},
},
CheckDestroy: func(_ *terraform.State) error {
if createMocker != nil {
createMocker.UnPatch()
}
if mocker != nil {
mocker.UnPatch()
}
return nil
},
})
}

Expand Down
10 changes: 5 additions & 5 deletions powerscale/provider/file_system_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func (r *FileSystemResource) Create(ctx context.Context, req resource.CreateRequ
if !plan.AccessControl.IsNull() && (plan.AccessControl.ValueString() != "") {
createReq = createReq.XIsiIfsAccessControl(plan.AccessControl.ValueString())
}
errAuth := helper.ValidateUserAndGroup(ctx, *r.client, plan.Owner, plan.Group, plan.QueryZone.ValueString())
errAuth := helper.ValidateUserAndGroup(ctx, r.client, plan.Owner, plan.Group, plan.QueryZone.ValueString())
if errAuth != nil {
errStr := constants.CreateFileSystemErrorMsg
message := helper.GetErrorString(errAuth, errStr)
Expand Down Expand Up @@ -384,7 +384,7 @@ func (r *FileSystemResource) Read(ctx context.Context, req resource.ReadRequest,
return
}

//copy to model
// copy to model
var state models.FileSystemResource
helper.UpdateFileSystemResourceState(ctx, &plan, &state, acl, meta)
helper.UpdateFileSystemResourcePlanData(&plan, &state)
Expand Down Expand Up @@ -442,7 +442,7 @@ func (r *FileSystemResource) Update(ctx context.Context, req resource.UpdateRequ
return
}

if err := helper.UpdateFileSystem(ctx, *r.client, planDirName, &plan, &state); err != nil {
if err := helper.UpdateFileSystem(ctx, r.client, planDirName, &plan, &state); err != nil {
resp.Diagnostics.AddError(
fmt.Sprintf("Error updating the File system Resource - %s", planDirName),
err.Error(),
Expand Down Expand Up @@ -474,7 +474,7 @@ func (r *FileSystemResource) Update(ctx context.Context, req resource.UpdateRequ
return
}

//copy to model
// copy to model
helper.UpdateFileSystemResourceState(ctx, &plan, &state, acl, meta)
helper.UpdateFileSystemResourcePlanData(&plan, &state)

Expand Down Expand Up @@ -512,7 +512,7 @@ func (r *FileSystemResource) ImportState(ctx context.Context, req resource.Impor
return
}

//copy to model
// copy to model
helper.UpdateFileSystemResourceState(ctx, nil, &state, acl, meta)
state.ID = types.StringValue(id)
dir, name := filepath.Split(id)
Expand Down
3 changes: 1 addition & 2 deletions powerscale/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type PscaleProvider struct {
// client can contain the upstream provider SDK or HTTP client used to
// communicate with the upstream service. Resource and DataSource
// implementations can then make calls using this client.
client *client.Client

// version is set to the provider version on release, "dev" when the
// provider is built and ran locally, and "test" when running acceptance
// testing.
Expand Down Expand Up @@ -153,7 +153,6 @@ func (p *PscaleProvider) Configure(ctx context.Context, req provider.ConfigureRe
}

// client configuration for data sources and resources
p.client = pscaleClient
resp.DataSourceData = pscaleClient
resp.ResourceData = pscaleClient
}
Expand Down
Loading

0 comments on commit 2d7a235

Please sign in to comment.