feat(topology): add pod log drawer and connect to Pod topology nodes#552
feat(topology): add pod log drawer and connect to Pod topology nodes#552SunsetB612 wants to merge 5 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
karmada-bot
added
the
size/XXL
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a resource topology visualization feature, allowing users to view the relationships between resource templates, bindings, and member cluster workloads down to the pod level. Backend changes include logic to trace ownership chains and evaluate pod health, while the frontend introduces a new Topology page using @xyflow/react, a pod log viewer, and a refactored workload detail drawer. Feedback identifies a critical performance bottleneck where a mutex is held during synchronous network requests, a suggestion to optimize pod listing via label selectors, and several instances of hardcoded strings that require internationalization.
| mu.Lock() | ||
| defer mu.Unlock() |
There was a problem hiding this comment.
Holding the mu lock for the entire duration of the goroutine via defer is a significant performance bottleneck. This lock is held while getPodsByWorkUID (line 450) is called, which performs multiple synchronous network requests to member clusters. This effectively serializes the processing of all clusters, negating the benefits of the errgroup.
Recommendation: Acquire the lock only when accessing or modifying the shared resp object, and release it immediately after.
| return nil, err | ||
| } | ||
| ownerUIDs := getPodDirectOwnerUIDs(ctx, memberClient, namespace, kind, workloadUID) | ||
| podList, err := memberClient.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{}) |
There was a problem hiding this comment.
Listing all pods in a namespace and filtering them client-side is inefficient, especially in namespaces with many pods. If the workload's label selector is available, it should be passed to the List call via metav1.ListOptions{LabelSelector: ...} to reduce API server load and network traffic.
| 自动刷新: | ||
| <Switch size="small" checked={logAutoRefresh} onChange={setLogAutoRefresh} /> | ||
| </span> | ||
| <span className="flex items-center gap-2"> | ||
| 自动滚动到底部: | ||
| <Switch size="small" checked={logAutoScroll} onChange={setLogAutoScroll} /> |
There was a problem hiding this comment.
Hardcoded Chinese strings found: "自动刷新" and "自动滚动到底部". These should be moved to the i18n locale files to support internationalization.
| <Select | ||
| placeholder="Namespace" | ||
| options={nsOptions} | ||
| loading={isNsDataLoading} | ||
| showSearch | ||
| allowClear | ||
| className="min-w-[180px]" | ||
| value={namespace || undefined} | ||
| onChange={(v) => { | ||
| setNamespace(v || ''); | ||
| setName(''); | ||
| }} | ||
| /> | ||
| <Select | ||
| placeholder="Kind" | ||
| options={kindOptions} | ||
| className="min-w-[150px]" | ||
| value={kind} | ||
| onChange={(v) => setKind(v)} | ||
| /> | ||
| <Select | ||
| placeholder="Resource Name" | ||
| className="min-w-[220px]" | ||
| showSearch | ||
| allowClear | ||
| value={name || undefined} | ||
| onChange={(v) => setName(v || '')} | ||
| options={[]} | ||
| mode={undefined} | ||
| searchValue={name} | ||
| onSearch={(v) => setName(v)} | ||
| notFoundContent={null} | ||
| /> | ||
| <Button type="primary" onClick={handleSearch}> | ||
| Query |
There was a problem hiding this comment.
UI strings such as "Namespace", "Kind", "Resource Name", and "Query" are hardcoded in English. These should be localized using the project's i18n system (e.g., i18nInstance.t('key', 'default')).
| 自动刷新: | ||
| <Switch size="small" checked={logAutoRefresh} onChange={setLogAutoRefresh} /> | ||
| </span> | ||
| <span className="flex items-center gap-2"> | ||
| 自动滚动到底部: | ||
| <Switch size="small" checked={logAutoScroll} onChange={setLogAutoScroll} /> |
There was a problem hiding this comment.
Hardcoded Chinese strings found: "自动刷新" and "自动滚动到底部". Please use i18nInstance.t for these labels to maintain consistency with the rest of the application's internationalization.
SunsetB612
force-pushed
the
feat/topology-pod-log
branch
from
d14dd93 to
cce6319
Compare
Signed-off-by: SunsetB612 <10235101575@stu.ecnu.edu.cn>
…ork nodes Signed-off-by: SunsetB612 <10235101575@stu.ecnu.edu.cn>
Signed-off-by: SunsetB612 <10235101575@stu.ecnu.edu.cn>
… graph Signed-off-by: SunsetB612 <10235101575@stu.ecnu.edu.cn>
Signed-off-by: SunsetB612 <10235101575@stu.ecnu.edu.cn>
SunsetB612
force-pushed
the
feat/topology-pod-log
branch
from
cce6319 to
fc99d5e
Compare
2 participants
No description provided.