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

upcoming: [M3-7756] - Placement Groups limits updates #10191

Merged
merged 7 commits into from
Feb 15, 2024

Conversation

abailly-akamai
Copy link
Contributor

@abailly-akamai abailly-akamai commented Feb 13, 2024

Description 📝

This PR implements recent changes to the Placement Groups API that need to be rolled into the APP to prevent building on the wrong specs (and incur more refactoring as we go)

Changes 🔄

Type changes

  • new Capability for account: "Placement Group"
  • new Capability for region: "Placement Group"
  • compliant changed to is_compliant
  • Region Interface:
    • maximum_pgs_per_customer: number
    • maximum_vms_per_pg: number
      as a result of 3.1 and 3.2 we need to deprecate the "capacity" field from the PG Interface, update related code and mock data/factories
  • An update in the API for the migrate flow has brought up the need for a strict value when:
    • doing a POST to create a PG
    • POST assigning a Linode to a PG

Preview 📷

There is no change to the UI, everything should be looking and behaving exactly the same except for the PG landing page which lost the sentence about max PG amount reached

How to test 🧪

Prerequisites

  • Turn both the "Placement Group" flag and MSW

Verification steps

  • Very the Placement Group feature as a whole

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

@@ -3,6 +3,7 @@ import { COUNTRY_CODE_TO_CONTINENT_CODE } from './constants';
export type Capabilities =
| 'Bare Metal'
| 'Block Storage'
| 'Block Storage Migrations'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated to this PR. This key actually is returned from the API so I added it to our types

@@ -27,6 +29,8 @@ export interface Region {
label: string;
country: Country;
capabilities: Capabilities[];
maximum_pgs_per_customer: number;
maximum_vms_per_pg: number;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are the big changes warranting this PR. Limits now come from the region endpoint, not the PG itself

'aria-disabled',
'true'
);
});
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The UI has changed so we don't need to test for this anymore

linodesError: error,
region,
};
};
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 our new hook to:

  • have a handy util to fetch PG data
  • reduce repetition in code and keep things DRYer
  • facilitate future refactors

It is meant (at least for now) to be consumed by components already having a defined placement group

packages/manager/src/mocks/serverHandlers.ts Outdated Show resolved Hide resolved
@abailly-akamai abailly-akamai marked this pull request as ready for review February 13, 2024 22:13
@abailly-akamai abailly-akamai requested a review from a team as a code owner February 13, 2024 22:13
@abailly-akamai abailly-akamai requested review from cpathipa and hkhalil-akamai and removed request for a team February 13, 2024 22:13
@abailly-akamai abailly-akamai changed the title upcoming: [M3-7756] - Placement Groups type updates upcoming: [M3-7756] - Placement Groups limits updates Feb 13, 2024
Copy link

github-actions bot commented Feb 13, 2024

Coverage Report:
Base Coverage: 81.23%
Current Coverage: 81.26%

Copy link
Contributor

@hkhalil-akamai hkhalil-akamai left a comment

Choose a reason for hiding this comment

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

Tested and found no regressions.

packages/manager/src/mocks/serverHandlers.ts Outdated Show resolved Hide resolved
@abailly-akamai
Copy link
Contributor Author

@hkhalil-akamai regions.json is generated at build time to act as a cold cache for the api/v4/regions query

Now, while it's super useful in the case of one click apps since that query takes a few seconds, it does seem trivial to cold cache the regions endpoint. I'll ask at the cafe 👍

@bnussman-akamai bnussman-akamai added the Approved Multiple approvals and ready to merge! label Feb 14, 2024
@carrillo-erik
Copy link
Contributor

The updates looks good, once this is merged I'll update my changes in my PR.

@abailly-akamai abailly-akamai merged commit 2bc4ccc into linode:develop Feb 15, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Placement Groups
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants