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.

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

fix: [BREAKING] Support ID field for nodes in NodeBalancerConfigRebuildOptions #469

Merged

Conversation

lgarber-akamai
Copy link
Contributor

📝 Description

This change resolves an issue that prevented existing nodes from being preserved using the id field in the NodeBalancer Config Rebuild response body. This works by creating a new NodeBalancerConfigRebuildNodeOptions struct that extends the NodeBalancerNodeCreateOptions struct with a new ID field.

NOTE: This IS a breaking change but we deemed it low-impact enough to be worth making.

✔️ How to Test

The following test steps assume you have pulled down this PR locally.

Integration Testing

make ARGS="-run TestNodeBalancer_Rebuild" fixtures

Manual Testing

  1. In a linodego sandbox environment (e.g. dx-devenv), run the following:
package main

import (
	"context"
	"fmt"
	"github.com/linode/linodego"
	"github.com/sanity-io/litter"
	"log"
	"os"
)

func main() {
	ctx := context.Background()

	client := linodego.NewClient(nil)
	client.SetToken(os.Getenv("LINODE_TOKEN"))

	inst, err := client.CreateInstance(
		ctx,
		linodego.InstanceCreateOptions{
			Label:     "test-instance",
			Region:    "us-mia",
			Type:      "g6-nanode-1",
			PrivateIP: true,
		},
	)
	if err != nil {
		log.Fatal(err)
	}

	nbLabel := "test-nb"

	nb, err := client.CreateNodeBalancer(
		ctx,
		linodego.NodeBalancerCreateOptions{
			Label:  &nbLabel,
			Region: "us-mia",
			Configs: []*linodego.NodeBalancerConfigCreateOptions{
				{
					Port: 80,
					Nodes: []linodego.NodeBalancerNodeCreateOptions{
						{
							Address: inst.IPv4[1].String() + ":80",
							Label:   "test-node",
						},
					},
				},
			},
		},
	)
	if err != nil {
		log.Fatal(err)
	}

	configs, err := client.ListNodeBalancerConfigs(ctx, nb.ID, nil)
	if err != nil {
		log.Fatal(err)
	}
	config := configs[0]

	oldNodes, err := client.ListNodeBalancerNodes(ctx, nb.ID, config.ID, nil)
	if err != nil {
		log.Fatal(err)
	}

	oldNode := oldNodes[0]
	fmt.Println("Old node:")
	litter.Dump(oldNode)
	fmt.Println()

	_, err = client.RebuildNodeBalancerConfig(
		ctx,
		nb.ID,
		config.ID,
		linodego.NodeBalancerConfigRebuildOptions{
			Port: 81,
			Nodes: []linodego.NodeBalancerConfigRebuildNodeOptions{
				{
					NodeBalancerNodeCreateOptions: oldNode.GetCreateOptions(),
					ID:                            oldNode.ID,
				},
			},
		},
	)
	if err != nil {
		log.Fatal(err)
	}

	newNodes, err := client.ListNodeBalancerNodes(ctx, nb.ID, config.ID, nil)
	if err != nil {
		log.Fatal(err)
	}

	fmt.Println("Updated node:")
	litter.Dump(newNodes[0])
}
  1. Ensure the output is similar to the following:
Old node:
linodego.NodeBalancerNode{
  ID: 1943768499,
  Address: "192.168.149.92:80",
  Label: "test-node",
  Status: "Unknown",
  Weight: 50,
  Mode: "accept",
  ConfigID: 869351,
  NodeBalancerID: 552780,
}

Updated node:
linodego.NodeBalancerNode{
  ID: 1943768499,
  Address: "192.168.149.92:80",
  Label: "test-node-updated",
  Status: "Unknown",
  Weight: 50,
  Mode: "accept",
  ConfigID: 869351,
  NodeBalancerID: 552780,
}
  1. Ensure the IDs of both nodes match and the label of the updated node is test-node-updated.

@lgarber-akamai lgarber-akamai requested a review from a team as a code owner March 11, 2024 15:28
@lgarber-akamai lgarber-akamai requested review from ykim-1 and yec-akamai and removed request for a team March 11, 2024 15:28
Copy link
Contributor

@yec-akamai yec-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Tests all passed locally 🎉

Copy link
Member

@zliang-akamai zliang-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works on my end, nice work!

@lgarber-akamai lgarber-akamai merged commit 823c699 into linode:main Mar 11, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants