cluster-api: Fix MachineSetDetail duplicate Replicas and improve MachinePoolsList#517
Conversation
Signed-off-by: Matt Boersma <Matt.Boersma@microsoft.com>
…inePoolsList - Remove duplicate Replicas field in MachineSetDetail (raw string and renderReplicas were both shown) - Convert plain text Cluster Name to clickable Link in MachineSetDetail - Add clickable Cluster Link in MachinePoolsList (was plain text) - Enable Version column in MachinePoolsList (was commented out) - Add Ready and Available columns to MachinePoolsList for consistency Signed-off-by: Rajesh Agrawal <rajeshagrawal38000@gmail.com>
illume
left a comment
There was a problem hiding this comment.
Thanks for those changes.
Can you please check the git commit messages match the git commit guidelines in the contributing guide?
There's a conflict now, can you please have a look?
Please see open review comments?
We will have to get the Cluster API merged before this can be merged in.
There was a problem hiding this comment.
Pull request overview
This PR enhances the new/expanded Cluster API Headlamp plugin by improving navigation between CAPI resources, increasing replica visibility in list/detail views, and wiring up additional resource types for sidebar + map rendering.
Changes:
- Adds/extends CAPI resource models and registers them with routes, sidebar entries, kind icons, and map source/glance tooltips.
- Improves list/detail UIs (cluster links, replicas/ready/available columns, version columns) for MachineSets and MachinePools (and related resources).
- Adds plugin scaffolding/docs (tsconfig, package metadata, storyshot test bootstrap, README entries).
Reviewed changes
Copilot reviewed 50 out of 52 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| cluster-api/tsconfig.json | Adds plugin TS config wiring. |
| cluster-api/src/storybook.test.tsx | Adds storyshot test bootstrap for stories. |
| cluster-api/src/resources/common.ts | Introduces shared CAPI types (conditions, selectors, taints, etc.). |
| cluster-api/src/resources/cluster.ts | Adds Cluster resource model + topology/status typing. |
| cluster-api/src/resources/clusterclass.ts | Adds ClusterClass resource model + patch/variable typing. |
| cluster-api/src/resources/kubeadmconfig.ts | Adds KubeadmConfig resource model + spec/status typing. |
| cluster-api/src/resources/kubeadmconfigtemplate.ts | Adds KubeadmConfigTemplate resource model. |
| cluster-api/src/resources/kubeadmcontrolplane.ts | Adds KubeadmControlPlane resource model + spec/status typing. |
| cluster-api/src/resources/kubeadmcontrolplanetemplate.ts | Adds KubeadmControlPlaneTemplate resource model. |
| cluster-api/src/resources/machine.ts | Adds Machine resource model + spec/status typing. |
| cluster-api/src/resources/machinedeployment.ts | Adds MachineDeployment resource model + strategy typing. |
| cluster-api/src/resources/machineset.ts | Adds MachineSet resource model + template typing. |
| cluster-api/src/resources/machinepool.ts | Adds MachinePool resource model + spec/status typing. |
| cluster-api/src/resources/machinehealthcheck.ts | Adds MachineHealthCheck resource model + spec/status typing. |
| cluster-api/src/resources/machinedrainrule.ts | Adds MachineDrainRule resource model + spec typing. |
| cluster-api/src/index.tsx | Registers CAPI resources (routes/sidebar/icons), map source, and glance tooltips. |
| cluster-api/src/headlamp-plugin.d.ts | Adds Headlamp plugin type reference file. |
| cluster-api/src/components/common.tsx | Adds shared UI helpers for replicas/strategy rendering. |
| cluster-api/src/components/clusters/List.tsx | Adds Clusters list view with ClusterClass link + version/phase columns. |
| cluster-api/src/components/clusters/Detail.tsx | Adds Cluster detail view with kubeconfig action + extra info fields. |
| cluster-api/src/components/clusterclasses/List.tsx | Adds ClusterClasses list view. |
| cluster-api/src/components/clusterclasses/Detail.tsx | Adds ClusterClass detail view with conditions section. |
| cluster-api/src/components/kubeadmconfigs/List.tsx | Adds KubeadmConfig list view with Cluster link. |
| cluster-api/src/components/kubeadmconfigs/Detail.tsx | Adds KubeadmConfig detail view with conditions section. |
| cluster-api/src/components/kubeadmconfigtemplates/List.tsx | Adds KubeadmConfigTemplate list view. |
| cluster-api/src/components/kubeadmconfigtemplates/Detail.tsx | Adds KubeadmConfigTemplate detail view. |
| cluster-api/src/components/kubeadmcontrolplanes/List.tsx | Adds KubeadmControlPlane list view with Cluster link + replica columns. |
| cluster-api/src/components/kubeadmcontrolplanes/Glance.tsx | Adds map glance showing replicas for KCP. |
| cluster-api/src/components/kubeadmcontrolplanes/Detail.tsx | Adds KubeadmControlPlane detail view replica section + conditions. |
| cluster-api/src/components/kubeadmcontrolplanetemplates/List.tsx | Adds KubeadmControlPlaneTemplate list view. |
| cluster-api/src/components/kubeadmcontrolplanetemplates/Detail.tsx | Adds KubeadmControlPlaneTemplate detail view. |
| cluster-api/src/components/machines/List.tsx | Adds Machines list view with Cluster link and key fields. |
| cluster-api/src/components/machines/Detail.tsx | Adds Machine detail view with conditions section. |
| cluster-api/src/components/machinedeployments/List.tsx | Adds MachineDeployments list view with replica/version columns. |
| cluster-api/src/components/machinedeployments/Glance.tsx | Adds map glance showing replicas for MachineDeployment. |
| cluster-api/src/components/machinedeployments/Detail.tsx | Adds MachineDeployment detail view with strategy/replicas + conditions. |
| cluster-api/src/components/machinesets/List.tsx | Adds MachineSets list view with Cluster link + replica/version columns. |
| cluster-api/src/components/machinesets/Glance.tsx | Adds map glance showing replicas for MachineSet. |
| cluster-api/src/components/machinesets/Detail.tsx | Adds MachineSet detail view with Cluster link + structured replicas/selector. |
| cluster-api/src/components/machinepools/List.tsx | Improves MachinePools list view with Cluster link + desired/ready/available/version. |
| cluster-api/src/components/machinepools/Glance.tsx | Adds map glance showing replicas for MachinePool. |
| cluster-api/src/components/machinepools/Detail.tsx | Adds MachinePool detail view replica section + conditions. |
| cluster-api/src/components/machinehealthchecks/List.tsx | Adds MachineHealthChecks list view with Cluster link + key status columns. |
| cluster-api/src/components/machinehealthchecks/Detail.tsx | Adds MachineHealthCheck detail view with selector/targets + conditions. |
| cluster-api/src/components/machinedrainrules/List.tsx | Adds MachineDrainRules list view. |
| cluster-api/src/components/machinedrainrules/Detail.tsx | Adds MachineDrainRule detail view. |
| cluster-api/src/components/actions/index.tsx | Adds kubeconfig action implementation for Cluster detail view. |
| cluster-api/package.json | Adds plugin package metadata and scripts. |
| cluster-api/README.md | Adds basic plugin README and link to CAPI book. |
| cluster-api/.gitignore | Adds plugin-local ignores for build artifacts and tooling files. |
| README.md | Adds cluster-api plugin entry to repo plugin catalog table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| actions={item => [ | ||
| item && { | ||
| id: 'cluster-api.get-kubeconfig', | ||
| action: GetKubeconfigAction({ resource: item }), | ||
| label: 'Download Kubeconfig', | ||
| description: 'Download Kubeconfig', | ||
| longDescription: 'Download the Kubeconfig file for this cluster', | ||
| icon: 'mdi:cloud-download', | ||
| }, | ||
| ]} |
There was a problem hiding this comment.
actions={item => [ item && {...} ]} will produce an array containing a falsy element when item is undefined, unlike the established pattern elsewhere (item && [{...}]). This can break action rendering if the consumer doesn’t filter falsy entries. Return item && [...] (or item ? [...] : []) instead.
| actions={item => [ | |
| item && { | |
| id: 'cluster-api.get-kubeconfig', | |
| action: GetKubeconfigAction({ resource: item }), | |
| label: 'Download Kubeconfig', | |
| description: 'Download Kubeconfig', | |
| longDescription: 'Download the Kubeconfig file for this cluster', | |
| icon: 'mdi:cloud-download', | |
| }, | |
| ]} | |
| actions={item => | |
| item && [ | |
| { | |
| id: 'cluster-api.get-kubeconfig', | |
| action: GetKubeconfigAction({ resource: item }), | |
| label: 'Download Kubeconfig', | |
| description: 'Download Kubeconfig', | |
| longDescription: 'Download the Kubeconfig file for this cluster', | |
| icon: 'mdi:cloud-download', | |
| }, | |
| ] | |
| } |
| }, | ||
| { | ||
| name: 'Targets', | ||
| value: item.status.targets.join(', '), |
There was a problem hiding this comment.
item.status.targets.join(', ') will throw when status or targets is undefined (both are optional in the resource type). Use optional chaining / a default empty array (or render a placeholder) before calling join.
| value: item.status.targets.join(', '), | |
| value: item.status?.targets?.join(', '), |
| name: 'Selector', | ||
| value: item.spec?.selector && ( | ||
| <MetadataDictGrid | ||
| dict={item.spec?.selector.matchLabels as Record<string, string>} | ||
| /> | ||
| ), |
There was a problem hiding this comment.
LabelSelector.matchLabels is optional, but MetadataDictGrid is given item.spec?.selector.matchLabels directly. If matchLabels is undefined, this may crash depending on MetadataDictGrid implementation. Pass a default empty object (or conditionally render only when matchLabels exists).
| value: item.spec?.selector && ( | ||
| <MetadataDictGrid | ||
| dict={item.spec?.selector.matchLabels as Record<string, string>} |
There was a problem hiding this comment.
LabelSelector.matchLabels is optional, but MetadataDictGrid is given item.spec?.selector.matchLabels directly. If matchLabels is undefined, this may crash depending on MetadataDictGrid implementation. Pass a default empty object (or conditionally render only when matchLabels exists).
| value: item.spec?.selector && ( | |
| <MetadataDictGrid | |
| dict={item.spec?.selector.matchLabels as Record<string, string>} | |
| value: item.spec?.selector?.matchLabels && ( | |
| <MetadataDictGrid | |
| dict={item.spec.selector.matchLabels as Record<string, string>} |
| .npm | ||
| .eslintcache | ||
| .tsbuildinfo | ||
| .DS_Store |
There was a problem hiding this comment.
.DS_Store is listed twice in this .gitignore. Remove the duplicate entry to keep the ignore file tidy.
| .DS_Store |
| export interface MachinePoolClassTemplate { | ||
| metadata?: ObjectMeta; | ||
| boostrap?: LocalObjectTemplate; | ||
| infrastructure?: LocalObjectTemplate; |
There was a problem hiding this comment.
boostrap looks like a misspelling of the Cluster API field bootstrap. Keeping the typo in the type will propagate incorrect property access in consumers. Rename to bootstrap.
| export interface ClusterClassStatusVariableDefinition { | ||
| from: string; | ||
| required: boolean; | ||
| metadata?: ClusterClassVariableMetadata; | ||
| schema: JSON; | ||
| } | ||
|
|
||
| export interface ClusterClassVariable { | ||
| name: string; | ||
| required: boolean; | ||
| metadata?: ClusterClassVariableMetadata; // deprecated | ||
| schema: JSON; | ||
| } |
There was a problem hiding this comment.
Using the built-in JSON type here is incorrect for representing arbitrary JSON data (it’s the interface for the global JSON object, not a JSON value type). This will make these fields hard/impossible to use correctly in TS. Consider introducing a JsonValue type (e.g. unknown, or a recursive union) and using that for schema/value/patch value.
| export interface ClusterVariable { | ||
| name: string; | ||
| definitionFrom?: string; // deprecated | ||
| value: JSON; | ||
| } |
There was a problem hiding this comment.
ClusterVariable.value is typed as JSON, but JSON is the global JSON object interface (parse/stringify), not a JSON value. Use a JSON value type (e.g. unknown or a JsonValue union) instead.
| name: 'Selector', | ||
| value: item.spec?.selector && ( | ||
| <MetadataDictGrid | ||
| dict={item.spec?.selector.matchLabels as Record<string, string>} |
There was a problem hiding this comment.
LabelSelector.matchLabels is optional, but MetadataDictGrid is given item.spec?.selector.matchLabels directly. If matchLabels is undefined, this may crash depending on MetadataDictGrid implementation. Pass a default empty object (or conditionally render only when matchLabels exists).
| dict={item.spec?.selector.matchLabels as Record<string, string>} | |
| dict={(item.spec?.selector.matchLabels ?? {}) as Record<string, string>} |
| { | ||
| name: 'Clusters ', // Use a trailing space because "Clusters" is taken | ||
| kind: 'Cluster', | ||
| path: 'capiclusters', | ||
| DetailComponent: ClusterDetail, | ||
| ListComponent: ClustersList, | ||
| icon: 'mdi:cloud', | ||
| }, |
There was a problem hiding this comment.
The name field has a trailing space to work around a collision, but it also becomes the sidebar identifier and label, which can leak into UI and make route sidebar matching brittle. Prefer a unique internal name (e.g. capi-clusters) while keeping label: 'Clusters'.
Summary
Replicasfield — both a raw string (status/spec) andrenderReplicas()were rendered. Keep only the structured version.Linknavigating to the parent Cluster detail page.Link(was plain textgetValue).Context
Builds on PR #273 (Cluster API plugin).
Related to #485
Test plan
npm run tscpassesnpm run lintpasses with zero warningsnpm run buildsucceeds