Skip to content

Commit 011bc9b

Browse files
stefanonardoclaude
andcommitted
OCPBUGS-81630: Fix unnecessary error on Node Terminal tab
Show a loading spinner while the debug pod is being created, instead of briefly flashing "Debug pod not found or was deleted." The error occurred because useK8sWatchResource(null) returns loaded=true before pod creation completes. Also guard useEffect cleanup against undefined namespace to prevent TypeError when component unmounts before pod creation completes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 46a9c16 commit 011bc9b

2 files changed

Lines changed: 164 additions & 6 deletions

File tree

frontend/packages/console-app/src/components/nodes/NodeTerminal.tsx

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -244,16 +244,22 @@ const NodeTerminal: FC<NodeTerminalProps> = ({ obj: node }) => {
244244
createDebugPod();
245245
window.addEventListener('beforeunload', closeTab);
246246
return () => {
247-
deleteNamespace(namespace.metadata.name);
247+
if (namespace) {
248+
deleteNamespace(namespace.metadata.name);
249+
}
248250
window.removeEventListener('beforeunload', closeTab);
249251
};
250252
}, [nodeName, isWindows]);
251253

252-
return errorMessage ? (
253-
<NodeTerminalError error={errorMessage} />
254-
) : (
255-
<NodeTerminalInner pod={pod} loaded={loaded} loadError={loadError} />
256-
);
254+
if (errorMessage) {
255+
return <NodeTerminalError error={errorMessage} />;
256+
}
257+
258+
if (!podName) {
259+
return <LoadingBox />;
260+
}
261+
262+
return <NodeTerminalInner pod={pod} loaded={loaded} loadError={loadError} />;
257263
};
258264

