Skip to content

Commit bb133c0

Browse files
mclarke47mclarke47
committed
refactor implementation
1 parent 976e418 commit bb133c0

File tree

2 files changed

+155
-45
lines changed

2 files changed

+155
-45
lines changed

‎src/top.ts‎

Lines changed: 49 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,9 @@ export async function topNodes(api: CoreV1Api): Promise<NodeStatus[]> {
8282
return result;
8383
}
8484

85-
// TODO describe what this method does
85+
// Returns the current pod CPU/Memory usage including the CPU/Memory usage of each container
8686
export async function topPods(api: CoreV1Api, metrics: Metrics, namespace?: string): Promise<PodStatus[]> {
87+
// Figure out which pod list endpoint to call
8788
const getPodList = async (): Promise<V1PodList> => {
8889
if (namespace) {
8990
return (await api.listNamespacedPod(namespace)).body;
@@ -105,57 +106,62 @@ export async function topPods(api: CoreV1Api, metrics: Metrics, namespace?: stri
105106
for (const pod of podList.items) {
106107
const podMetric = podMetricsMap.get(pod.metadata!.name!);
107108

108-
// TODO we are calculating the contianer usages twice
109-
// Once per pod, then below per container
110-
// calculate the pod usage in this method instead to avoid
111-
// duplicating work
112-
const cpuTotal = totalCPU(pod);
113-
const memTotal = totalMemory(pod);
114109
const containerStatuses: ContainerStatus[] = [];
115110
let currentPodCPU: number | bigint = 0;
116111
let currentPodMem: number | bigint = 0;
117-
118-
if (podMetric !== undefined) {
119-
// Create a map of container names to their resource spec
120-
// to make it easier to look up when we need it later
121-
const containerResourceStatuses = pod.spec!.containers.reduce((accum, next) => {
122-
const containerCpuTotal = totalCPUForContainer(next);
123-
const containerMemTotal = totalMemoryForContainer(next);
124-
accum.set(next.name, [containerCpuTotal, containerMemTotal]);
125-
return accum;
126-
}, new Map<string, [ResourceStatus, ResourceStatus]>());
127-
128-
podMetric.containers.forEach((containerMetrics: ContainerMetric) => {
112+
let podRequestsCPU: number | bigint = 0;
113+
let podLimitsCPU: number | bigint = 0;
114+
let podRequestsMem: number | bigint = 0;
115+
let podLimitsMem: number | bigint = 0;
116+
117+
pod.spec!.containers.forEach((container) => {
118+
// get the the container CPU/Memory container.resources.requests
119+
const containerCpuTotal = totalCPUForContainer(container);
120+
const containerMemTotal = totalMemoryForContainer(container);
121+
122+
// sum each container's CPU/Memory container.resources.requests
123+
// to get the pod's overall request limit
124+
podRequestsCPU = add(podRequestsCPU, containerCpuTotal.request);
125+
podLimitsCPU = add(podLimitsCPU, containerCpuTotal.limit);
126+
127+
podRequestsMem = add(podLimitsMem, containerMemTotal.request);
128+
podLimitsMem = add(podLimitsMem, containerMemTotal.limit);
129+
130+
// Find the container metrics by container.name
131+
// if both the pod and container metrics exist
132+
const containerMetrics =
133+
podMetric !== undefined
134+
? podMetric.containers.find((c) => c.name === container.name)
135+
: undefined;
136+
137+
// Store the current usage of each container
138+
// Sum each container to get the overall pod usage
139+
if (containerMetrics !== undefined) {
129140
const currentContainerCPUUsage = quantityToScalar(containerMetrics.usage.cpu);
130141
const currentContainerMemUsage = quantityToScalar(containerMetrics.usage.memory);
142+
131143
currentPodCPU = add(currentPodCPU, currentContainerCPUUsage);
132144
currentPodMem = add(currentPodMem, currentContainerMemUsage);
133145

134-
const containerResourceStatus = containerResourceStatuses.get(containerMetrics.name);
135-
136-
if (containerResourceStatus !== undefined) {
137-
const [containerCpuTotal, containerMemTotal] = containerResourceStatus;
138-
139-
const containerCpuUsage = new CurrentResourceUsage(
140-
currentContainerCPUUsage,
141-
containerCpuTotal.request,
142-
containerCpuTotal.limit,
143-
);
144-
const containerMemUsage = new CurrentResourceUsage(
145-
currentContainerMemUsage,
146-
containerMemTotal.request,
147-
containerMemTotal.limit,
148-
);
149-
150-
containerStatuses.push(
151-
new ContainerStatus(containerMetrics.name, containerCpuUsage, containerMemUsage),
152-
);
153-
}
154-
});
155-
}
146+
const containerCpuUsage = new CurrentResourceUsage(
147+
currentContainerCPUUsage,
148+
containerCpuTotal.request,
149+
containerCpuTotal.limit,
150+
);
151+
const containerMemUsage = new CurrentResourceUsage(
152+
currentContainerMemUsage,
153+
containerMemTotal.request,
154+
containerMemTotal.limit,
155+
);
156+
157+
containerStatuses.push(
158+
new ContainerStatus(containerMetrics.name, containerCpuUsage, containerMemUsage),
159+
);
160+
}
161+
});
156162

157-
const podCpuUsage = new CurrentResourceUsage(currentPodCPU, cpuTotal.request, cpuTotal.limit);
158-
const podMemUsage = new CurrentResourceUsage(currentPodMem, memTotal.request, memTotal.limit);
163+
const podCpuUsage = new CurrentResourceUsage(currentPodCPU, podRequestsCPU, podLimitsCPU);
164+
const podMemUsage = new CurrentResourceUsage(currentPodMem, podRequestsMem, podLimitsMem);
159165
result.push(new PodStatus(pod, podCpuUsage, podMemUsage, containerStatuses));
160166
}
161167
return result;

‎src/top_test.ts‎

Lines changed: 106 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,21 @@ const mockedPodMetrics: PodMetricsList = {
4848
],
4949
};
5050

51+
const bestEffortPodList: V1Pod[] = [
52+
{
53+
metadata: {
54+
name: 'dice-roller-7c76898b4d-shm9p',
55+
},
56+
spec: {
57+
containers: [
58+
{
59+
name: 'nginx',
60+
},
61+
],
62+
},
63+
},
64+
];
65+
5166
const podList: V1Pod[] = [
5267
{
5368
metadata: {
@@ -145,6 +160,17 @@ describe('Top', () => {
145160
podMetrics.done();
146161
pods.done();
147162
});
163+
it('should return use cluster scope when namespace empty string', async () => {
164+
const [topPodsFunc, scope] = systemUnderTest('');
165+
const podMetrics = scope.get('/apis/metrics.k8s.io/v1beta1/pods').reply(200, emptyPodMetrics);
166+
const pods = scope.get('/api/v1/pods').reply(200, {
167+
items: [],
168+
});
169+
const result = await topPodsFunc();
170+
expect(result).to.deep.equal([]);
171+
podMetrics.done();
172+
pods.done();
173+
});
148174
it('should return cluster wide pod metrics', async () => {
149175
const [topPodsFunc, scope] = systemUnderTest();
150176
const podMetrics = scope.get('/apis/metrics.k8s.io/v1beta1/pods').reply(200, mockedPodMetrics);
@@ -154,7 +180,6 @@ describe('Top', () => {
154180
const result = await topPodsFunc();
155181
expect(result.length).to.equal(2);
156182
expect(result[0].CPU).to.deep.equal({
157-
// TODO fix this
158183
CurrentUsage: 0.05,
159184
LimitTotal: 0.1,
160185
RequestTotal: 0.1,
@@ -180,7 +205,6 @@ describe('Top', () => {
180205
},
181206
]);
182207
expect(result[1].CPU).to.deep.equal({
183-
// TODO fix this
184208
CurrentUsage: 1.4,
185209
LimitTotal: 2.1,
186210
RequestTotal: 2.1,
@@ -221,6 +245,86 @@ describe('Top', () => {
221245
podMetrics.done();
222246
pods.done();
223247
});
248+
it('should return best effort pod metrics', async () => {
249+
const [topPodsFunc, scope] = systemUnderTest();
250+
const podMetrics = scope.get('/apis/metrics.k8s.io/v1beta1/pods').reply(200, mockedPodMetrics);
251+
const pods = scope.get('/api/v1/pods').reply(200, {
252+
items: bestEffortPodList,
253+
});
254+
const result = await topPodsFunc();
255+
expect(result.length).to.equal(1);
256+
expect(result[0].CPU).to.deep.equal({
257+
CurrentUsage: 0.05,
258+
LimitTotal: 0,
259+
RequestTotal: 0,
260+
});
261+
expect(result[0].Memory).to.deep.equal({
262+
CurrentUsage: BigInt('4005888'),
263+
RequestTotal: 0,
264+
LimitTotal: 0,
265+
});
266+
expect(result[0].Containers).to.deep.equal([
267+
{
268+
CPUUsage: {
269+
CurrentUsage: 0.05,
270+
LimitTotal: 0,
271+
RequestTotal: 0,
272+
},
273+
Container: 'nginx',
274+
MemoryUsage: {
275+
CurrentUsage: BigInt('4005888'),
276+
LimitTotal: 0,
277+
RequestTotal: 0,
278+
},
279+
},
280+
]);
281+
podMetrics.done();
282+
pods.done();
283+
});
284+
it('should return 0 when pod metrics missing', async () => {
285+
const [topPodsFunc, scope] = systemUnderTest();
286+
const podMetrics = scope.get('/apis/metrics.k8s.io/v1beta1/pods').reply(200, emptyPodMetrics);
287+
const pods = scope.get('/api/v1/pods').reply(200, {
288+
items: podList,
289+
});
290+
const result = await topPodsFunc();
291+
expect(result.length).to.equal(2);
292+
expect(result[0].CPU).to.deep.equal({
293+
CurrentUsage: 0,
294+
LimitTotal: 0.1,
295+
RequestTotal: 0.1,
296+
});
297+
expect(result[0].Memory).to.deep.equal({
298+
CurrentUsage: 0,
299+
RequestTotal: BigInt('104857600'),
300+
LimitTotal: BigInt('104857600'),
301+
});
302+
expect(result[0].Containers).to.deep.equal([]);
303+
expect(result[1].CPU).to.deep.equal({
304+
CurrentUsage: 0,
305+
LimitTotal: 2.1,
306+
RequestTotal: 2.1,
307+
});
308+
expect(result[1].Memory).to.deep.equal({
309+
CurrentUsage: 0,
310+
LimitTotal: BigInt('209715200'),
311+
RequestTotal: BigInt('157286400'),
312+
});
313+
expect(result[1].Containers).to.deep.equal([]);
314+
podMetrics.done();
315+
pods.done();
316+
});
317+
it('should return empty array when pods missing', async () => {
318+
const [topPodsFunc, scope] = systemUnderTest();
319+
const podMetrics = scope.get('/apis/metrics.k8s.io/v1beta1/pods').reply(200, mockedPodMetrics);
320+
const pods = scope.get('/api/v1/pods').reply(200, {
321+
items: [],
322+
});
323+
const result = await topPodsFunc();
324+
expect(result.length).to.equal(0);
325+
podMetrics.done();
326+
pods.done();
327+
});
224328
it('should return namespace pod metrics', async () => {
225329
const [topPodsFunc, scope] = systemUnderTest(TEST_NAMESPACE);
226330
const podMetrics = scope

0 commit comments

Comments
 (0)