upcoming: [M3-7520] - AGLB Fixes and Improvements #9954

bnussman-akamai · 2023-12-01T21:53:16Z

Description 📝

Fixes ActionCell styles 🎨
Make copy changes required by technical writer ✏️
- Add rule drawer now has dynamic copy depending on the protocol
Improves spacing of some error notices ⛔️
Fixes a lot of validation / payloads to match what the Linode API allows 🚛
- You should now be able to add TCP Rules now
- You should be able to delete Rules

Preview 📷

Before	After

How to test 🧪

Prerequisites

Use MSW to test or use the dev-test-aglb credentials

Verification steps

Verify the changes listed in the Description section
Check for any regressions to existing behavior

As an Author I have considered 🤔

github-actions · 2023-12-04T14:09:50Z

Coverage Report: ✅
Base Coverage: 85.37%
Current Coverage: 85.39%

bnussman-akamai · 2023-12-04T20:30:34Z

...nager/src/features/LoadBalancers/LoadBalancerDetail/Certificates/CreateCertificateDrawer.tsx

-      label: '',
-      type,
-    },
+    initialValues: initialValues[type],


Added dynamic initialValues because ca certs cannot have a key

bnussman-akamai · 2023-12-04T20:30:57Z

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/LoadBalancerSummary.tsx


  const items = [
    {
      title: 'Hostname',
-      value: <IPAddress ips={[loadbalancer?.hostname ?? '']} isHovered />,
+      value: <IPAddress ips={[loadbalancer?.hostname ?? 'None']} isHovered />,


Show None instead of nothing when there is no hostname.

Good call. One nitpick: do we want to hide the copy button when there is no hostname? It seems silly to be able to copy "None". May be more trouble than it's worth to change, because we'd also need some minor styling.

bnussman-akamai · 2023-12-04T20:31:31Z

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/DeleteRuleDialog.tsx

+      label: route.label,
+      protocol: route.protocol,


The API needs us to pass these values. Passing protocol also helps our validation.

bnussman-akamai · 2023-12-04T20:32:18Z

packages/manager/src/features/LoadBalancers/LoadBalancerLanding/LoadBalancerRow.tsx

@@ -36,13 +39,23 @@ export const LoadBalancerRow = ({ handlers, loadBalancer }: Props) => {
        </TableCell>
      </Hidden>
      <Hidden smDown>
-        <TableCell>{hostname}</TableCell>
+        <TableCell>
+          {hostname ? <IPAddress ips={[hostname]} /> : 'None'}


Show None if there is no hostname. Also makes the hostname copyable.

bnussman-akamai · 2023-12-04T20:32:56Z

packages/manager/src/features/LoadBalancers/LoadBalancerLanding/LoadBalancerRow.tsx

      </Hidden>
      <Hidden mdDown>
        <TableCell>
-          {regions.map((region) => (
+          <Stack py={1} spacing={0.5}>
+            {alphaRegions.map(({ id, label }) => (


We're going to hardcode the regions for now because the dev environment does not return the production regions we need.

bnussman-akamai · 2023-12-04T20:33:11Z

packages/manager/src/features/LoadBalancers/LoadBalancerLanding/Ports.tsx

@@ -21,6 +21,10 @@ export const Ports = ({ loadbalancerId }: PortProps) => {
    return <Skeleton sx={{ minWidth: '100px' }} />;
  }

+  if (ports?.length === 0) {
+    return <Typography>None</Typography>;


Shows None is no configurations have been setup

bnussman-akamai · 2023-12-04T20:33:23Z

packages/validation/src/loadbalancers.schema.ts

Changes here match APIv4

bnussman-akamai · 2023-12-04T20:51:30Z

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/utils.ts

+export const getNormzlizedRulePayload = (rule: RulePayload) => ({
+  match_condition: {
+    ...rule.match_condition,
+    hostname: rule.match_condition.hostname
+      ? rule.match_condition.hostname
+      : null,
+  },
+  service_targets: rule.service_targets,
+});


I really didn't want to have to do this, but we need to so that the API accepts our payload. I tired doing this other ways but this seemed to be the most typesafe and straightforward way

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/LoadBalancerSummary.tsx

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/utils.test.ts

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/utils.ts

...manager/src/features/LoadBalancers/LoadBalancerDetail/ServiceTargets/ServiceTargetDrawer.tsx

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/ServiceTargets/utils.test.ts

tyler-akamai · 2023-12-05T15:41:08Z

Verified:

Action menu alignment
Better description in Add Rules
Better spacing for error in Add a Service Target drawer
Better spacing for error in Create Route drawer
No regressions to existing behavior
General code review

tyler-akamai · 2023-12-05T16:17:53Z

Should we clamp these after a certain number of lines?

tyler-akamai · 2023-12-05T16:02:20Z

packages/manager/cypress/e2e/core/loadBalancers/load-balancer-landing-page.spec.ts

-              'be.visible'
-            );
-          });
+          // TODO: AGLB - Confirm that regions from the API are listed for load balancer


were these not working?

The Dev environment does not have the same regions as prod, so we need to hardcode for now.

tyler-akamai · 2023-12-05T16:20:33Z

packages/api-v4/src/aglb/types.ts

-  path: string;
-  host: string;
+  path?: string | null;
+  host?: string | null;


do we need this to be both optional and potentially null?

I think we want both because we want users to be able to pass null or completely omit the property.

tyler-akamai · 2023-12-05T16:20:49Z

packages/api-v4/src/aglb/types.ts

@@ -169,7 +169,7 @@ export interface ServiceTarget extends ServiceTargetPayload {

 export interface Endpoint {
  ip: string;
-  host?: string;
+  host?: string | null;


bnussman-akamai · 2023-12-05T16:33:57Z

@tyler-akamai We will clamp the regions post-alpha

mjac0bs

It's exciting to see things working more smoothly, nicely done! A couple minor things of feedback, approving pending those are addressed.

Verified:
✅ Fixed ActionCell styles
✅ Add rule drawer now has dynamic copy depending on the protocol
✅ Error notice spacing looks good
Fixes a lot of validation / payloads to match what the Linode API allows 🚛

❗ I seem to be getting an error now on the service target label field when trying to add a rule. See screencap below.

✅ Able to add TCP Rules now (this works, despite the visible error)
✅ Able to delete Rules

Screen.Recording.2023-12-05.at.9.23.55.AM.mov

packages/manager/.changeset/pr-9954-upcoming-features-1701698414248.md

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/LoadBalancerSummary.tsx

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/utils.test.ts

mjac0bs · 2023-12-05T17:08:37Z

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/utils.test.ts

Thanks for the 🧪 !

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/utils.ts

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/ServiceTargets/utils.test.ts

packages/manager/src/features/LoadBalancers/LoadBalancerLanding/LoadBalancerRow.tsx

mjac0bs · 2023-12-05T17:20:00Z

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/LoadBalancerSummary.tsx


  const items = [
    {
      title: 'Hostname',
-      value: <IPAddress ips={[loadbalancer?.hostname ?? '']} isHovered />,
+      value: <IPAddress ips={[loadbalancer?.hostname ?? 'None']} isHovered />,


Good call. One nitpick: do we want to hide the copy button when there is no hostname? It seems silly to be able to copy "None". May be more trouble than it's worth to change, because we'd also need some minor styling.

mjac0bs · 2023-12-05T17:31:43Z

packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Certificates/index.tsx

@@ -38,7 +38,7 @@ export const LoadBalancerCertificates = () => {
        Upload certificates to your Load Balancer for use across your
        Configurations and Service Targets.
      </Typography>
-      <Tabs index={tabIndex === -1 ? 0 : tabIndex}>
+      <Tabs index={tabIndex === -1 ? 0 : tabIndex} onChange={() => null}>


Oh yay, thank you 🙌🏼 I noticed this and was trying to decide the best place to mention it.

bnussman added 2 commits December 1, 2023 16:12

fix action cell

94b2465

update copy for rule drawer

0d156f1

bnussman-akamai added the ACLB Relating to the Akamai Cloud Load Balancer label Dec 1, 2023

bnussman-akamai self-assigned this Dec 1, 2023

bnussman-akamai marked this pull request as ready for review December 4, 2023 13:22

bnussman-akamai requested a review from a team as a code owner December 4, 2023 13:22

bnussman-akamai requested review from carrillo-erik and tyler-akamai and removed request for a team December 4, 2023 13:22

spacing fixes

35dbf26

bnussman-akamai requested review from mjac0bs and cpathipa December 4, 2023 13:58

add changesets

c4aca0d

bnussman added 7 commits December 4, 2023 10:16

Merge branch 'develop' into aglb-fixes-1

b8c39d8

normalize service target payload

5a014b9

payload fixes

ab6c46c

tcp rule payload fix

5d9f012

make deleting rules work

faa1bfa

clean up for demo

3793553

make e2es pass for alpha

2553e90

bnussman-akamai requested a review from a team as a code owner December 4, 2023 19:55

bnussman-akamai requested review from cliu-akamai and removed request for a team December 4, 2023 19:55

clean up

ba3d8af

bnussman-akamai commented Dec 4, 2023

View reviewed changes

add tests

0956f04

bnussman-akamai commented Dec 4, 2023

View reviewed changes

bnussman-akamai changed the title ~~upcoming: AGLB Fixes and Improvements~~ upcoming: [M3-7520] - AGLB Fixes and Improvements Dec 4, 2023

bnussman-akamai added the Ready for Review label Dec 4, 2023

Merge branch 'develop' into aglb-fixes-1

37e3022

cpathipa reviewed Dec 5, 2023

View reviewed changes

small fixes

7df6d06

spelling fixes

f55d697

tyler-akamai reviewed Dec 5, 2023

View reviewed changes

tyler-akamai approved these changes Dec 5, 2023

View reviewed changes

fix remaining bugs

69d3aae

cpathipa approved these changes Dec 5, 2023

View reviewed changes

fix remaining aglb console errors

9ca5e93

mjac0bs approved these changes Dec 5, 2023

View reviewed changes

mjac0bs reviewed Dec 5, 2023

View reviewed changes

bnussman added 2 commits December 5, 2023 12:34

feedback @mjac0bs

08af0c1

fix last beta chip console error

4c3e542

bnussman-akamai merged commit 27616c5 into linode:develop Dec 5, 2023
15 checks passed

mjac0bs mentioned this pull request Dec 11, 2023

Release v1.108.0 - staging → master #9986

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upcoming: [M3-7520] - AGLB Fixes and Improvements #9954

upcoming: [M3-7520] - AGLB Fixes and Improvements #9954

bnussman-akamai commented Dec 1, 2023 •

edited

Loading

github-actions bot commented Dec 4, 2023 •

edited

Loading

bnussman-akamai Dec 4, 2023

bnussman-akamai Dec 4, 2023

mjac0bs Dec 5, 2023

bnussman-akamai Dec 4, 2023

bnussman-akamai Dec 4, 2023

bnussman-akamai Dec 4, 2023

bnussman-akamai Dec 4, 2023

bnussman-akamai Dec 4, 2023

bnussman-akamai Dec 4, 2023

tyler-akamai commented Dec 5, 2023 •

edited

Loading

tyler-akamai commented Dec 5, 2023

tyler-akamai Dec 5, 2023

bnussman-akamai Dec 5, 2023

tyler-akamai Dec 5, 2023

bnussman-akamai Dec 5, 2023

tyler-akamai Dec 5, 2023

bnussman-akamai commented Dec 5, 2023

mjac0bs left a comment •

edited

Loading

mjac0bs Dec 5, 2023

mjac0bs Dec 5, 2023

mjac0bs Dec 5, 2023

upcoming: [M3-7520] - AGLB Fixes and Improvements #9954

upcoming: [M3-7520] - AGLB Fixes and Improvements #9954

Conversation

bnussman-akamai commented Dec 1, 2023 • edited Loading

Description 📝

Preview 📷

How to test 🧪

Prerequisites

Verification steps

As an Author I have considered 🤔

github-actions bot commented Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tyler-akamai commented Dec 5, 2023 • edited Loading

tyler-akamai commented Dec 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnussman-akamai commented Dec 5, 2023

mjac0bs left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnussman-akamai commented Dec 1, 2023 •

edited

Loading

github-actions bot commented Dec 4, 2023 •

edited

Loading

tyler-akamai commented Dec 5, 2023 •

edited

Loading

mjac0bs left a comment •

edited

Loading