259265
export default NodeTerminal;
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
import { render, screen, act } from '@testing-library/react';
2+
import { useK8sWatchResource } from '@console/internal/components/utils/k8s-watch-hook';
3+
import type { NodeKind, PodKind } from '@console/internal/module/k8s';
4+
import { k8sCreate, k8sGet, k8sKillByName } from '@console/internal/module/k8s';
5+
import NodeTerminal from '../NodeTerminal';
6+
7+
jest.mock('@console/internal/components/utils/k8s-watch-hook', () => ({
8+
useK8sWatchResource: jest.fn(),
9+
}));
10+
11+
jest.mock('@console/internal/components/pod', () => ({
12+
PodConnectLoader: jest.fn(() => 'PodConnectLoader'),
13+
}));
14+
15+
jest.mock('@console/internal/module/k8s', () => ({
16+
k8sCreate: jest.fn(),
17+
k8sGet: jest.fn(),
18+
k8sKillByName: jest.fn(),
19+
}));
20+
21+
const mockNode = {
22+
apiVersion: 'v1',
23+
kind: 'Node',
24+
metadata: { name: 'test-node', uid: 'test-uid' },
25+
status: { nodeInfo: { operatingSystem: 'linux' } },
26+
} as NodeKind;
27+
28+
const mockNamespace = { metadata: { name: 'openshift-debug-abc' } };
29+
30+
const mockPod = {
31+
apiVersion: 'v1',
32+
kind: 'Pod',
33+
metadata: { name: 'test-node-debug', namespace: 'openshift-debug-abc' },
34+
};
35+
36+
const setupPodCreation = () => {
37+
(k8sCreate as jest.Mock).mockResolvedValueOnce(mockNamespace).mockResolvedValueOnce(mockPod);
38+
(k8sGet as jest.Mock).mockRejectedValue(new Error('not found'));
39+
(k8sKillByName as jest.Mock).mockResolvedValue({});
40+
};
41+
42+
const renderAndCreatePod = async () => {
43+
jest.useFakeTimers();
44+
await act(async () => {
45+
render(<NodeTerminal obj={mockNode} />);
46+
});
47+
await act(async () => {
48+
jest.advanceTimersByTime(1100);
49+
});
50+
jest.useRealTimers();
51+
};
52+
53+
describe('NodeTerminal', () => {
54+
beforeEach(() => {
55+
jest.spyOn(console, 'warn').mockImplementation();
56+
jest.spyOn(console, 'error').mockImplementation();
57+
});
58+
59+
afterEach(() => {
60+
jest.restoreAllMocks();
61+
jest.useRealTimers();
62+
});
63+
64+
it('should show loading spinner while debug pod is being created', () => {
65+
(k8sCreate as jest.Mock).mockReturnValue(new Promise(() => {}));
66+
(k8sGet as jest.Mock).mockReturnValue(new Promise(() => {}));
67+
(useK8sWatchResource as jest.Mock).mockReturnValue([undefined, true, undefined]);
68+
69+
render(<NodeTerminal obj={mockNode} />);
70+
71+
expect(screen.getByRole('progressbar')).toBeInTheDocument();
72+
expect(screen.queryByText('Debug pod not found or was deleted.')).not.toBeInTheDocument();
73+
});
74+
75+
it('should show error when watch returns a load error', async () => {
76+
setupPodCreation();
77+
(useK8sWatchResource as jest.Mock).mockImplementation((resource) =>
78+
resource ? [{}, true, new Error('Connection refused')] : [undefined, true, undefined],
79+
);
80+
81+
await renderAndCreatePod();
82+
83+
expect(screen.getByText('Connection refused')).toBeVisible();
84+
});
85+
86+
it('should show loading when watch has not loaded yet', async () => {
87+
setupPodCreation();
88+
(useK8sWatchResource as jest.Mock).mockImplementation((resource) =>
89+
resource ? [{}, false, undefined] : [undefined, true, undefined],
90+
);
91+
92+
await renderAndCreatePod();
93+
94+
expect(screen.getByRole('progressbar')).toBeInTheDocument();
95+
});
96+
97+
it('should show not found error when pod is loaded but missing', async () => {
98+
setupPodCreation();
99+
(useK8sWatchResource as jest.Mock).mockImplementation((resource) =>
100+
resource ? [undefined, true, undefined] : [undefined, true, undefined],
101+
);
102+
103+
await renderAndCreatePod();
104+
105+
expect(screen.getByText('Debug pod not found or was deleted.')).toBeVisible();
106+
});
107+
108+
it('should show error with message when pod phase is Failed', async () => {
109+
const failedPod = ({
110+
...mockPod,
111+
status: { phase: 'Failed', message: 'ImagePullBackOff' },
112+
} as unknown) as PodKind;
113+
114+
setupPodCreation();
115+
(useK8sWatchResource as jest.Mock).mockImplementation((resource) =>
116+
resource ? [failedPod, true, undefined] : [undefined, true, undefined],
117+
);
118+
119+
await renderAndCreatePod();
120+
121+
expect(screen.getByText(/The debug pod failed.*ImagePullBackOff/)).toBeVisible();
122+
});
123+
124+
it('should render terminal when pod is Running', async () => {
125+
const runningPod = ({
126+
...mockPod,
127+
status: { phase: 'Running' },
128+
} as unknown) as PodKind;
129+
130+
setupPodCreation();
131+
(useK8sWatchResource as jest.Mock).mockImplementation((resource) =>
132+
resource ? [runningPod, true, undefined] : [undefined, true, undefined],
133+
);
134+
135+
await renderAndCreatePod();
136+
137+
expect(screen.getByText('PodConnectLoader')).toBeInTheDocument();
138+
});
139+
140+
it('should show error when pod creation fails', async () => {
141+
(k8sCreate as jest.Mock).mockRejectedValue(new Error('Forbidden'));
142+
(k8sGet as jest.Mock).mockRejectedValue(new Error('not found'));
143+
(k8sKillByName as jest.Mock).mockResolvedValue({});
144+
(useK8sWatchResource as jest.Mock).mockReturnValue([undefined, true, undefined]);
145+
146+
await act(async () => {
147+
render(<NodeTerminal obj={mockNode} />);
148+
});
149+
150+
expect(screen.getByText('Forbidden')).toBeVisible();
151+
});
152+
});

0 commit comments

Comments
 (0)