Skip to content

Commit

Permalink
fix: review PR
Browse files Browse the repository at this point in the history
  • Loading branch information
gaetanars committed Mar 31, 2023
1 parent ff0991c commit fed743d
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 114 deletions.
2 changes: 1 addition & 1 deletion docs/resources/alb_pool.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ resource "cloudavenue_alb_pool" "example" {

Required:

- `ip_address` (String) IP address of pool member.
- `ip_address` (String) IP address of pool member. Must be a valid IP with net.ParseIP.
- `port` (Number) Member port. Value must be at least 1.

Optional:
Expand Down
48 changes: 20 additions & 28 deletions internal/provider/alb/pool_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
govcdtypes "github.com/vmware/go-vcloud-director/v2/types/v56"

"github.com/hashicorp/terraform-plugin-framework/types"

"github.com/orange-cloudavenue/terraform-provider-cloudavenue/pkg/utils"
)

var ErrPersistenceProfileIsEmpty = errors.New("persistence profile is empty")
Expand All @@ -17,43 +19,33 @@ type albPool interface {
GetAlbPool() (*govcd.NsxtAlbPool, error)
}

func processMembers(poolMembers []govcdtypes.NsxtAlbPoolMember) []member {
members := []member{}
if len(poolMembers) > 0 {
for _, poolMember := range poolMembers {
members = append(members, member{
Enabled: types.BoolValue(poolMember.Enabled),
IPAddress: types.StringValue(poolMember.IpAddress),
Port: types.Int64Value(int64(poolMember.Port)),
Ratio: types.Int64Value(int64(*poolMember.Ratio)),
})
}
func processMembers(poolMembers []govcdtypes.NsxtAlbPoolMember) (members []member) {
for _, poolMember := range poolMembers {
members = append(members, member{
Enabled: types.BoolValue(poolMember.Enabled),
IPAddress: types.StringValue(poolMember.IpAddress),
Port: types.Int64Value(int64(poolMember.Port)),
Ratio: types.Int64Value(int64(*poolMember.Ratio)),
})
}

return members
return
}

func processHealthMonitors(poolHealthMonitors []govcdtypes.NsxtAlbPoolHealthMonitor) []string {
var healtMonitors []string
if len(poolHealthMonitors) > 0 {
for _, poolHealthMonitor := range poolHealthMonitors {
healtMonitors = append(healtMonitors, poolHealthMonitor.Type)
}
func processHealthMonitors(poolHealthMonitors []govcdtypes.NsxtAlbPoolHealthMonitor) (healthMonitors []string) {
for _, poolHealthMonitor := range poolHealthMonitors {
healthMonitors = append(healthMonitors, poolHealthMonitor.Type)
}

return healtMonitors
return
}

func processPersistenceProfile(poolPersistenceProfile *govcdtypes.NsxtAlbPoolPersistenceProfile) persistenceProfile {
p := persistenceProfile{}
if poolPersistenceProfile != nil {
p.Type = types.StringValue(poolPersistenceProfile.Type)
if poolPersistenceProfile.Value == "" {
p.Value = types.StringNull()
} else {
p.Value = types.StringValue(poolPersistenceProfile.Value)
}
return persistenceProfile{}
}

return p
return persistenceProfile{
Type: types.StringValue(poolPersistenceProfile.Type),
Value: utils.StringValueOrNull(poolPersistenceProfile.Value),
}
}
68 changes: 36 additions & 32 deletions internal/provider/alb/pool_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,18 +108,23 @@ func (r *albPoolResource) Create(ctx context.Context, req resource.CreateRequest
}

// Prepare config.
albPoolConfig, err := r.getAlbPoolConfig(plan)
albPoolConfig, err := r.getAlbPoolConfig(ctx, plan)
if err != nil {
resp.Diagnostics.AddError("Unable to prepare ALB Pool Config", err.Error())
return
}

// Lock EdgeGW
resp.Diagnostics.Append(r.org.LockParentEdgeGW(ctx, r.edgegw)...)
edgeGW, err := r.org.GetEdgeGateway(r.edgegw)
if err != nil {
resp.Diagnostics.AddError("Unable to get Edge Gateway", err.Error())
return
}
resp.Diagnostics.Append(edgeGW.Lock(ctx)...)
if resp.Diagnostics.HasError() {
return
}
defer resp.Diagnostics.Append(r.org.UnlockParentEdgeGW(ctx, r.edgegw)...)
defer resp.Diagnostics.Append(edgeGW.Unlock(ctx)...)

// Create ALB Pool
createdAlbPool, err := r.client.Vmware.CreateNsxtAlbPool(albPoolConfig)
Expand All @@ -133,9 +138,6 @@ func (r *albPoolResource) Create(ctx context.Context, req resource.CreateRequest

// Set state to fully populated data
resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...)
if resp.Diagnostics.HasError() {
return
}
}

// Read refreshes the Terraform state with the latest data.
Expand Down Expand Up @@ -186,9 +188,7 @@ func (r *albPoolResource) Read(ctx context.Context, req resource.ReadRequest, re
}

// Set members
members := processMembers(albPool.NsxtAlbPool.Members)

if len(members) > 0 {
if members := processMembers(albPool.NsxtAlbPool.Members); len(members) > 0 {
plan.Members, diags = types.SetValueFrom(ctx, types.ObjectType{AttrTypes: memberAttrTypes}, members)
resp.Diagnostics.Append(diags...)
if resp.Diagnostics.HasError() {
Expand Down Expand Up @@ -220,9 +220,6 @@ func (r *albPoolResource) Read(ctx context.Context, req resource.ReadRequest, re

// Set refreshed state
resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...)
if resp.Diagnostics.HasError() {
return
}
}

// Update updates the resource and sets the updated Terraform state on success.
Expand All @@ -249,18 +246,23 @@ func (r *albPoolResource) Update(ctx context.Context, req resource.UpdateRequest
}

// Prepare config.
albPoolConfig, err := r.getAlbPoolConfig(plan)
albPoolConfig, err := r.getAlbPoolConfig(ctx, plan)
if err != nil {
resp.Diagnostics.AddError("Unable to prepare ALB Pool Config", err.Error())
return
}

// Lock EdgeGW
resp.Diagnostics.Append(r.org.LockParentEdgeGW(ctx, r.edgegw)...)
edgeGW, err := r.org.GetEdgeGateway(r.edgegw)
if err != nil {
resp.Diagnostics.AddError("Unable to get Edge Gateway", err.Error())
return
}
resp.Diagnostics.Append(edgeGW.Lock(ctx)...)
if resp.Diagnostics.HasError() {
return
}
defer resp.Diagnostics.Append(r.org.UnlockParentEdgeGW(ctx, r.edgegw)...)
defer resp.Diagnostics.Append(edgeGW.Unlock(ctx)...)

// Update ALB Pool.
_, err = albPool.Update(albPoolConfig)
Expand All @@ -271,9 +273,6 @@ func (r *albPoolResource) Update(ctx context.Context, req resource.UpdateRequest

// Set state to fully populated data
resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...)
if resp.Diagnostics.HasError() {
return
}
}

// Delete deletes the resource and removes the Terraform state on success.
Expand All @@ -293,11 +292,16 @@ func (r *albPoolResource) Delete(ctx context.Context, req resource.DeleteRequest
}

// Lock EdgeGW
resp.Diagnostics.Append(r.org.LockParentEdgeGW(ctx, r.edgegw)...)
edgeGW, err := r.org.GetEdgeGateway(r.edgegw)
if err != nil {
resp.Diagnostics.AddError("Unable to get Edge Gateway", err.Error())
return
}
resp.Diagnostics.Append(edgeGW.Lock(ctx)...)
if resp.Diagnostics.HasError() {
return
}
defer resp.Diagnostics.Append(r.org.UnlockParentEdgeGW(ctx, r.edgegw)...)
defer resp.Diagnostics.Append(edgeGW.Unlock(ctx)...)

// Get albPool
albPool, err := r.GetAlbPool()
Expand Down Expand Up @@ -366,8 +370,8 @@ func (r *albPoolResource) GetAlbPool() (*govcd.NsxtAlbPool, error) {

// getAlbPoolConfig is the main function for getting *govcdtypes.NsxtAlbPool for API request. It nests multiple smaller
// functions for smaller types.
func (r *albPoolResource) getAlbPoolConfig(d *albPoolModel) (*govcdtypes.NsxtAlbPool, error) {
edgeID, err := r.org.GetEdgeGatewayID(r.edgegw)
func (r *albPoolResource) getAlbPoolConfig(ctx context.Context, d *albPoolModel) (*govcdtypes.NsxtAlbPool, error) {
edge, err := r.org.GetEdgeGateway(r.edgegw)
if err != nil {
return nil, err
}
Expand All @@ -378,27 +382,27 @@ func (r *albPoolResource) getAlbPoolConfig(d *albPoolModel) (*govcdtypes.NsxtAlb
Description: d.Description.ValueString(),
Enabled: d.Enabled.ValueBoolPointer(),
GatewayRef: govcdtypes.OpenApiReference{
ID: edgeID,
ID: edge.GetID(),
},
Algorithm: d.Algorithm.ValueString(),
DefaultPort: utils.TakeIntPointer(int(d.DefaultPort.ValueInt64())),
GracefulTimeoutPeriod: utils.TakeIntPointer(int(d.GracefulTimeoutPeriod.ValueInt64())),
PassiveMonitoringEnabled: d.PassiveMonitoringEnabled.ValueBoolPointer(),
}

poolMembers, err := r.getAlbPoolMembersType(d)
poolMembers, err := r.getAlbPoolMembersType(ctx, d)
if err != nil {
return nil, fmt.Errorf("error defining pool members: %w", err)
}
albPoolConfig.Members = poolMembers

persistenceProfile, err := r.getAlbPoolPersistenceProfileType(d)
persistenceProfile, err := r.getAlbPoolPersistenceProfileType(ctx, d)
if err != nil && !errors.Is(err, ErrPersistenceProfileIsEmpty) {
return nil, fmt.Errorf("error defining persistence profile: %w", err)
}
albPoolConfig.PersistenceProfile = persistenceProfile

healthMonitors, err := r.getAlbPoolHealthMonitorType(d)
healthMonitors, err := r.getAlbPoolHealthMonitorType(ctx, d)
if err != nil {
return nil, fmt.Errorf("error defining health monitors: %w", err)
}
Expand All @@ -408,9 +412,9 @@ func (r *albPoolResource) getAlbPoolConfig(d *albPoolModel) (*govcdtypes.NsxtAlb
}

// getAlbPoolMembersType.
func (r *albPoolResource) getAlbPoolMembersType(d *albPoolModel) ([]govcdtypes.NsxtAlbPoolMember, error) {
func (r *albPoolResource) getAlbPoolMembersType(ctx context.Context, d *albPoolModel) ([]govcdtypes.NsxtAlbPoolMember, error) {
var members []member
diags := d.Members.ElementsAs(context.Background(), &members, true)
diags := d.Members.ElementsAs(ctx, &members, true)
if diags.HasError() {
return nil, errors.New(diags[0].Detail())
}
Expand All @@ -427,13 +431,13 @@ func (r *albPoolResource) getAlbPoolMembersType(d *albPoolModel) ([]govcdtypes.N
}

// getAlbPoolPersistenceProfileType.
func (r *albPoolResource) getAlbPoolPersistenceProfileType(d *albPoolModel) (*govcdtypes.NsxtAlbPoolPersistenceProfile, error) {
func (r *albPoolResource) getAlbPoolPersistenceProfileType(ctx context.Context, d *albPoolModel) (*govcdtypes.NsxtAlbPoolPersistenceProfile, error) {
if d.PersistenceProfile.IsNull() {
return nil, ErrPersistenceProfileIsEmpty
}

p := &persistenceProfile{}
diags := d.PersistenceProfile.As(context.Background(), p, basetypes.ObjectAsOptions{
diags := d.PersistenceProfile.As(ctx, p, basetypes.ObjectAsOptions{
UnhandledNullAsEmpty: false,
UnhandledUnknownAsEmpty: false,
})
Expand All @@ -449,9 +453,9 @@ func (r *albPoolResource) getAlbPoolPersistenceProfileType(d *albPoolModel) (*go
}

// getAlbPoolHealthMonitorType.
func (r *albPoolResource) getAlbPoolHealthMonitorType(d *albPoolModel) ([]govcdtypes.NsxtAlbPoolHealthMonitor, error) {
func (r *albPoolResource) getAlbPoolHealthMonitorType(ctx context.Context, d *albPoolModel) ([]govcdtypes.NsxtAlbPoolHealthMonitor, error) {
var healthMonitors []string
diags := d.HealthMonitors.ElementsAs(context.Background(), &healthMonitors, true)
diags := d.HealthMonitors.ElementsAs(ctx, &healthMonitors, true)
if diags.HasError() {
return nil, errors.New(diags[0].Detail())
}
Expand Down
11 changes: 7 additions & 4 deletions internal/provider/alb/pool_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,13 @@ func albPoolSchema() superschema.Schema {
},
},
Resource: &schemaR.StringAttribute{
MarkdownDescription: " should be created.",
MarkdownDescription: "should be created.",
PlanModifiers: []planmodifier.String{
stringplanmodifier.RequiresReplace(),
},
},
DataSource: &schemaD.StringAttribute{
MarkdownDescription: " was created.",
MarkdownDescription: "was created.",
},
},
"edge_gateway_name": superschema.StringAttribute{
Expand All @@ -125,13 +125,13 @@ func albPoolSchema() superschema.Schema {
},
},
Resource: &schemaR.StringAttribute{
MarkdownDescription: " should be created.",
MarkdownDescription: "should be created.",
PlanModifiers: []planmodifier.String{
stringplanmodifier.RequiresReplace(),
},
},
DataSource: &schemaD.StringAttribute{
MarkdownDescription: " was created.",
MarkdownDescription: "was created.",
},
},
"enabled": superschema.BoolAttribute{
Expand Down Expand Up @@ -215,6 +215,9 @@ func albPoolSchema() superschema.Schema {
},
Resource: &schemaR.StringAttribute{
Required: true,
Validators: []validator.String{
fstringvalidator.IsIP(),
},
},
DataSource: &schemaD.StringAttribute{
Computed: true,
Expand Down
17 changes: 17 additions & 0 deletions internal/provider/common/edgegw/edgegw.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
package edgegw

import (
"context"
"fmt"

"github.com/vmware/go-vcloud-director/v2/govcd"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/types"

"github.com/orange-cloudavenue/terraform-provider-cloudavenue/internal/client"
"github.com/orange-cloudavenue/terraform-provider-cloudavenue/internal/provider/common/mutex"
)

var ErrEdgeGatewayIDOrNameIsEmpty = fmt.Errorf("edge gateway ID or name is empty")

var gwMutexKV = mutex.NewKV()

type Handler interface {
// GetEdgeGateway allows retrieving NSX-T edge gateway by ID Or Name.
GetEdgeGateway(egw BaseEdgeGW) (EdgeGateway, error)
Expand Down Expand Up @@ -60,3 +65,15 @@ func (e EdgeGateway) GetName() string {
func (e EdgeGateway) GetID() string {
return e.EdgeGateway.ID
}

// Lock locks the Edge Gateway.
func (e EdgeGateway) Lock(ctx context.Context) (d diag.Diagnostics) {
gwMutexKV.KvLock(ctx, e.GetID())
return
}

// Unlock unlocks the Edge Gateway.
func (e EdgeGateway) Unlock(ctx context.Context) (d diag.Diagnostics) {
gwMutexKV.KvUnlock(ctx, e.GetID())
return
}
Loading

0 comments on commit fed743d

Please sign in to comment.