Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve time complexity of apstra_datacenter_generic_system link updates #597

Merged
merged 2 commits into from
Feb 27, 2024
Merged

Improve time complexity of apstra_datacenter_generic_system link updates #597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
850383d
improve time complexity of update
chrismarget-j Feb 27, 2024
6f68615
remove deprecated function
chrismarget-j Feb 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions apstra/blueprint/datacenter_generic_system.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,8 @@ func (o *DatacenterGenericSystem) deleteLinksFromSystem(ctx context.Context, lin
// it operates only on the links passed as a function argument because only
// those links need to be updated/validated.
func (o *DatacenterGenericSystem) updateLinkParams(ctx context.Context, planLinks, stateLinks []*DatacenterGenericSystemLink, bp *apstra.TwoStageL3ClosClient, diags *diag.Diagnostics) {
// one at a time, check/update each link
// create an empty request with room for each link to be modified
request := make(apstra.SetLinkLagParamsRequest, len(planLinks))
for i, link := range planLinks {
// we don't keep the link ID, but we have each link's target switch and
// interface name. That's enough to uniquely identify the link from the
Expand All @@ -558,7 +559,22 @@ func (o *DatacenterGenericSystem) updateLinkParams(ctx context.Context, planLink
return
}

link.updateParams(ctx, linkId, stateLinks[i], bp, diags) // collect all errors
linkParams := link.lagParams(ctx, linkId, stateLinks[i], bp, diags)
if linkParams != nil {
request[linkId] = *linkParams
}
}

if len(request) != 0 {
err := bp.SetLinkLagParams(ctx, &request)
if err != nil {
diags.AddError("failed updating generic system link parameters", err.Error()) // collect all errors
}
}

// one at a time, check/update each link transform ID
for i, link := range planLinks {
link.updateTransformId(ctx, stateLinks[i], bp, diags) // collect all errors
}
}

Expand Down
60 changes: 26 additions & 34 deletions apstra/blueprint/datacenter_generic_system_link.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,14 +153,32 @@ func (o *DatacenterGenericSystemLink) getTransformId(ctx context.Context, client
o.TargetSwitchIfTransformId = types.Int64Value(int64(transformId))
}

// updateParams checks/updates the following link parameters.
// - transform ID
// - group label
// - LAG mode
// - tags
// Because the DatacenterGenericSystemLink object doesn't know the link ID,
// the ID of the link's graph node is passed as a function argument.
func (o *DatacenterGenericSystemLink) updateParams(ctx context.Context, id apstra.ObjectId, state *DatacenterGenericSystemLink, client *apstra.TwoStageL3ClosClient, diags *diag.Diagnostics) {
func (o *DatacenterGenericSystemLink) lagParams(ctx context.Context, id apstra.ObjectId, state *DatacenterGenericSystemLink, client *apstra.TwoStageL3ClosClient, diags *diag.Diagnostics) *apstra.LinkLagParams {
if o.Tags.Equal(state.Tags) && o.LagMode.Equal(state.LagMode) && o.GroupLabel.Equal(state.GroupLabel) {
return nil // nothing to do
}

var tags []string
diags.Append(o.Tags.ElementsAs(ctx, &tags, false)...)
if tags == nil {
tags = []string{} // convert nil -> empty slice to clear tags
}

var lagMode apstra.RackLinkLagMode
err := lagMode.FromString(o.LagMode.ValueString())
if err != nil {
diags.AddError(fmt.Sprintf("failed to parse lag mode %s", o.LagMode), err.Error())
return nil
}

return &apstra.LinkLagParams{
GroupLabel: o.GroupLabel.ValueString(),
LagMode: lagMode,
Tags: tags,
}
}

func (o *DatacenterGenericSystemLink) updateTransformId(ctx context.Context, state *DatacenterGenericSystemLink, client *apstra.TwoStageL3ClosClient, diags *diag.Diagnostics) {
// set the transform ID if it has changed
if !o.TargetSwitchIfTransformId.Equal(state.TargetSwitchIfTransformId) {
err := client.SetTransformIdByIfName(ctx, apstra.ObjectId(o.TargetSwitchId.ValueString()),
Expand All @@ -175,32 +193,6 @@ func (o *DatacenterGenericSystemLink) updateParams(ctx context.Context, id apstr
}
}
}

// set the tags, lag mode and group label if any have changed
if !o.Tags.Equal(state.Tags) || !o.LagMode.Equal(state.LagMode) || !o.GroupLabel.Equal(state.GroupLabel) {
var tags []string
diags.Append(o.Tags.ElementsAs(ctx, &tags, false)...)
if tags == nil {
tags = []string{} // convert nil -> empty slice to clear tags
}

var lagMode apstra.RackLinkLagMode
err := lagMode.FromString(o.LagMode.ValueString())
if err != nil {
diags.AddError(fmt.Sprintf("failed to parse lag mode %s", o.LagMode), err.Error())
return
}

// set lag params + tag set
err = client.SetLinkLagParams(ctx, &apstra.SetLinkLagParamsRequest{id: apstra.LinkLagParams{
GroupLabel: o.GroupLabel.ValueString(),
LagMode: lagMode,
Tags: tags,
}})
if err != nil {
diags.AddError(fmt.Sprintf("failed to set link %s LAG parameters", id), err.Error())
}
}
}

func (o *DatacenterGenericSystemLink) versionConstraintsAsGenericSystemLink(_ context.Context, path path.Path, _ *diag.Diagnostics) apiversions.Constraints {
Expand Down
12 changes: 11 additions & 1 deletion apstra/resource_datacenter_generic_system_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import (
"context"
"fmt"
"github.com/Juniper/apstra-go-sdk/apstra"
apiversions "github.com/Juniper/terraform-provider-apstra/apstra/api_versions"
testutils "github.com/Juniper/terraform-provider-apstra/apstra/test_utils"
"github.com/Juniper/terraform-provider-apstra/apstra/utils"
"github.com/hashicorp/go-version"
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"net"
Expand Down Expand Up @@ -273,11 +275,13 @@ func TestResourceDatacenterGenericSystem_A(t *testing.T) {
}

type testCase struct {
steps []testStep
steps []testStep
versionConstraints version.Constraints
}

testCases := map[string]testCase{
"lots_of_changes": {
versionConstraints: version.MustConstraints(version.NewConstraint(">" + apiversions.Apstra411)), // tags allowed in 4.1.1 only
steps: []testStep{
{
genericSystem: genericSystem{
Expand Down Expand Up @@ -659,6 +663,11 @@ func TestResourceDatacenterGenericSystem_A(t *testing.T) {
tName, tCase := tName, tCase
t.Run(tName, func(t *testing.T) {
t.Parallel()

if !tCase.versionConstraints.Check(version.Must(version.NewVersion(bpClient.Client().ApiVersion()))) {
t.Skipf("test case %s requires Apstra %s", tName, tCase.versionConstraints.String())
}

steps := make([]resource.TestStep, len(tCase.steps))
for i, step := range tCase.steps {
step.genericSystem.bpId = bpClient.Id().String()
Expand All @@ -668,6 +677,7 @@ func TestResourceDatacenterGenericSystem_A(t *testing.T) {
PreConfig: step.preConfig,
}
}

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
Steps: steps,
Expand Down
Loading