mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-26 05:29:45 +00:00
Stop calling an LXC's Proxmox listing memory cache-inclusive (#3017)
Proxmox's `/cluster/resources` `mem` for a container is its cgroup usage with page cache already subtracted (`PVE::CGroup::get_memory_stat`: `memory.current - memory.stat file` on cgroup v2, `usage_in_bytes - total_cache` on v1). #3011 makes the backend read it as known pressure for containers. The frontend still listed `cluster-resources` as cache-inclusive for every guest, so the container drawer said the reading "may include cached memory the container can reuse" and appended agent setup advice. - `memoryObservation.ts` classifies `cluster-resources` by guest kind: cache-excluded for a container, cache-inclusive for a QEMU guest or a guest of unknown kind. The other Proxmox fallbacks are unchanged. - The canonical resource drawer (`resourceDetailDrawerMetricsHistoryModel.ts`) now passes the guest kind, so it qualifies the same reading the same way. A retained VM reading there also gets the VM wording the Workloads drawer already uses. - The frontend-primitives contract clause that said a container's Proxmox reading always carries the caveat is corrected. Tests: the new container cases in `memoryObservation.test.ts`, `GuestDrawerOverview.memory-cache.test.tsx`, `GuestRow.test.tsx` and `resourceDetailDrawerMetricsHistoryModel.branchcov0712.test.ts` fail against main's source (6 failures) and pass with the fix. Existing container-copy tests now use `status-mem`, which is still cache-inclusive. Browser pass: private mock backend built from main. Before the fix, LXC `auth-service-01` (`cluster-resources`) showed `Current · Proxmox (may include cache)` and an `About this reading` note. After the fix it shows `Current · Proxmox` with no note. A QEMU drawer keeps its caveat. Checked at 1440 and 390 px wide. Refs #2762
This commit is contained in:
@@ -243,8 +243,9 @@ An independent qualified memory reading is not paused by a disk deferral.
|
||||
Neither Last seen, backup completion nor running power state renews memory.
|
||||
Unannotated unrelated platforms and actual zero readings retain their meaning.
|
||||
A current reading from a cache-inclusive Proxmox source (`status-mem`,
|
||||
`status-freemem`, `cluster-resources`, `derived-total-minus-used`, the same set
|
||||
the backend marks `MayIncludeCache`) keeps its numbers, colour and thresholds;
|
||||
`status-freemem`, `cluster-resources`, `derived-total-minus-used`, the same
|
||||
sources and guest kinds the backend marks `MayIncludeCache`) keeps its numbers,
|
||||
colour and thresholds;
|
||||
the shared presentation adds `mayIncludeCache` and a `cacheNote` saying a high
|
||||
percentage alone does not show memory pressure and which fix can supply the
|
||||
guest's own figure. A VM row shows `May include cache` (`Cache?` in compact
|
||||
@@ -252,9 +253,13 @@ layouts) only while the guest's own memory meets its warning or critical
|
||||
threshold, the figure that colours the bar in host-share mode too, which is
|
||||
when a cache-inclusive figure reads as pressure; below that the caveat is the
|
||||
cell title, the chart's native hover title and assistive text, not a label on every
|
||||
agentless VM. A container, whose Proxmox reading is always the cgroup value,
|
||||
keeps the caveat in the bar tooltip and drawer instead of a label on every
|
||||
row. A retained or freshness-unknown reading keeps its own freshness label
|
||||
agentless VM. A container's `cluster-resources` reading is not
|
||||
cache-inclusive: Proxmox subtracts page cache from the cgroup usage
|
||||
(`PVE::CGroup::get_memory_stat`) before it reaches `/cluster/resources`, so
|
||||
`memoryObservation.ts` classifies that source by guest kind and neither row,
|
||||
tooltip nor drawer qualifies it. A QEMU guest, or a guest of unknown kind,
|
||||
keeps the caveat for it. Any other cache-inclusive container reading keeps the
|
||||
caveat in the bar tooltip and drawer instead of a label on every row. A retained or freshness-unknown reading keeps its own freshness label
|
||||
with the caveat in its explanation, an unavailable reading is never qualified,
|
||||
and the drawer states the note as visible text in `About this reading`. An
|
||||
unavailable explanation names only what is known: a powered-off guest reads
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
{
|
||||
"version": 1,
|
||||
"base_sha": "6aef85ac88e3fcb812388c52fa92842a6ef278f5",
|
||||
"verified_at": "2026-10-16T23:27:30Z",
|
||||
"result": "passed",
|
||||
"changed_paths": [
|
||||
"frontend-modern/src/components/Infrastructure/resourceDetailDrawerMetricsHistoryModel.ts",
|
||||
"frontend-modern/src/utils/memoryObservation.ts"
|
||||
],
|
||||
"content_sha256": {
|
||||
"frontend-modern/src/components/Infrastructure/resourceDetailDrawerMetricsHistoryModel.ts": "6d3917fdd33adf43199becfcb2fc0e32e14c3d282bebefaf3566e586b6feb83d",
|
||||
"frontend-modern/src/utils/memoryObservation.ts": "692666d2a7dbee34ecabde32aac5c4ad226282f96e95feb210b1a69d87a14bdd"
|
||||
},
|
||||
"routes": [
|
||||
"/proxmox"
|
||||
],
|
||||
"viewports": [
|
||||
{
|
||||
"width": 1440,
|
||||
"height": 900
|
||||
},
|
||||
{
|
||||
"width": 390,
|
||||
"height": 844
|
||||
}
|
||||
],
|
||||
"states": [
|
||||
"private mock backend built from origin/main; LXC auth-service-01 with a current cluster-resources memory observation",
|
||||
"before (main's memoryObservation.ts): container drawer Details read 'Current · Proxmox (may include cache)' and showed 'About this reading' saying cached memory the container can reuse",
|
||||
"after: container drawer Details read 'Current · Proxmox · <time>' with no cache caveat and no About this reading row",
|
||||
"after: QEMU VM checkout-web-01 drawer still reads 'Proxmox (may include cache)' with the VM cache note",
|
||||
"after at 390px: LXC row memory cell carries no cache cue, drawer reading has no caveat, no horizontal page scroll"
|
||||
],
|
||||
"interactions": [
|
||||
"opened the LXC row drawer and expanded Details",
|
||||
"opened a VM row drawer and expanded Details",
|
||||
"swapped main's memoryObservation.ts in via Vite HMR for the before view, then restored the fix and reloaded"
|
||||
]
|
||||
}
|
||||
+12
@@ -67,6 +67,18 @@ describe('canonical drawer History honours selected guest observation provenance
|
||||
},
|
||||
);
|
||||
|
||||
it('qualifies a retained Proxmox listing reading by the guest kind', () => {
|
||||
const listing = (type: 'vm' | 'system-container') => {
|
||||
const value = { ...resource('last-known'), type };
|
||||
value.memory!.observation!.source = 'cluster-resources';
|
||||
return getResourceMetricsHistoryDeferredMetrics(value).memory.message;
|
||||
};
|
||||
// A QEMU listing counts the guest's cache; a container's has it removed.
|
||||
expect(listing('vm')).toContain('cached memory the VM can reuse');
|
||||
expect(listing('system-container')).not.toContain('cached memory');
|
||||
expect(listing('system-container')).toContain('Source: Proxmox.');
|
||||
});
|
||||
|
||||
it.each([-1, 101, NaN, Infinity])('withdraws invalid retained memory %s', (value) => {
|
||||
expect(
|
||||
getResourceMetricsHistoryDeferredMetrics(resource('last-known', value)).memory.lastKnownValue,
|
||||
|
||||
+10
@@ -166,11 +166,21 @@ const isProxmoxGuest = (resource: Resource): boolean =>
|
||||
(resource.platformType === 'proxmox-pve' ||
|
||||
resource.platformScopes?.includes('proxmox-pve') === true);
|
||||
|
||||
// The guest kind decides whether a Proxmox listing reading counts cache, the
|
||||
// same distinction the Workloads drawer makes.
|
||||
const memoryReading = (resource: Resource) =>
|
||||
getMemoryObservationPresentation(
|
||||
resource.memory ? finiteMetric(getMemoryPercent(resource)) : undefined,
|
||||
resource.memory?.observation,
|
||||
isProxmoxGuest(resource),
|
||||
{
|
||||
guestKind:
|
||||
resource.type === 'vm'
|
||||
? 'vm'
|
||||
: resource.type === 'system-container'
|
||||
? 'container'
|
||||
: undefined,
|
||||
},
|
||||
);
|
||||
|
||||
const guestDiskReading = (resource: Resource) => {
|
||||
|
||||
+14
-2
@@ -91,8 +91,12 @@ describe('guest drawer cache-inclusive memory note', () => {
|
||||
setValue(guest('agent'));
|
||||
expect(screen.queryByText('About this reading')).not.toBeInTheDocument();
|
||||
expect(screen.getByText('Memory reading').closest('tr')).not.toHaveTextContent('cache');
|
||||
setValue(guest('cluster-resources', { type: 'lxc' }));
|
||||
setValue(guest('status-mem', { type: 'lxc' }));
|
||||
expect(screen.getByText('About this reading').closest('tr')).toHaveTextContent('container');
|
||||
// The container's own listing value already excludes page cache.
|
||||
setValue(guest('cluster-resources', { type: 'lxc' }));
|
||||
expect(screen.queryByText('About this reading')).not.toBeInTheDocument();
|
||||
expect(screen.getByText('Memory reading').closest('tr')).not.toHaveTextContent('cache');
|
||||
});
|
||||
|
||||
it('keeps the caveat on a retained reading', () => {
|
||||
@@ -108,10 +112,18 @@ describe('guest drawer cache-inclusive memory note', () => {
|
||||
expect(screen.getByText('About this reading')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('gives a container the Pulse agent only', () => {
|
||||
it('does not call a container listing reading cache-inclusive', () => {
|
||||
render(() => (
|
||||
<GuestDrawerOverview {...overviewProps(guest('cluster-resources', { type: 'lxc' }))} />
|
||||
));
|
||||
expect(screen.getByText('Memory reading').closest('tr')).toHaveTextContent('Current · Proxmox');
|
||||
expect(screen.getByText('Memory reading').closest('tr')).not.toHaveTextContent('cache');
|
||||
expect(screen.queryByText('About this reading')).not.toBeInTheDocument();
|
||||
expect(screen.queryByText(/may include cache/)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('gives a container the Pulse agent only', () => {
|
||||
render(() => <GuestDrawerOverview {...overviewProps(guest('status-mem', { type: 'lxc' }))} />);
|
||||
const note = screen.getByText('About this reading').closest('tr')!;
|
||||
expect(note).toHaveTextContent('Pulse agent in the container');
|
||||
expect(note).not.toHaveTextContent('QEMU');
|
||||
|
||||
@@ -1487,7 +1487,7 @@ describe('GuestRow', () => {
|
||||
guest: makeGuest({
|
||||
type: 'lxc',
|
||||
memory: {
|
||||
...observation('current', observedAt, 'cluster-resources'),
|
||||
...observation('current', observedAt, 'status-mem'),
|
||||
used: 0.9 * 4294967296,
|
||||
usage: 90,
|
||||
},
|
||||
@@ -1553,7 +1553,7 @@ describe('GuestRow', () => {
|
||||
const { container } = renderGuestRow({
|
||||
guest: makeGuest({
|
||||
type: 'lxc',
|
||||
memory: observation('current', observedAt, 'cluster-resources'),
|
||||
memory: observation('current', observedAt, 'status-mem'),
|
||||
}),
|
||||
metricDisplayMode: mode,
|
||||
visibleColumnIds: ['name', 'memory'],
|
||||
@@ -1568,6 +1568,28 @@ describe('GuestRow', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it(`reads a container's own ${mode} listing value as cache-excluded`, () => {
|
||||
const { container } = renderGuestRow({
|
||||
guest: makeGuest({
|
||||
type: 'lxc',
|
||||
memory: {
|
||||
...observation('current', observedAt, 'cluster-resources'),
|
||||
used: 0.9 * 4294967296,
|
||||
usage: 90,
|
||||
},
|
||||
}),
|
||||
metricDisplayMode: mode,
|
||||
visibleColumnIds: ['name', 'memory'],
|
||||
});
|
||||
expect(container.querySelector(noticeSelector)).toBeNull();
|
||||
expect(container.querySelector('[data-workload-col="memory"]')).not.toHaveTextContent(
|
||||
/cache/i,
|
||||
);
|
||||
if (mode === 'bars') {
|
||||
expect(screen.getByTestId('memory-bar')).not.toHaveAttribute('data-reading-message');
|
||||
}
|
||||
});
|
||||
|
||||
it(`adds no cache cue to a cache-aware ${mode} reading`, () => {
|
||||
const { container } = renderGuestRow({
|
||||
guest: makeGuest({
|
||||
|
||||
@@ -122,7 +122,8 @@ describe('cache-inclusive Proxmox memory sources', () => {
|
||||
(source) => {
|
||||
const precaution =
|
||||
'Defer agent setup, restarts and live guest-agent probes during backups or a guest incident.';
|
||||
for (const type of ['qemu', 'lxc']) {
|
||||
// A container's listing reading is cache-excluded, so it carries no note.
|
||||
for (const type of source === 'cluster-resources' ? ['qemu'] : ['qemu', 'lxc']) {
|
||||
for (const state of ['current', 'last-known', 'future-state']) {
|
||||
const input = reading(source, state, { type });
|
||||
const original = structuredClone(input);
|
||||
@@ -160,6 +161,42 @@ describe('cache-inclusive Proxmox memory sources', () => {
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ type: 'lxc' },
|
||||
{ type: 'system-container', workloadType: 'system-container' as const },
|
||||
])('reads a container listing value as cache-excluded (%o)', (changes) => {
|
||||
// Proxmox subtracts page cache from an LXC's cgroup usage before it
|
||||
// reaches /cluster/resources, unlike a QEMU guest's listing mem.
|
||||
for (const state of ['current', 'last-known']) {
|
||||
const presentation = getWorkloadMemoryObservationPresentation(
|
||||
reading('cluster-resources', state, changes),
|
||||
)!;
|
||||
expect(presentation.mayIncludeCache).toBe(false);
|
||||
expect(presentation.cacheNote).toBeUndefined();
|
||||
expect(presentation.summary).not.toContain('may include cache');
|
||||
expect(presentation.message).not.toContain('cached memory');
|
||||
expect(presentation.summary).toContain('Proxmox');
|
||||
}
|
||||
expect(
|
||||
getWorkloadMemoryObservationPresentation(reading('cluster-resources', 'current', changes), {
|
||||
includeCurrent: false,
|
||||
}),
|
||||
).toBeNull();
|
||||
// Other Proxmox fallbacks keep their caveat for a container.
|
||||
expect(
|
||||
getWorkloadMemoryObservationPresentation(reading('status-mem', 'current', changes))
|
||||
?.mayIncludeCache,
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('keeps a listing value of unknown guest kind cache-inclusive', () => {
|
||||
expect(
|
||||
getWorkloadMemoryObservationPresentation(
|
||||
reading('cluster-resources', 'current', { type: 'docker' }),
|
||||
)?.mayIncludeCache,
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('names the fix that fits the guest: the QEMU agent is a VM-only, Linux-only option', () => {
|
||||
const vm = getWorkloadMemoryObservationPresentation(reading('status-mem'))!;
|
||||
expect(vm.cacheNote).toContain('memory the VM can reuse');
|
||||
@@ -177,7 +214,7 @@ describe('cache-inclusive Proxmox memory sources', () => {
|
||||
{ type: 'system-container', workloadType: 'system-container' as const },
|
||||
]) {
|
||||
const note = getWorkloadMemoryObservationPresentation(
|
||||
reading('cluster-resources', 'current', changes),
|
||||
reading('status-mem', 'current', changes),
|
||||
)!.cacheNote;
|
||||
expect(note).toContain('memory the container can reuse');
|
||||
expect(note).toContain('Pulse agent in the container');
|
||||
|
||||
@@ -28,7 +28,8 @@ export interface MemoryObservationPresentation {
|
||||
|
||||
// Proxmox's own listing and status values. Pulse selects them only when no
|
||||
// cache-aware source (guest agent, linked Pulse agent, node meminfo) answered;
|
||||
// the same four sources set MayIncludeCache in unifiedresources.QualifyGuestMemory.
|
||||
// unifiedresources.QualifyGuestMemoryForKind sets MayIncludeCache for the same
|
||||
// sources and guest kinds.
|
||||
const cacheInclusiveMemorySources = new Set([
|
||||
'status-mem',
|
||||
'status-freemem',
|
||||
@@ -38,6 +39,14 @@ const cacheInclusiveMemorySources = new Set([
|
||||
|
||||
export type MemoryGuestKind = 'vm' | 'container';
|
||||
|
||||
// A container's cluster-resources mem is its cgroup usage with page cache
|
||||
// already subtracted (PVE::CGroup::get_memory_stat), so it is not
|
||||
// cache-inclusive. A QEMU listing, or one whose guest kind is unknown, is the
|
||||
// host's view of guest RAM and counts the guest's own cache.
|
||||
const isCacheInclusiveMemorySource = (source: string, kind: MemoryGuestKind | undefined): boolean =>
|
||||
cacheInclusiveMemorySources.has(source) &&
|
||||
!(source === 'cluster-resources' && kind === 'container');
|
||||
|
||||
const getCacheNote = (kind: MemoryGuestKind | undefined): string => {
|
||||
const noun = kind === 'vm' ? 'VM' : kind === 'container' ? 'container' : 'guest';
|
||||
const caveat = `Proxmox's reading may include cached memory the ${noun} can reuse, so a high percentage alone does not mean it is short of memory.`;
|
||||
@@ -153,7 +162,7 @@ export const getMemoryObservationPresentation = (
|
||||
const mayIncludeCache =
|
||||
state !== 'unavailable' &&
|
||||
observation !== undefined &&
|
||||
cacheInclusiveMemorySources.has(observation.source);
|
||||
isCacheInclusiveMemorySource(observation.source, options.guestKind);
|
||||
// Most table rows are current and need no cue. Qualify them identically,
|
||||
// but avoid building source/date strings that will never be rendered. A
|
||||
// current reading from a cache-inclusive source still carries its caveat.
|
||||
|
||||
Reference in New Issue
Block a user