Skip to content

upcoming: [M3-7690] - Support PlacementGroups in CLI tool#10438

Merged
abailly-akamai merged 2 commits intolinode:developfrom
carrillo-erik:upcoming/M3-7690
May 7, 2024
Merged

upcoming: [M3-7690] - Support PlacementGroups in CLI tool#10438
abailly-akamai merged 2 commits intolinode:developfrom
carrillo-erik:upcoming/M3-7690

Conversation

@carrillo-erik
Copy link
Contributor

@carrillo-erik carrillo-erik commented May 3, 2024

Description 📝

This PR allows Placement Group data to be passed in as arguments via the Linode CLI.

Changes 🔄

  • The Placement Group object is flattened to obtain and pass the id value. In the end the relevant argument and value are passed as follows: placement_group.id 1234.

Target release date 🗓️

05/13/2024

Preview 📷

Before After
Screenshot 2024-05-03 at 10 22 09 AM linode-cli-arg

How to test 🧪

Prerequisites

(How to setup test environment)

  • ...

Verification steps

(How to verify changes)

  • Verify that a new Linode instance can be created when passing a Placement Group id argument using the linode-cli tool.

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@carrillo-erik carrillo-erik self-assigned this May 3, 2024
@carrillo-erik carrillo-erik requested a review from a team as a code owner May 3, 2024 10:30
@carrillo-erik carrillo-erik requested review from abailly-akamai, dwiley-akamai and mjac0bs and removed request for a team May 3, 2024 10:30
acc.push(` --${key}.user_data="${userData.user_data}"`);
} else if (key === 'placement_group') {
const placementGroup = value as PlacementGroup;
acc.push(` --${key}.id ${placementGroup.id}`);
Copy link
Contributor Author

@carrillo-erik carrillo-erik May 3, 2024

Choose a reason for hiding this comment

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

This line could also be written as

acc.push(`  --${key}.id="${placementGroup.id}"`);

Will need to check with DX team to ensure the correct formatting is applied to the placement_group.id argument.

Screenshot 2024-05-03 at 3 36 51 AM

Screenshot 2024-05-03 at 3 39 08 AM

👀⚠️ I didn't bother redacting the root_pass values in the CLI Modal images because these instances were never created and used only to show the difference in output of the placement_group.id value.

--authorized_users Gritty \\
--interfaces.ipam_address null --interfaces.ipv4.nat_1_1 \"any\" --interfaces.ipv4.vpc \"123\" --interfaces.label null --interfaces.primary true --interfaces.purpose \"vpc\" --interfaces.subnet_id 8296 \\
--metadata.user_data="cmVrbmpnYmloZXVma2xkbQpqZXZia2Y=" \\
--placement_group.id 1234 \\
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is tied to the comment on the other comment below, placement_group.id is expected to be an int and should not require " " marks around the value. Only string type values take " " marks.

@github-actions
Copy link

github-actions bot commented May 3, 2024

Coverage Report:
Base Coverage: 81.82%
Current Coverage: 81.82%

@abailly-akamai
Copy link
Contributor

we need a review from @lgarber-akamai

Copy link

@lgarber-akamai lgarber-akamai left a comment

Choose a reason for hiding this comment

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

LGTM from a CLI perspective!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants