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

ref: PG affinity_type -> placement_group_type; is_strict -> placement_group_policy #546

Merged

Conversation

lgarber-akamai
Copy link
Contributor

@lgarber-akamai lgarber-akamai commented Jul 8, 2024

📝 Description

This pull request implements the following upcoming breaking changes for the VM Placement GA:

  • AffinityType -> PlacementGroupType
  • IsStrict -> PlacementGroupPolicy

NOTE: We will need communication around this breaking change in the release notes.

✔️ How to Test

The following test steps assume you have pulled down this PR locally and have pointed your local environment at an API instance where PGs are available:

export LINODE_URL=...
export LINODE_TOKEN=...

Integration Testing

make ARGS="-run TestPlacementGroup TestInstance_withPG" fixtures

Manual Testing

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

import (
	"context"
	"fmt"
	"log"
	"os"

	"github.com/linode/linodego"
)

const targetRegion = "eu-west"

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

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

	// Create a PG
	pg, err := client.CreatePlacementGroup(ctx, linodego.PlacementGroupCreateOptions{
		Label:                "test-pg",
		Region:               targetRegion,
		PlacementGroupType:   linodego.PlacementGroupTypeAntiAffinityLocal,
		PlacementGroupPolicy: linodego.PlacementGroupPolicyStrict,
	})
	if err != nil {
		log.Fatal(err)
	}

	// Provision two instances assigned to the PG
	compliantOnly := true

	for i := 0; i < 2; i++ {
		_, err := client.CreateInstance(ctx, linodego.InstanceCreateOptions{
			Region: targetRegion,
			Type:   "g6-nanode-1",
			Label:  fmt.Sprintf("test-instance-%d", i),
			PlacementGroup: &linodego.InstanceCreatePlacementGroupOptions{
				ID:            pg.ID,
				CompliantOnly: &compliantOnly,
			},
		})
		if err != nil {
			log.Fatal(err)
		}
	}
}
  1. Navigate to Cloud Manager and ensure a new Placement Group has been created with two instances assigned to it.
  2. Modify this code with update logic, assignment logic, etc.
  3. Re-run the file and ensure everything works as expected.

@lgarber-akamai lgarber-akamai added breaking-change for breaking changes in the changelog. improvement for improvements in existing functionality in the changelog. do-not-merge PRs that should not be merged until the commented issue is resolved labels Jul 8, 2024
@lgarber-akamai lgarber-akamai marked this pull request as ready for review July 8, 2024 19:54
@lgarber-akamai lgarber-akamai requested a review from a team as a code owner July 8, 2024 19:54
@lgarber-akamai lgarber-akamai requested review from ykim-1 and ezilber-akamai and removed request for a team July 8, 2024 19:54
@lgarber-akamai lgarber-akamai force-pushed the ref/vm-placement-pre-ga-changes branch from 0889b61 to 54770e4 Compare July 11, 2024 15:51
@lgarber-akamai lgarber-akamai changed the title ref: Placement Group affinity_type -> placement_group_type; is_strict -> placement_group_policy ref: PG affinity_type -> placement_group_type; is_strict -> placement_group_policy Jul 11, 2024
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.

Works well on my end!

Copy link
Contributor

@ezilber-akamai ezilber-akamai left a comment

Choose a reason for hiding this comment

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

Working as expected!

Copy link
Contributor

@ykim-1 ykim-1 left a comment

Choose a reason for hiding this comment

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

LGTM, working as expected. Nice work!

@lgarber-akamai lgarber-akamai removed the do-not-merge PRs that should not be merged until the commented issue is resolved label Jul 22, 2024
@lgarber-akamai lgarber-akamai merged commit 0269a9a into linode:main Jul 22, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change for breaking changes in the changelog. improvement for improvements in existing functionality in the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants