upcoming: [APL-55] - Application platform for Linode Kubernetes
Description 📝
UI implementation for the upcoming APL ( Application platform for Linode) feature
Changes 🔄
List any change relevant to the reviewer.
- APL UI elements added to the createKubernetes and KubernetesOverview pages
- HA mode will automatically be set whenever APL is enabled
- Kubernetes plans that are too small will be grayed out whenever APL is enabled
- Breadcrumb is extended so it support a array of numbers
Target release date 🗓️
N/A
Preview 📷
https://github.com/user-attachments/assets/9a5a1fcd-12bc-4cbd-8414-8552f1a61b84
How to test 🧪
- coming soon
please add the 'do not merge' and 'work in progress' tags
This PR is stale because it has been open 15 days with no activity. Please attend to this PR or it will be closed in 5 days
@dennisvankekem is this PR still relevant or a work in progress? I am going to help move things with APL and wondering about the state of this contribution. If moving forward with it, can we get it rebased and updated? I'll start reviewing the code then - thx!
@abailly-akamai I will update this PR with some final minor changes and then the code is ready to test. To test the functionality of the feature however we have to wait until the APL feature in API is merged first. I will rebase and mark this PR for review once I've pushed the last changes which is expected somewhere tomorrow
@abailly-akamai Thanks for your review, sadly I didn't get notified by Github in my email for some reason, so your review wasn't noticed by me. I will look at it tomorrow!
Changes:
- Split conditional api_root call into separate functions for
- GetKubernetesCluster
- CreateKubernetesCluster
- Removed Apl_enabled from types and instead extended said types with dedicated beta type
- Created AplAvailability hook where the beta flag logic is handled
- Made AplEnabled required and removed the possibility of being undefined
- CreateCluster.tsx now has conditional logic based on flag to use beta features.
- createCluster function itself has been rewritten to handle calls conditional to avoid duplicated functions. (This function also gets send to child components and I do not want that to cascade in more duplicated code)
- Change const APL to apl_enabled as the name APL for state might be confusing in what it does
- Removed console.error from APLSummary.tsx
One major problem is that with this current solution a lot of Kubernetes functions needed to be duplicated. Although the damage for create functions is somewhat controlled, checking for is_apl_enabled in ClusterOverview would borderline cascade into an entire duplication of all Kubernetes functions just for one parameter. With the main culprit being:
export const useKubernetesClusterQuery = (id: number) => {
return useQuery<KubernetesCluster, APIError[]>(kubernetesQueries.cluster(id));
};
Calling for kubernetesQueries which again cascades into all Kubernetes queries, which are in return strongly typed and therefore cannot easily be conditional rendered based on showAPL flag.
Please let me know what to do with this, perhaps there is a smart solution for this?
@dennisvankekem thx for addressing this initial feedback - I will give it another go and see what we can come up with to avoid too much duplication
@dennisvankekem in the interest of time we may want to loosen the typing grip based on the refactor and duplication needed. I did not expect things to cascade that far and I agree the debt may trump the original intent.
Am thinking we could do:
export interface CreateKubeClusterPayload {
label?: string; // Label will be assigned by the API if not provided
region?: string; // Will be caught by Yup if undefined
node_pools: CreateNodePoolData[];
k8s_version?: string; // Will be caught by Yup if undefined
control_plane?: ControlPlaneOptions;
// an important comment about this type, such as this needs a value for BETA
apl_enabled: boolean | undefined
}
which is not idea but would get us to a compromise where we are always reminder to declare the property. Thoughts?
Meanwhile Can you:
- fix the current type issues
- fix any broken test
also, you probably to conditionaly render copy referring to APL. ex (i don't think we want that copy for non-adopters since APL isn't a thing to them):
Coverage Report: ✅
Base Coverage: 87.22%
Current Coverage: 87.23%
'work in progress' and 'do not merge' labels may be removed
✅ Payload is looking good
Non-blocking: UX needs to consider the tablet/mobile breakpoints for this. As of right now, the UI does not look great at these sizes.
Cloud Manager E2E
Run #6641
Run Properties:
Passed #6641 •
c0d8900fb3: upcoming: [APL-55] - Application platform for Linode Kubernetes (#10753)
| Project |
Cloud Manager E2E
|
| Run status |
|
| Run duration | 25m 28s |
| Commit |
|
| Committer | Dennis van Kekem |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
2
|
|
|
2
|
|
|
0
|
|
|
429
|