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

change: [M3-7860] - Source ACLB region info from API data and use Jakarta instead of Sydney #10274

Conversation

bnussman-akamai
Copy link
Contributor

@bnussman-akamai bnussman-akamai commented Mar 11, 2024

Description ๐Ÿ“

  • Updates ACLB to pull region information from API data instead of using hardcoded region data wherever possible ๐Ÿ—บ๏ธ
  • Updates the hardcoded beta regions to use Jakarta ๐Ÿ‡ฎ๐Ÿ‡ฉ instead of Sydney ๐Ÿ‡ฆ๐Ÿ‡บ

Preview ๐Ÿ“ท

Before After
Screenshot 2024-03-11 at 4 15 10โ€ฏPM Screenshot 2024-03-11 at 4 15 00โ€ฏPM
Screenshot 2024-03-11 at 4 15 52โ€ฏPM Screenshot 2024-03-11 at 4 16 04โ€ฏPM
Screenshot 2024-03-11 at 4 17 55โ€ฏPM Screenshot 2024-03-11 at 4 17 45โ€ฏPM
โฌ†๏ธ Yes this is expected. The dev environment does not have these regions so we intentionally fallback to a blank flag and region ID

How to test ๐Ÿงช

Note

Behavior will be different in dev compared to production because dev lacks certain regions in the API response.
You should test this PR using both dev and production

Testing dev

  • Login to dev-test-aglb
  • Verify you see region IDs listed on the landing page, details page, and settings page
  • Verify the create page lists the proper regions (uses Jakarta ๐Ÿ‡ฎ๐Ÿ‡ฉ instead of Sydney ๐Ÿ‡ฆ๐Ÿ‡บ)

Yes, this is expected:
Screenshot 2024-03-11 at 4 23 14โ€ฏPM

Testing staging/production

  • Login to prod-test-065
  • Verify you see region ID and labels on the landing page table
  • Verify you see region ID and labels, and country flags on the details page and settings page
  • Verify the create page lists the proper regions (uses Jakarta ๐Ÿ‡ฎ๐Ÿ‡ฉ instead of Sydney ๐Ÿ‡ฆ๐Ÿ‡บ)

Screenshot 2024-03-11 at 4 24 52โ€ฏPM

As an Author I have considered ๐Ÿค”

  • ๐Ÿ‘€ 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

@bnussman-akamai bnussman-akamai added the ACLB Relating to the Akamai Cloud Load Balancer label Mar 11, 2024
@bnussman-akamai bnussman-akamai self-assigned this Mar 11, 2024
@bnussman-akamai bnussman-akamai requested a review from a team as a code owner March 11, 2024 20:28
@bnussman-akamai bnussman-akamai requested review from carrillo-erik and jaalah-akamai and removed request for a team March 11, 2024 20:28
Copy link

github-actions bot commented Mar 11, 2024

Coverage Report: โœ…
Base Coverage: 81.39%
Current Coverage: 81.39%

Copy link
Contributor

@jaalah-akamai jaalah-akamai left a comment

Choose a reason for hiding this comment

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

I do find it weird that at times we'll only display the region id, but I'm sure that's temporary?

โœ… Confirmed empty flag state in dev and that Jakarta (id-cgk) displays instead of Sidney (Region Id only)
โœ… Confirmed flags display in prod account and that Jakarta (id-cgk) displays instead of Sidney

@bnussman-akamai
Copy link
Contributor Author

Yeah, it's not great @jaalah-akamai.

It's more of a configuration problem. The VMs that run the underlying ACLBs are deployed in regions that don't exist in alpha. What really should have happened is that alpha ACLBs should be using alpha only regions (basically just Newark), but they chose to deploy alpha ACLBs nodes using production Linodes.

We could continue to hardcode the values to keep alpha looking nice but I don't think we should be doing that to cover up for other team's design choices.

@bnussman-akamai bnussman-akamai merged commit 0914e6d into linode:develop Mar 13, 2024
18 checks passed
@carrillo-erik
Copy link
Contributor

I was able to verify both user accounts by logging into both environments as described in the PR description. Really appreciate the comment blocks this PR removes, every bit helps. Also, the refactor of the RegionsCell component slimmed down the code base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACLB Relating to the Akamai Cloud Load Balancer Ready for Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants