From 863a7f5620f3c907b86c1f2dd2c46c142bc19f35 Mon Sep 17 00:00:00 2001 From: John Swanke Date: Mon, 24 Aug 2026 13:50:32 -0400 Subject: [PATCH 01/12] prevent flapping Signed-off-by: John Swanke --- backend/src/lib/server-side-events.ts | 6 +- backend/src/routes/events.ts | 190 +++++++++++++++++++++++++- backend/test/routes/events.test.ts | 114 ++++++++++++++++ frontend/src/atoms.ts | 10 +- frontend/src/components/LoadData.tsx | 44 +++++- 5 files changed, 354 insertions(+), 10 deletions(-) diff --git a/backend/src/lib/server-side-events.ts b/backend/src/lib/server-side-events.ts index 73840733644..0f8a2a90146 100644 --- a/backend/src/lib/server-side-events.ts +++ b/backend/src/lib/server-side-events.ts @@ -323,13 +323,15 @@ export class ServerSideEvents { parts.push(loaded) } - // remove START, SETTINGS and LOADED from events + // remove START, SETTINGS, FLAPPING and LOADED from events const start = parts.shift() const end = parts.pop() const inx = parts.findIndex(({ data }) => { return (data as { type?: 'SETTINGS' }).type === 'SETTINGS' }) const settings = parts.splice(inx, 1)[0] + const flappingEvents = parts.filter(({ data }) => (data as { type?: string }).type === 'FLAPPING') + parts = parts.filter(({ data }) => (data as { type?: string }).type !== 'FLAPPING') // separate resource by kind // we want to send the resources that populate the main console pages first @@ -397,7 +399,7 @@ export class ServerSideEvents { // send packets of resources // with resources that fill main console pages first let sentCount = 0 - const sending = [start, settings] + const sending = [start, settings, ...flappingEvents] do { sending.push(...clusters.splice(0, 200)) sending.push(...agents.splice(0, 200)) diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index 6071e9e985f..49dd96276be 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -35,10 +35,179 @@ export interface SettingsEvent { settings: Record } -type ServerSideEventData = WatchEvent | SettingsEvent | { type: 'START' | 'LOADED' } +export interface FlappingEvent { + type: 'FLAPPING' + message: string + kind: string + namespace: string + name: string +} + +type ServerSideEventData = WatchEvent | SettingsEvent | FlappingEvent | { type: 'START' | 'LOADED' | 'EOP' } let requests: { cancel: () => void }[] = [] +/** + * Flapping resource throttling + * + * When a watched kube resource updates erroneously at high frequency (especially Policies), + * unbounded cacheResource → ServerSideEvents.pushEvent calls can: + * 1) Overwhelm the event processing loop / liveliness probe + * 2) Grow client event queues until the pod OOMs + * + * Detection: if the same kind/namespace/name is modified more than FLAP_THRESHOLD (N) times + * within FLAP_WINDOW_MS (M), the resource is considered flapping. + * While flapping, browser broadcasts are limited to once every FLAP_COOLDOWN_MS (P). + * Only kinds listed in FLAP_THROTTLE_KINDS are subject to this check. + * + * N is initially 5 times + * M is initially 5 seconds + * P is initially 1 minute + * Kind is initially ['Policy'] + */ +export const FLAP_THRESHOLD = 5 // N: modifications that trigger flapping detection +export const FLAP_WINDOW_MS = 5 * 1000 // M: sliding window for counting modifications +export const FLAP_COOLDOWN_MS = 60 * 1000 // P: min interval between browser updates while flapping +export const FLAP_THROTTLE_KINDS = ['Policy'] // kinds subject to flapping detection + +interface FlapTrackerEntry { + timestamps: number[] + throttled: boolean + lastForwardedAt: number + flappingEventID?: number + kind: string + namespace: string + name: string +} + +const flapTracker: Record = {} + +/** Clear flap tracker state. Used for test isolation. */ +export function resetFlapTracker() { + for (const key in flapTracker) { + const entry = flapTracker[key] + if (entry.flappingEventID) { + ServerSideEvents.removeEvent(entry.flappingEventID) + } + delete flapTracker[key] + } +} + +export function getFlapTracker() { + return flapTracker +} + +export function resourceFlapKey( + resource: Pick & { metadata?: { namespace?: string; name?: string } } +) { + return `${resource.kind}/${resource.metadata?.namespace ?? ''}/${resource.metadata?.name ?? ''}` +} + +export function formatFlappingMessage(kind: string, namespace: string, name: string): string { + const timesPerMinute = Math.max(1, Math.round(60_000 / FLAP_COOLDOWN_MS)) + return `${kind} ${namespace} ${name} is flapping, verify this resource is configured correctly. Until corrected, this resource will not update in the UI more then ${timesPerMinute} times per minute` +} + +async function notifyFlapping(entry: FlapTrackerEntry): Promise { + const message = formatFlappingMessage(entry.kind, entry.namespace, entry.name) + logger.warn({ msg: message, kind: entry.kind, namespace: entry.namespace, name: entry.name }) + if (entry.flappingEventID) { + ServerSideEvents.removeEvent(entry.flappingEventID) + } + entry.flappingEventID = await ServerSideEvents.pushEvent({ + data: { + type: 'FLAPPING', + message, + kind: entry.kind, + namespace: entry.namespace, + name: entry.name, + } satisfies FlappingEvent, + }) +} + +function clearFlappingNotice(entry: FlapTrackerEntry): void { + if (entry.flappingEventID) { + ServerSideEvents.removeEvent(entry.flappingEventID) + entry.flappingEventID = undefined + } +} + +/** + * Records a modification for flap detection and returns whether this update should be + * forwarded to browser clients. Non-throttled kinds always return true. + */ +export function shouldForwardResourceUpdate( + resource: Pick & { metadata?: { namespace?: string; name?: string } }, + now = Date.now() +): boolean { + if (!FLAP_THROTTLE_KINDS.includes(resource.kind)) { + return true + } + + const key = resourceFlapKey(resource) + const kind = resource.kind + const namespace = resource.metadata?.namespace ?? '' + const name = resource.metadata?.name ?? '' + + let entry = flapTracker[key] + if (!entry) { + entry = { timestamps: [], throttled: false, lastForwardedAt: 0, kind, namespace, name } + flapTracker[key] = entry + } + + entry.timestamps.push(now) + entry.timestamps = entry.timestamps.filter((t) => now - t <= FLAP_WINDOW_MS) + + const wasThrottled = entry.throttled + entry.throttled = entry.timestamps.length > FLAP_THRESHOLD + + if (entry.throttled && !wasThrottled) { + void notifyFlapping(entry) + } else if (!entry.throttled && wasThrottled) { + clearFlappingNotice(entry) + } + + if (!entry.throttled) { + entry.lastForwardedAt = now + return true + } + + // While flapping: allow at most one browser update every FLAP_COOLDOWN_MS + if (entry.lastForwardedAt === 0 || now - entry.lastForwardedAt >= FLAP_COOLDOWN_MS) { + entry.lastForwardedAt = now + return true + } + return false +} + +/** + * When TEST_THROTTLING=true, synthesize rapid Policy updates so flapping throttle can be verified + * without a misconfigured cluster resource. + */ +function startTestThrottling(): void { + if (process.env.TEST_THROTTLING !== 'true') return + + logger.warn({ msg: 'TEST_THROTTLING enabled — synthesizing flapping Policy updates' }) + let revision = 0 + const interval = setInterval(() => { + revision += 1 + const resource: IResource = { + kind: 'Policy', + apiVersion: 'policy.open-cluster-management.io/v1', + metadata: { + name: 'test-flapping-policy', + namespace: 'default', + uid: 'test-flapping-policy-uid', + resourceVersion: String(revision), + }, + } + void cacheResource(resource, true).catch((err: unknown) => { + logger.error({ msg: 'TEST_THROTTLING cacheResource failed', error: errorToString(err) }) + }) + }, 200) + interval.unref() +} + export async function getKubeResources(kind: string, apiVersion: string) { const option = { apiVersion, kind } const apiVersionPlural = apiVersionPluralFn(option) @@ -344,6 +513,7 @@ const definitions: IWatchOptions[] = [ export function startWatching(): void { ServerSideEvents.eventFilter = eventFilter startAccessCacheCleanup() + startTestThrottling() for (const definition of definitions) { void listAndWatch(definition) @@ -814,21 +984,28 @@ export async function cacheResource(resource: IResource, forwardEventsToClients ) { return resource.metadata.resourceVersion } - const eventID = await existing.eventID const latestExisting = cache[uid] if (latestExisting === existing) { - // if no other cacheResource call updated the cache while we were awaiting, we can replace the cache entry and event - if (eventID > 0) ServerSideEvents.removeEvent(eventID) + // Decide whether to replace the broadcast event after flapping throttle check below break } // if a deleteResource ran while we were awaiting, we will exit the loop because the resource is no longer existing // if another cacheResource call updated the cache while we were awaiting, we will check again if the resourceVersion is the same existing = latestExisting } + + // Always update the in-memory cache; only throttle browser broadcasts for flapping resources + const shouldForward = forwardEventsToClients && shouldForwardResourceUpdate(resource) + if (shouldForward && existing) { + const eventID = await existing.eventID + // if no other cacheResource call updated the cache while we were awaiting, we can replace the cache entry and event + if (cache[uid] === existing && eventID > 0) ServerSideEvents.removeEvent(eventID) + } + const compressed = deflateResource(resource, eventDict) - const eventID = forwardEventsToClients + const eventID = shouldForward ? compressed.then((compressed) => ServerSideEvents.pushEvent({ data: { type: 'MODIFIED', object: compressed } })) - : NO_BROADCAST_EVENT_ID + : (existing?.eventID ?? NO_BROADCAST_EVENT_ID) cache[uid] = { compressed, eventID } if (resource.kind === 'ManagedCluster') { @@ -894,6 +1071,7 @@ function eventFilter(token: string, serverSideEvent: ServerSideEvent { expect(cache[`token-${tokenCount - 1}`]).toBeUndefined() }) }) + + describe('flapping resource throttling', () => { + beforeEach(() => { + resetFlapTracker() + resetResourceCache() + ServerSideEvents.reset() + }) + + afterEach(() => { + resetFlapTracker() + resetResourceCache() + ServerSideEvents.reset() + }) + + it('should format the flapping warning message using cooldown rate', () => { + const timesPerMinute = Math.max(1, Math.round(60_000 / FLAP_COOLDOWN_MS)) + expect(formatFlappingMessage('Policy', 'default', 'policy-a')).toBe( + `Policy default policy-a is flapping, verify this resource is configured correctly. Until corrected, this resource will not update in the UI more then ${timesPerMinute} times per minute` + ) + }) + + it('should forward non-Policy kinds without throttling', () => { + const now = Date.now() + for (let i = 0; i < FLAP_THRESHOLD + 10; i++) { + expect( + shouldForwardResourceUpdate( + { kind: 'ManagedCluster', metadata: { name: 'cluster-a', namespace: '' } }, + now + i + ) + ).toBe(true) + } + }) + + it('should throttle Policy updates after more than N modifications within M seconds', () => { + const base = Date.now() + const policy = { kind: 'Policy', metadata: { name: 'flappy', namespace: 'default' } } + + let forwarded = 0 + // Sustained high-frequency updates across the cooldown window. + // lastForwardedAt is set near base + (FLAP_THRESHOLD-1)*100, so run past that + cooldown. + const end = base + (FLAP_THRESHOLD - 1) * 100 + FLAP_COOLDOWN_MS + 500 + for (let t = base; t <= end; t += 100) { + if (shouldForwardResourceUpdate(policy, t)) { + forwarded += 1 + } + } + + expect(getFlapTracker()['Policy/default/flappy'].throttled).toBe(true) + // First FLAP_THRESHOLD forwards, then at most one more after cooldown while still flapping + expect(forwarded).toBe(FLAP_THRESHOLD + 1) + }) + + it('should stop throttling when modifications fall back within the detection window', () => { + const base = Date.now() + const policy = { kind: 'Policy', metadata: { name: 'recovering', namespace: 'ns1' } } + + for (let i = 0; i <= FLAP_THRESHOLD; i++) { + shouldForwardResourceUpdate(policy, base + i) + } + expect(getFlapTracker()['Policy/ns1/recovering'].throttled).toBe(true) + + // Advance far enough that every prior timestamp falls outside the detection window + expect(shouldForwardResourceUpdate(policy, base + FLAP_WINDOW_MS + FLAP_THRESHOLD + 10)).toBe(true) + expect(getFlapTracker()['Policy/ns1/recovering'].throttled).toBe(false) + }) + + it('should keep updating cache while suppressing excess browser events for a flapping Policy', async () => { + const pushSpy = jest.spyOn(ServerSideEvents, 'pushEvent') + + for (let i = 1; i <= FLAP_THRESHOLD + 3; i++) { + await cacheResource({ + kind: 'Policy', + apiVersion: 'policy.open-cluster-management.io/v1', + metadata: { + name: 'flappy-policy', + namespace: 'default', + uid: 'flappy-uid', + resourceVersion: String(i), + }, + }) + } + + // Allow async FLAPPING notify + MODIFIED pushes to settle + await new Promise((resolve) => setTimeout(resolve, 0)) + for (const entry of Object.values(getEventCache())) { + await Promise.all(Object.values(entry).map((e) => e.eventID)) + } + + const modifiedPushes = pushSpy.mock.calls.filter( + (call) => (call[0].data as { type?: string })?.type === 'MODIFIED' + ) + const flappingPushes = pushSpy.mock.calls.filter( + (call) => (call[0].data as { type?: string })?.type === 'FLAPPING' + ) + + // First FLAP_THRESHOLD updates forward; subsequent ones in the window are suppressed + expect(modifiedPushes.length).toBe(FLAP_THRESHOLD) + expect(flappingPushes.length).toBe(1) + + const resources = await getKubeResources('Policy', 'policy.open-cluster-management.io/v1') + expect(resources).toHaveLength(1) + expect(resources[0].metadata.resourceVersion).toBe(String(FLAP_THRESHOLD + 3)) + + pushSpy.mockRestore() + }) + }) }) diff --git a/frontend/src/atoms.ts b/frontend/src/atoms.ts index e75042e001f..dcc81e611bd 100644 --- a/frontend/src/atoms.ts +++ b/frontend/src/atoms.ts @@ -203,7 +203,15 @@ export interface SettingsEvent { settings: Record } -export type ServerSideEventData = WatchEvent | SettingsEvent | { type: 'START' | 'LOADED' } +export interface FlappingEvent { + type: 'FLAPPING' + message: string + kind: string + namespace: string + name: string +} + +export type ServerSideEventData = WatchEvent | SettingsEvent | FlappingEvent | { type: 'START' | 'LOADED' | 'EOP' } export function usePolicies() { const policies = useRecoilValue(policiesState) diff --git a/frontend/src/components/LoadData.tsx b/frontend/src/components/LoadData.tsx index 6bf2b5fc982..76ee1f49960 100644 --- a/frontend/src/components/LoadData.tsx +++ b/frontend/src/components/LoadData.tsx @@ -1,6 +1,7 @@ /* Copyright Contributors to the Open Cluster Management project */ import get from 'lodash/get' import { Fragment, ReactNode, useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react' +import { Alert, AlertActionCloseButton, AlertGroup } from '@patternfly/react-core' // eslint-disable-next-line @typescript-eslint/no-restricted-imports import { SetterOrUpdater, useRecoilValue, useSetRecoilState } from 'recoil' import { tokenExpired } from '../logout' @@ -186,7 +187,9 @@ import { usersState, vmClusterRolesState, WatchEvent, + FlappingEvent, } from '../atoms' +import { useTranslation } from '../lib/acm-i18next' import { PluginDataContext } from '../lib/PluginDataContext' import { useQuery } from '../lib/useQuery' import { MultiClusterHubComponent } from '../resources/multi-cluster-hub-component' @@ -194,9 +197,11 @@ import { ClaimMappings } from '~/resources/authentication' import { usePageActivity } from '../lib/usePageActivity' export function LoadData(props: { children?: ReactNode }) { + const { t } = useTranslation() const { loadCompleted, setLoadStarted, setLoadCompleted, setIsStreamIdle, setIsReconnecting, mounted } = useContext(PluginDataContext) const [eventsLoaded, setEventsLoaded] = useState(false) + const [flappingAlerts, setFlappingAlerts] = useState([]) const idleTimeoutMs = useEventStreamIdleTimeout() const gracePeriodMs = useEventStreamIdleGracePeriod() const { isActive } = usePageActivity(idleTimeoutMs, mounted) @@ -565,6 +570,15 @@ export function LoadData(props: { children?: ReactNode }) { case 'SETTINGS': setSettings(data.settings) break + case 'FLAPPING': + setFlappingAlerts((alerts) => { + const key = `${data.kind}/${data.namespace}/${data.name}` + if (alerts.some((a) => `${a.kind}/${a.namespace}/${a.name}` === key)) { + return alerts.map((a) => (`${a.kind}/${a.namespace}/${a.name}` === key ? data : a)) + } + return [...alerts, data] + }) + break } } catch (err) { console.error(err) @@ -704,7 +718,35 @@ export function LoadData(props: { children?: ReactNode }) { const children = useMemo(() => {props.children}, [props.children]) - return children + return ( + + {flappingAlerts.length > 0 && ( + + {flappingAlerts.map((alert) => { + const key = `${alert.kind}/${alert.namespace}/${alert.name}` + return ( + { + setFlappingAlerts((alerts) => alerts.filter((a) => `${a.kind}/${a.namespace}/${a.name}` !== key)) + }} + /> + } + > + {alert.message} + + ) + })} + + )} + {children} + + ) } function resetCaches(caches: Record>>) { From fc325109f75e678815623d29d9222c2c46df92aa Mon Sep 17 00:00:00 2001 From: John Swanke Date: Mon, 24 Aug 2026 14:05:58 -0400 Subject: [PATCH 02/12] road rabbit Signed-off-by: John Swanke --- backend/src/routes/events.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index 49dd96276be..df24742f3be 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -185,7 +185,7 @@ export function shouldForwardResourceUpdate( * without a misconfigured cluster resource. */ function startTestThrottling(): void { - if (process.env.TEST_THROTTLING !== 'true') return + if (process.env.NODE_ENV !== 'production' && process.env.TEST_THROTTLING !== 'true') return logger.warn({ msg: 'TEST_THROTTLING enabled — synthesizing flapping Policy updates' }) let revision = 0 From 8534c2daf4beaa0441d75afb0c77b7b575db7dc3 Mon Sep 17 00:00:00 2001 From: John Swanke Date: Mon, 24 Aug 2026 14:18:35 -0400 Subject: [PATCH 03/12] fix check Signed-off-by: John Swanke --- frontend/public/locales/en/translation.json | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/public/locales/en/translation.json b/frontend/public/locales/en/translation.json index 2d2b44635c4..2ad25c0d9fe 100644 --- a/frontend/public/locales/en/translation.json +++ b/frontend/public/locales/en/translation.json @@ -2747,6 +2747,7 @@ "Resource name": "Resource name", "Resource nodes": "Resource nodes", "Resource type": "Resource type", + "Resource update throttled": "Resource update throttled", "resource.error": "Failed to load.", "resource.labels": "Labels", "resource.loading": "Loading...", From 899421f3713ab04597f1f6708aa8f891d1b07d97 Mon Sep 17 00:00:00 2001 From: John Swanke Date: Thu, 27 Aug 2026 14:06:33 -0400 Subject: [PATCH 04/12] fixes Signed-off-by: John Swanke --- backend/src/routes/events.ts | 16 +++++-- backend/test/routes/events.test.ts | 3 +- frontend/src/atoms.ts | 4 +- frontend/src/components/FlappingAlerts.tsx | 56 ++++++++++++++++++++++ frontend/src/components/LoadData.tsx | 48 +++++-------------- frontend/src/components/LoadPluginData.tsx | 2 + frontend/src/lib/PluginDataContext.tsx | 22 ++++++++- 7 files changed, 107 insertions(+), 44 deletions(-) create mode 100644 frontend/src/components/FlappingAlerts.tsx diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index df24742f3be..6d3456bbc33 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -37,10 +37,12 @@ export interface SettingsEvent { export interface FlappingEvent { type: 'FLAPPING' - message: string kind: string namespace: string name: string + threshold: number + windowMs: number + cooldownMs: number } type ServerSideEventData = WatchEvent | SettingsEvent | FlappingEvent | { type: 'START' | 'LOADED' | 'EOP' } @@ -104,8 +106,9 @@ export function resourceFlapKey( } export function formatFlappingMessage(kind: string, namespace: string, name: string): string { + const windowMinutes = Math.max(1, Math.round(FLAP_WINDOW_MS / 60_000)) const timesPerMinute = Math.max(1, Math.round(60_000 / FLAP_COOLDOWN_MS)) - return `${kind} ${namespace} ${name} is flapping, verify this resource is configured correctly. Until corrected, this resource will not update in the UI more then ${timesPerMinute} times per minute` + return `${kind} ${name} in namespace ${namespace} has been modified more than ${FLAP_THRESHOLD} times in the last ${windowMinutes} minutes. Verify this resource is configured correctly. Updates are being limited to ${timesPerMinute} times per minute.` } async function notifyFlapping(entry: FlapTrackerEntry): Promise { @@ -117,10 +120,12 @@ async function notifyFlapping(entry: FlapTrackerEntry): Promise { entry.flappingEventID = await ServerSideEvents.pushEvent({ data: { type: 'FLAPPING', - message, kind: entry.kind, namespace: entry.namespace, name: entry.name, + threshold: FLAP_THRESHOLD, + windowMs: FLAP_WINDOW_MS, + cooldownMs: FLAP_COOLDOWN_MS, } satisfies FlappingEvent, }) } @@ -191,7 +196,7 @@ function startTestThrottling(): void { let revision = 0 const interval = setInterval(() => { revision += 1 - const resource: IResource = { + const resource: IResource & { spec: { disabled: boolean } } = { kind: 'Policy', apiVersion: 'policy.open-cluster-management.io/v1', metadata: { @@ -200,6 +205,9 @@ function startTestThrottling(): void { uid: 'test-flapping-policy-uid', resourceVersion: String(revision), }, + spec: { + disabled: false, + }, } void cacheResource(resource, true).catch((err: unknown) => { logger.error({ msg: 'TEST_THROTTLING cacheResource failed', error: errorToString(err) }) diff --git a/backend/test/routes/events.test.ts b/backend/test/routes/events.test.ts index ec0e85bcc13..980e3e5f48b 100644 --- a/backend/test/routes/events.test.ts +++ b/backend/test/routes/events.test.ts @@ -1518,9 +1518,10 @@ describe('events Route', () => { }) it('should format the flapping warning message using cooldown rate', () => { + const windowMinutes = Math.max(1, Math.round(FLAP_WINDOW_MS / 60_000)) const timesPerMinute = Math.max(1, Math.round(60_000 / FLAP_COOLDOWN_MS)) expect(formatFlappingMessage('Policy', 'default', 'policy-a')).toBe( - `Policy default policy-a is flapping, verify this resource is configured correctly. Until corrected, this resource will not update in the UI more then ${timesPerMinute} times per minute` + `Policy policy-a in namespace default has been modified more than ${FLAP_THRESHOLD} times in the last ${windowMinutes} minutes. Verify this resource is configured correctly. Updates are being limited to ${timesPerMinute} times per minute.` ) }) diff --git a/frontend/src/atoms.ts b/frontend/src/atoms.ts index dcc81e611bd..79ae93e6b6d 100644 --- a/frontend/src/atoms.ts +++ b/frontend/src/atoms.ts @@ -205,10 +205,12 @@ export interface SettingsEvent { export interface FlappingEvent { type: 'FLAPPING' - message: string kind: string namespace: string name: string + threshold: number + windowMs: number + cooldownMs: number } export type ServerSideEventData = WatchEvent | SettingsEvent | FlappingEvent | { type: 'START' | 'LOADED' | 'EOP' } diff --git a/frontend/src/components/FlappingAlerts.tsx b/frontend/src/components/FlappingAlerts.tsx new file mode 100644 index 00000000000..a9e59e88643 --- /dev/null +++ b/frontend/src/components/FlappingAlerts.tsx @@ -0,0 +1,56 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import { useContext } from 'react' +import { Alert, AlertActionCloseButton, AlertGroup } from '@patternfly/react-core' +import { useTranslation } from '../lib/acm-i18next' +import { PluginContext } from '../lib/PluginContext' + +/** + * Renders warnings for resources that are being throttled due to flapping (see backend/src/routes/events.ts). + * + * This must be rendered inside actual ACM/MCE routed page content (e.g. via LoadPluginData), not inside + * PluginDataContextProvider/LoadData. That provider is registered as a global OpenShift Console + * `console.context-provider` extension, so anything it renders directly would appear on every console page, + * not just ACM/MCE pages. + */ +export function FlappingAlerts() { + const { t } = useTranslation() + const { dataContext } = useContext(PluginContext) + const { flappingAlerts, setFlappingAlerts } = useContext(dataContext) + + if (flappingAlerts.length === 0) return null + + return ( + + {flappingAlerts.map((alert) => { + const key = `${alert.kind}/${alert.namespace}/${alert.name}` + return ( + { + setFlappingAlerts((alerts) => alerts.filter((a) => `${a.kind}/${a.namespace}/${a.name}` !== key)) + }} + /> + } + > + {t( + '{{kind}} {{name}} in namespace {{namespace}} has been modified more than {{threshold}} times in the last {{windowMinutes}} minutes. Verify this resource is configured correctly. Updates are being limited to {{timesPerMinute}} times per minute.', + { + kind: alert.kind, + name: alert.name, + namespace: alert.namespace, + threshold: alert.threshold, + windowMinutes: Math.max(1, Math.round(alert.windowMs / 60_000)), + timesPerMinute: Math.max(1, Math.round(60_000 / alert.cooldownMs)), + } + )} + + ) + })} + + ) +} diff --git a/frontend/src/components/LoadData.tsx b/frontend/src/components/LoadData.tsx index 76ee1f49960..3b3a4f19179 100644 --- a/frontend/src/components/LoadData.tsx +++ b/frontend/src/components/LoadData.tsx @@ -1,7 +1,6 @@ /* Copyright Contributors to the Open Cluster Management project */ import get from 'lodash/get' import { Fragment, ReactNode, useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react' -import { Alert, AlertActionCloseButton, AlertGroup } from '@patternfly/react-core' // eslint-disable-next-line @typescript-eslint/no-restricted-imports import { SetterOrUpdater, useRecoilValue, useSetRecoilState } from 'recoil' import { tokenExpired } from '../logout' @@ -187,9 +186,7 @@ import { usersState, vmClusterRolesState, WatchEvent, - FlappingEvent, } from '../atoms' -import { useTranslation } from '../lib/acm-i18next' import { PluginDataContext } from '../lib/PluginDataContext' import { useQuery } from '../lib/useQuery' import { MultiClusterHubComponent } from '../resources/multi-cluster-hub-component' @@ -197,11 +194,16 @@ import { ClaimMappings } from '~/resources/authentication' import { usePageActivity } from '../lib/usePageActivity' export function LoadData(props: { children?: ReactNode }) { - const { t } = useTranslation() - const { loadCompleted, setLoadStarted, setLoadCompleted, setIsStreamIdle, setIsReconnecting, mounted } = - useContext(PluginDataContext) + const { + loadCompleted, + setLoadStarted, + setLoadCompleted, + setIsStreamIdle, + setIsReconnecting, + setFlappingAlerts, + mounted, + } = useContext(PluginDataContext) const [eventsLoaded, setEventsLoaded] = useState(false) - const [flappingAlerts, setFlappingAlerts] = useState([]) const idleTimeoutMs = useEventStreamIdleTimeout() const gracePeriodMs = useEventStreamIdleGracePeriod() const { isActive } = usePageActivity(idleTimeoutMs, mounted) @@ -613,7 +615,7 @@ export function LoadData(props: { children?: ReactNode }) { eventSourceRef.current = undefined processIntervalRef.current = undefined } - }, [caches, mappers, restartKey, setIsReconnecting, setLoadStarted, setSettings, setters]) + }, [caches, mappers, restartKey, setFlappingAlerts, setIsReconnecting, setLoadStarted, setSettings, setters]) const { data: globalHubRes, @@ -718,35 +720,7 @@ export function LoadData(props: { children?: ReactNode }) { const children = useMemo(() => {props.children}, [props.children]) - return ( - - {flappingAlerts.length > 0 && ( - - {flappingAlerts.map((alert) => { - const key = `${alert.kind}/${alert.namespace}/${alert.name}` - return ( - { - setFlappingAlerts((alerts) => alerts.filter((a) => `${a.kind}/${a.namespace}/${a.name}` !== key)) - }} - /> - } - > - {alert.message} - - ) - })} - - )} - {children} - - ) + return children } function resetCaches(caches: Record>>) { diff --git a/frontend/src/components/LoadPluginData.tsx b/frontend/src/components/LoadPluginData.tsx index 364fa627602..a88217ecbd2 100644 --- a/frontend/src/components/LoadPluginData.tsx +++ b/frontend/src/components/LoadPluginData.tsx @@ -2,6 +2,7 @@ import { ReactNode, useContext, useEffect } from 'react' import { css } from '@emotion/css' import { PluginContext } from '../lib/PluginContext' +import { FlappingAlerts } from './FlappingAlerts' import { LostChangesProvider } from './LostChanges' import { LoadingPage } from './LoadingPage' import { StreamStatusOverlay } from './StreamStatusOverlay' @@ -83,6 +84,7 @@ export const LoadPluginData = (props: { children?: ReactNode }) => {
{isStreamIdle && } {isReconnecting && } + {props.children}
) : ( diff --git a/frontend/src/lib/PluginDataContext.tsx b/frontend/src/lib/PluginDataContext.tsx index cf5ef9e1151..161588eef60 100644 --- a/frontend/src/lib/PluginDataContext.tsx +++ b/frontend/src/lib/PluginDataContext.tsx @@ -2,6 +2,8 @@ import { createContext, useState, useMemo, useCallback, Dispatch, SetStateAction } from 'react' // eslint-disable-next-line @typescript-eslint/no-restricted-imports import * as atoms from '../atoms' + +type FlappingEvent = atoms.FlappingEvent // eslint-disable-next-line @typescript-eslint/no-restricted-imports import * as recoil from 'recoil' // eslint-disable-next-line @typescript-eslint/no-restricted-imports @@ -24,10 +26,12 @@ export type PluginData = { startLoading: boolean isStreamIdle: boolean isReconnecting: boolean + flappingAlerts: FlappingEvent[] setLoadCompleted: Dispatch> setLoadStarted: Dispatch> setIsStreamIdle: Dispatch> setIsReconnecting: Dispatch> + setFlappingAlerts: Dispatch> mounted: boolean mount: () => void unmount: () => void @@ -45,10 +49,12 @@ export const defaultContext = { startLoading: false, isStreamIdle: false, isReconnecting: false, + flappingAlerts: [], setLoadCompleted: () => {}, setLoadStarted: () => {}, setIsStreamIdle: () => {}, setIsReconnecting: () => {}, + setFlappingAlerts: () => {}, mounted: false, mount: () => {}, unmount: () => {}, @@ -63,6 +69,7 @@ export const usePluginDataContextValue = () => { const [startLoading, setStartLoading] = useState(false) const [isStreamIdle, setIsStreamIdle] = useState(false) const [isReconnecting, setIsReconnecting] = useState(false) + const [flappingAlerts, setFlappingAlerts] = useState([]) const [mountCount, setMountCount] = useState(0) const backendUrl = getBackendUrl() @@ -81,16 +88,29 @@ export const usePluginDataContextValue = () => { startLoading, isStreamIdle, isReconnecting, + flappingAlerts, setLoadCompleted, setLoadStarted, setIsStreamIdle, setIsReconnecting, + setFlappingAlerts, mounted: mountCount > 0, mount, unmount, load: () => setStartLoading(true), }), - [backendUrl, loadStarted, loadCompleted, startLoading, isStreamIdle, isReconnecting, mountCount, mount, unmount] + [ + backendUrl, + loadStarted, + loadCompleted, + startLoading, + isStreamIdle, + isReconnecting, + flappingAlerts, + mountCount, + mount, + unmount, + ] ) return contextValue } From 3ea8644f97eae88ff15faefa92f40ec1f84b51f2 Mon Sep 17 00:00:00 2001 From: John Swanke Date: Thu, 27 Aug 2026 14:08:47 -0400 Subject: [PATCH 05/12] i18n Signed-off-by: John Swanke --- frontend/public/locales/en/translation.json | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/public/locales/en/translation.json b/frontend/public/locales/en/translation.json index 2ad25c0d9fe..7753c1b2153 100644 --- a/frontend/public/locales/en/translation.json +++ b/frontend/public/locales/en/translation.json @@ -31,6 +31,7 @@ "{{count}} more_other": "{{count}} more", "{{count}} selected_one": "{{count}} selected", "{{count}} selected_other": "{{count}} selected", + "{{kind}} {{name}} in namespace {{namespace}} has been modified more than {{threshold}} times in the last {{windowMinutes}} minutes. Verify this resource is configured correctly. Updates are being limited to {{timesPerMinute}} times per minute.": "{{kind}} {{name}} in namespace {{namespace}} has been modified more than {{threshold}} times in the last {{windowMinutes}} minutes. Verify this resource is configured correctly. Updates are being limited to {{timesPerMinute}} times per minute.", "{{kind}} details": "{{kind}} details", "{{matched}} of {{total}} clusters": "{{matched}} of {{total}} clusters", "{{matched}} of {{total}} clusters matched": "{{matched}} of {{total}} clusters matched", From 72fab0484fa4e9762f51cf28c275073b5dd75e09 Mon Sep 17 00:00:00 2001 From: John Swanke Date: Tue, 8 Sep 2026 16:06:21 -0400 Subject: [PATCH 06/12] address comments Signed-off-by: John Swanke --- backend/src/lib/server-side-events.ts | 6 +- backend/src/resources/resource.ts | 1 + backend/src/routes/events.ts | 113 +++++-------- backend/test/routes/events.test.ts | 150 ++++++++++++++---- frontend/public/locales/en/translation.json | 4 +- frontend/src/atoms.ts | 12 +- frontend/src/components/FlappingAlerts.tsx | 56 ------- frontend/src/components/LoadData.tsx | 22 +-- frontend/src/components/LoadPluginData.tsx | 2 - frontend/src/lib/PluginDataContext.tsx | 22 +-- frontend/src/resources/policy.ts | 1 + .../routes/Governance/policies/Policies.tsx | 2 +- .../Governance/policies/PolicyTableCell.tsx | 24 ++- 13 files changed, 189 insertions(+), 226 deletions(-) delete mode 100644 frontend/src/components/FlappingAlerts.tsx diff --git a/backend/src/lib/server-side-events.ts b/backend/src/lib/server-side-events.ts index 0f8a2a90146..73840733644 100644 --- a/backend/src/lib/server-side-events.ts +++ b/backend/src/lib/server-side-events.ts @@ -323,15 +323,13 @@ export class ServerSideEvents { parts.push(loaded) } - // remove START, SETTINGS, FLAPPING and LOADED from events + // remove START, SETTINGS and LOADED from events const start = parts.shift() const end = parts.pop() const inx = parts.findIndex(({ data }) => { return (data as { type?: 'SETTINGS' }).type === 'SETTINGS' }) const settings = parts.splice(inx, 1)[0] - const flappingEvents = parts.filter(({ data }) => (data as { type?: string }).type === 'FLAPPING') - parts = parts.filter(({ data }) => (data as { type?: string }).type !== 'FLAPPING') // separate resource by kind // we want to send the resources that populate the main console pages first @@ -399,7 +397,7 @@ export class ServerSideEvents { // send packets of resources // with resources that fill main console pages first let sentCount = 0 - const sending = [start, settings, ...flappingEvents] + const sending = [start, settings] do { sending.push(...clusters.splice(0, 200)) sending.push(...agents.splice(0, 200)) diff --git a/backend/src/resources/resource.ts b/backend/src/resources/resource.ts index bb5f5e0c3ce..bcdcb981be1 100644 --- a/backend/src/resources/resource.ts +++ b/backend/src/resources/resource.ts @@ -43,6 +43,7 @@ interface OwnerReference { export interface IResource { kind: string apiVersion: string + throttled?: boolean metadata?: { name: string namespace?: string diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index 6d3456bbc33..412d1529f94 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -35,17 +35,7 @@ export interface SettingsEvent { settings: Record } -export interface FlappingEvent { - type: 'FLAPPING' - kind: string - namespace: string - name: string - threshold: number - windowMs: number - cooldownMs: number -} - -type ServerSideEventData = WatchEvent | SettingsEvent | FlappingEvent | { type: 'START' | 'LOADED' | 'EOP' } +type ServerSideEventData = WatchEvent | SettingsEvent | { type: 'START' | 'LOADED' | 'EOP' } let requests: { cancel: () => void }[] = [] @@ -57,29 +47,26 @@ let requests: { cancel: () => void }[] = [] * 1) Overwhelm the event processing loop / liveliness probe * 2) Grow client event queues until the pod OOMs * - * Detection: if the same kind/namespace/name is modified more than FLAP_THRESHOLD (N) times + * Detection: if the same namespace/name Policy is modified more than FLAP_THRESHOLD (N) times * within FLAP_WINDOW_MS (M), the resource is considered flapping. - * While flapping, browser broadcasts are limited to once every FLAP_COOLDOWN_MS (P). - * Only kinds listed in FLAP_THROTTLE_KINDS are subject to this check. + * Only Policy resources are subject to this check. + * While flapping, the resource is cached/broadcast when flapping is first detected (marked with + * `throttled: true`) and then at most once per minute (FLAP_COOLDOWN_MS) while still flapping. + * Cache suppression continues for the full cooldown even if the detection window clears before + * the minute elapses, so re-entry into flapping does not bypass the interval. * * N is initially 5 times * M is initially 5 seconds * P is initially 1 minute - * Kind is initially ['Policy'] */ export const FLAP_THRESHOLD = 5 // N: modifications that trigger flapping detection export const FLAP_WINDOW_MS = 5 * 1000 // M: sliding window for counting modifications -export const FLAP_COOLDOWN_MS = 60 * 1000 // P: min interval between browser updates while flapping -export const FLAP_THROTTLE_KINDS = ['Policy'] // kinds subject to flapping detection +export const FLAP_COOLDOWN_MS = 60 * 1000 // P: min interval between cache updates while flapping interface FlapTrackerEntry { timestamps: number[] throttled: boolean - lastForwardedAt: number - flappingEventID?: number - kind: string - namespace: string - name: string + lastCachedAt: number } const flapTracker: Record = {} @@ -87,10 +74,6 @@ const flapTracker: Record = {} /** Clear flap tracker state. Used for test isolation. */ export function resetFlapTracker() { for (const key in flapTracker) { - const entry = flapTracker[key] - if (entry.flappingEventID) { - ServerSideEvents.removeEvent(entry.flappingEventID) - } delete flapTracker[key] } } @@ -111,77 +94,50 @@ export function formatFlappingMessage(kind: string, namespace: string, name: str return `${kind} ${name} in namespace ${namespace} has been modified more than ${FLAP_THRESHOLD} times in the last ${windowMinutes} minutes. Verify this resource is configured correctly. Updates are being limited to ${timesPerMinute} times per minute.` } -async function notifyFlapping(entry: FlapTrackerEntry): Promise { - const message = formatFlappingMessage(entry.kind, entry.namespace, entry.name) - logger.warn({ msg: message, kind: entry.kind, namespace: entry.namespace, name: entry.name }) - if (entry.flappingEventID) { - ServerSideEvents.removeEvent(entry.flappingEventID) - } - entry.flappingEventID = await ServerSideEvents.pushEvent({ - data: { - type: 'FLAPPING', - kind: entry.kind, - namespace: entry.namespace, - name: entry.name, - threshold: FLAP_THRESHOLD, - windowMs: FLAP_WINDOW_MS, - cooldownMs: FLAP_COOLDOWN_MS, - } satisfies FlappingEvent, - }) -} - -function clearFlappingNotice(entry: FlapTrackerEntry): void { - if (entry.flappingEventID) { - ServerSideEvents.removeEvent(entry.flappingEventID) - entry.flappingEventID = undefined - } -} - /** - * Records a modification for flap detection and returns whether this update should be - * forwarded to browser clients. Non-throttled kinds always return true. + * Records a modification for flap detection on Policy resources and returns whether this + * update should be throttled (suppressed). Sets `resource.throttled` to true when flapping + * is detected. Non-Policy resources are never throttled. */ -export function shouldForwardResourceUpdate( - resource: Pick & { metadata?: { namespace?: string; name?: string } }, +export function shouldThrottleResource( + resource: Pick & { metadata?: { namespace?: string; name?: string } }, now = Date.now() ): boolean { - if (!FLAP_THROTTLE_KINDS.includes(resource.kind)) { - return true + if (resource.kind !== 'Policy') { + return false } const key = resourceFlapKey(resource) - const kind = resource.kind - const namespace = resource.metadata?.namespace ?? '' - const name = resource.metadata?.name ?? '' let entry = flapTracker[key] if (!entry) { - entry = { timestamps: [], throttled: false, lastForwardedAt: 0, kind, namespace, name } + entry = { timestamps: [], throttled: false, lastCachedAt: 0 } flapTracker[key] = entry } entry.timestamps.push(now) entry.timestamps = entry.timestamps.filter((t) => now - t <= FLAP_WINDOW_MS) - const wasThrottled = entry.throttled - entry.throttled = entry.timestamps.length > FLAP_THRESHOLD + const isFlapping = entry.timestamps.length > FLAP_THRESHOLD + entry.throttled = isFlapping - if (entry.throttled && !wasThrottled) { - void notifyFlapping(entry) - } else if (!entry.throttled && wasThrottled) { - clearFlappingNotice(entry) + if (isFlapping) { + resource.throttled = true + } else { + delete resource.throttled } - if (!entry.throttled) { - entry.lastForwardedAt = now + const withinCooldown = entry.lastCachedAt > 0 && now - entry.lastCachedAt < FLAP_COOLDOWN_MS + if (withinCooldown) { return true } - // While flapping: allow at most one browser update every FLAP_COOLDOWN_MS - if (entry.lastForwardedAt === 0 || now - entry.lastForwardedAt >= FLAP_COOLDOWN_MS) { - entry.lastForwardedAt = now - return true + if (isFlapping) { + entry.lastCachedAt = now + return false } + + entry.lastCachedAt = 0 return false } @@ -1002,8 +958,12 @@ export async function cacheResource(resource: IResource, forwardEventsToClients existing = latestExisting } - // Always update the in-memory cache; only throttle browser broadcasts for flapping resources - const shouldForward = forwardEventsToClients && shouldForwardResourceUpdate(resource) + // Skip caching/broadcasting for throttled updates except the transition update and once per minute. + if (shouldThrottleResource(resource)) { + return resource.metadata.resourceVersion + } + + const shouldForward = forwardEventsToClients if (shouldForward && existing) { const eventID = await existing.eventID // if no other cacheResource call updated the cache while we were awaiting, we can replace the cache entry and event @@ -1079,7 +1039,6 @@ function eventFilter(token: string, serverSideEvent: ServerSideEvent { ) }) - it('should forward non-Policy kinds without throttling', () => { + it('should never throttle non-Policy kinds', () => { const now = Date.now() for (let i = 0; i < FLAP_THRESHOLD + 10; i++) { expect( - shouldForwardResourceUpdate( - { kind: 'ManagedCluster', metadata: { name: 'cluster-a', namespace: '' } }, - now + i - ) - ).toBe(true) + shouldThrottleResource({ kind: 'ManagedCluster', metadata: { name: 'cluster-a', namespace: '' } }, now + i) + ).toBe(false) } }) + it('should only set throttled on the resource when flapping is detected', () => { + const base = Date.now() + const policy: { kind: string; metadata: { name: string; namespace: string }; throttled?: boolean } = { + kind: 'Policy', + metadata: { name: 'policy-a', namespace: 'default' }, + } + + for (let i = 0; i < FLAP_THRESHOLD; i++) { + shouldThrottleResource(policy, base + i) + expect(policy.throttled).toBeUndefined() + } + + shouldThrottleResource(policy, base + FLAP_THRESHOLD) + expect(policy.throttled).toBe(true) + + shouldThrottleResource(policy, base + FLAP_WINDOW_MS + FLAP_THRESHOLD + 10) + expect(policy.throttled).toBeUndefined() + }) + it('should throttle Policy updates after more than N modifications within M seconds', () => { const base = Date.now() const policy = { kind: 'Policy', metadata: { name: 'flappy', namespace: 'default' } } - let forwarded = 0 - // Sustained high-frequency updates across the cooldown window. - // lastForwardedAt is set near base + (FLAP_THRESHOLD-1)*100, so run past that + cooldown. - const end = base + (FLAP_THRESHOLD - 1) * 100 + FLAP_COOLDOWN_MS + 500 + let notThrottledCount = 0 + const end = base + (FLAP_THRESHOLD + 5) * 100 for (let t = base; t <= end; t += 100) { - if (shouldForwardResourceUpdate(policy, t)) { - forwarded += 1 + if (!shouldThrottleResource(policy, t)) { + notThrottledCount += 1 } } expect(getFlapTracker()['Policy/default/flappy'].throttled).toBe(true) - // First FLAP_THRESHOLD forwards, then at most one more after cooldown while still flapping - expect(forwarded).toBe(FLAP_THRESHOLD + 1) + // First FLAP_THRESHOLD updates, plus the transition update when flapping is detected. + // Further updates within the cooldown interval are throttled. + expect(notThrottledCount).toBe(FLAP_THRESHOLD + 1) }) - it('should stop throttling when modifications fall back within the detection window', () => { + it('should allow caching again after FLAP_COOLDOWN_MS while still throttled', () => { + const base = Date.now() + const policy = { kind: 'Policy', metadata: { name: 'periodic', namespace: 'default' } } + + for (let i = 0; i <= FLAP_THRESHOLD; i++) { + shouldThrottleResource(policy, base + i) + } + expect(getFlapTracker()['Policy/default/periodic'].throttled).toBe(true) + + expect(shouldThrottleResource(policy, base + FLAP_THRESHOLD + 100)).toBe(true) + + const periodicAt = base + FLAP_THRESHOLD + FLAP_COOLDOWN_MS + for (let i = 1; i <= FLAP_THRESHOLD; i++) { + shouldThrottleResource(policy, periodicAt - 1000 + i * 100) + } + + expect(shouldThrottleResource(policy, periodicAt)).toBe(false) + }) + + it('should continue suppressing caches after detection window clears until cooldown expires', () => { + const base = Date.now() + const policy: { kind: string; metadata: { name: string; namespace: string }; throttled?: boolean } = { + kind: 'Policy', + metadata: { name: 'sticky', namespace: 'default' }, + } + + for (let i = 0; i <= FLAP_THRESHOLD; i++) { + shouldThrottleResource(policy, base + i) + } + + const afterWindowClears = base + FLAP_WINDOW_MS + FLAP_THRESHOLD + 10 + expect(shouldThrottleResource(policy, afterWindowClears)).toBe(true) + expect(policy.throttled).toBeUndefined() + }) + + it('should stop throttling when modifications fall back and cooldown expires', () => { const base = Date.now() const policy = { kind: 'Policy', metadata: { name: 'recovering', namespace: 'ns1' } } for (let i = 0; i <= FLAP_THRESHOLD; i++) { - shouldForwardResourceUpdate(policy, base + i) + shouldThrottleResource(policy, base + i) } expect(getFlapTracker()['Policy/ns1/recovering'].throttled).toBe(true) - // Advance far enough that every prior timestamp falls outside the detection window - expect(shouldForwardResourceUpdate(policy, base + FLAP_WINDOW_MS + FLAP_THRESHOLD + 10)).toBe(true) + const afterCooldown = base + FLAP_THRESHOLD + FLAP_COOLDOWN_MS + 1 + expect(shouldThrottleResource(policy, afterCooldown)).toBe(false) expect(getFlapTracker()['Policy/ns1/recovering'].throttled).toBe(false) + expect(getFlapTracker()['Policy/ns1/recovering'].lastCachedAt).toBe(0) }) - it('should keep updating cache while suppressing excess browser events for a flapping Policy', async () => { + it('should cache at most once per minute while a Policy is flapping', async () => { const pushSpy = jest.spyOn(ServerSideEvents, 'pushEvent') + const base = 1_000_000_000_000 + let now = base + jest.spyOn(Date, 'now').mockImplementation(() => now) - for (let i = 1; i <= FLAP_THRESHOLD + 3; i++) { + for (let i = 1; i <= FLAP_THRESHOLD + 1; i++) { await cacheResource({ kind: 'Policy', apiVersion: 'policy.open-cluster-management.io/v1', @@ -1584,29 +1638,65 @@ describe('events Route', () => { resourceVersion: String(i), }, }) + now += 100 } - // Allow async FLAPPING notify + MODIFIED pushes to settle + now += 100 + await cacheResource({ + kind: 'Policy', + apiVersion: 'policy.open-cluster-management.io/v1', + metadata: { + name: 'flappy-policy', + namespace: 'default', + uid: 'flappy-uid', + resourceVersion: String(FLAP_THRESHOLD + 2), + }, + }) + await new Promise((resolve) => setTimeout(resolve, 0)) for (const entry of Object.values(getEventCache())) { await Promise.all(Object.values(entry).map((e) => e.eventID)) } - const modifiedPushes = pushSpy.mock.calls.filter( + let modifiedPushes = pushSpy.mock.calls.filter( (call) => (call[0].data as { type?: string })?.type === 'MODIFIED' ) - const flappingPushes = pushSpy.mock.calls.filter( - (call) => (call[0].data as { type?: string })?.type === 'FLAPPING' - ) + expect(modifiedPushes.length).toBe(FLAP_THRESHOLD + 1) + + const periodicAt = base + FLAP_THRESHOLD * 100 + FLAP_COOLDOWN_MS + for (let i = 1; i <= FLAP_THRESHOLD; i++) { + shouldThrottleResource( + { kind: 'Policy', metadata: { name: 'flappy-policy', namespace: 'default' } }, + periodicAt - 1000 + i * 100 + ) + } + + now = periodicAt + await cacheResource({ + kind: 'Policy', + apiVersion: 'policy.open-cluster-management.io/v1', + metadata: { + name: 'flappy-policy', + namespace: 'default', + uid: 'flappy-uid', + resourceVersion: String(FLAP_THRESHOLD * 2 + 3), + }, + }) + + await new Promise((resolve) => setTimeout(resolve, 0)) + for (const entry of Object.values(getEventCache())) { + await Promise.all(Object.values(entry).map((e) => e.eventID)) + } - // First FLAP_THRESHOLD updates forward; subsequent ones in the window are suppressed - expect(modifiedPushes.length).toBe(FLAP_THRESHOLD) - expect(flappingPushes.length).toBe(1) + modifiedPushes = pushSpy.mock.calls.filter((call) => (call[0].data as { type?: string })?.type === 'MODIFIED') + expect(modifiedPushes.length).toBe(FLAP_THRESHOLD + 2) const resources = await getKubeResources('Policy', 'policy.open-cluster-management.io/v1') expect(resources).toHaveLength(1) - expect(resources[0].metadata.resourceVersion).toBe(String(FLAP_THRESHOLD + 3)) + expect(resources[0].metadata.resourceVersion).toBe(String(FLAP_THRESHOLD * 2 + 3)) + expect(resources[0].throttled).toBe(true) + jest.restoreAllMocks() pushSpy.mockRestore() }) }) diff --git a/frontend/public/locales/en/translation.json b/frontend/public/locales/en/translation.json index 7753c1b2153..524c9fa3efe 100644 --- a/frontend/public/locales/en/translation.json +++ b/frontend/public/locales/en/translation.json @@ -31,7 +31,6 @@ "{{count}} more_other": "{{count}} more", "{{count}} selected_one": "{{count}} selected", "{{count}} selected_other": "{{count}} selected", - "{{kind}} {{name}} in namespace {{namespace}} has been modified more than {{threshold}} times in the last {{windowMinutes}} minutes. Verify this resource is configured correctly. Updates are being limited to {{timesPerMinute}} times per minute.": "{{kind}} {{name}} in namespace {{namespace}} has been modified more than {{threshold}} times in the last {{windowMinutes}} minutes. Verify this resource is configured correctly. Updates are being limited to {{timesPerMinute}} times per minute.", "{{kind}} details": "{{kind}} details", "{{matched}} of {{total}} clusters": "{{matched}} of {{total}} clusters", "{{matched}} of {{total}} clusters matched": "{{matched}} of {{total}} clusters matched", @@ -2603,6 +2602,8 @@ "policy.table.actions.enforcing": "Enforcing", "policy.table.actions.inform": "Inform", "policy.table.actions.informing": "Informing", + "policy.table.throttled": "Updates throttled", + "policy.table.throttled.tooltip": "This policy is updating too frequently. Policy updates are therefore being limited to once per minute. Verify this policy is configured correctly", "policy.tableHeader.name": "Policy name", "policy.violations_one": "{{count}} policy with violations", "policy.violations_other": "{{count}} policies with violations", @@ -2748,7 +2749,6 @@ "Resource name": "Resource name", "Resource nodes": "Resource nodes", "Resource type": "Resource type", - "Resource update throttled": "Resource update throttled", "resource.error": "Failed to load.", "resource.labels": "Labels", "resource.loading": "Loading...", diff --git a/frontend/src/atoms.ts b/frontend/src/atoms.ts index 79ae93e6b6d..c41a423cb1f 100644 --- a/frontend/src/atoms.ts +++ b/frontend/src/atoms.ts @@ -203,17 +203,7 @@ export interface SettingsEvent { settings: Record } -export interface FlappingEvent { - type: 'FLAPPING' - kind: string - namespace: string - name: string - threshold: number - windowMs: number - cooldownMs: number -} - -export type ServerSideEventData = WatchEvent | SettingsEvent | FlappingEvent | { type: 'START' | 'LOADED' | 'EOP' } +export type ServerSideEventData = WatchEvent | SettingsEvent | { type: 'START' | 'LOADED' | 'EOP' } export function usePolicies() { const policies = useRecoilValue(policiesState) diff --git a/frontend/src/components/FlappingAlerts.tsx b/frontend/src/components/FlappingAlerts.tsx deleted file mode 100644 index a9e59e88643..00000000000 --- a/frontend/src/components/FlappingAlerts.tsx +++ /dev/null @@ -1,56 +0,0 @@ -/* Copyright Contributors to the Open Cluster Management project */ -import { useContext } from 'react' -import { Alert, AlertActionCloseButton, AlertGroup } from '@patternfly/react-core' -import { useTranslation } from '../lib/acm-i18next' -import { PluginContext } from '../lib/PluginContext' - -/** - * Renders warnings for resources that are being throttled due to flapping (see backend/src/routes/events.ts). - * - * This must be rendered inside actual ACM/MCE routed page content (e.g. via LoadPluginData), not inside - * PluginDataContextProvider/LoadData. That provider is registered as a global OpenShift Console - * `console.context-provider` extension, so anything it renders directly would appear on every console page, - * not just ACM/MCE pages. - */ -export function FlappingAlerts() { - const { t } = useTranslation() - const { dataContext } = useContext(PluginContext) - const { flappingAlerts, setFlappingAlerts } = useContext(dataContext) - - if (flappingAlerts.length === 0) return null - - return ( - - {flappingAlerts.map((alert) => { - const key = `${alert.kind}/${alert.namespace}/${alert.name}` - return ( - { - setFlappingAlerts((alerts) => alerts.filter((a) => `${a.kind}/${a.namespace}/${a.name}` !== key)) - }} - /> - } - > - {t( - '{{kind}} {{name}} in namespace {{namespace}} has been modified more than {{threshold}} times in the last {{windowMinutes}} minutes. Verify this resource is configured correctly. Updates are being limited to {{timesPerMinute}} times per minute.', - { - kind: alert.kind, - name: alert.name, - namespace: alert.namespace, - threshold: alert.threshold, - windowMinutes: Math.max(1, Math.round(alert.windowMs / 60_000)), - timesPerMinute: Math.max(1, Math.round(60_000 / alert.cooldownMs)), - } - )} - - ) - })} - - ) -} diff --git a/frontend/src/components/LoadData.tsx b/frontend/src/components/LoadData.tsx index 3b3a4f19179..6bf2b5fc982 100644 --- a/frontend/src/components/LoadData.tsx +++ b/frontend/src/components/LoadData.tsx @@ -194,15 +194,8 @@ import { ClaimMappings } from '~/resources/authentication' import { usePageActivity } from '../lib/usePageActivity' export function LoadData(props: { children?: ReactNode }) { - const { - loadCompleted, - setLoadStarted, - setLoadCompleted, - setIsStreamIdle, - setIsReconnecting, - setFlappingAlerts, - mounted, - } = useContext(PluginDataContext) + const { loadCompleted, setLoadStarted, setLoadCompleted, setIsStreamIdle, setIsReconnecting, mounted } = + useContext(PluginDataContext) const [eventsLoaded, setEventsLoaded] = useState(false) const idleTimeoutMs = useEventStreamIdleTimeout() const gracePeriodMs = useEventStreamIdleGracePeriod() @@ -572,15 +565,6 @@ export function LoadData(props: { children?: ReactNode }) { case 'SETTINGS': setSettings(data.settings) break - case 'FLAPPING': - setFlappingAlerts((alerts) => { - const key = `${data.kind}/${data.namespace}/${data.name}` - if (alerts.some((a) => `${a.kind}/${a.namespace}/${a.name}` === key)) { - return alerts.map((a) => (`${a.kind}/${a.namespace}/${a.name}` === key ? data : a)) - } - return [...alerts, data] - }) - break } } catch (err) { console.error(err) @@ -615,7 +599,7 @@ export function LoadData(props: { children?: ReactNode }) { eventSourceRef.current = undefined processIntervalRef.current = undefined } - }, [caches, mappers, restartKey, setFlappingAlerts, setIsReconnecting, setLoadStarted, setSettings, setters]) + }, [caches, mappers, restartKey, setIsReconnecting, setLoadStarted, setSettings, setters]) const { data: globalHubRes, diff --git a/frontend/src/components/LoadPluginData.tsx b/frontend/src/components/LoadPluginData.tsx index a88217ecbd2..364fa627602 100644 --- a/frontend/src/components/LoadPluginData.tsx +++ b/frontend/src/components/LoadPluginData.tsx @@ -2,7 +2,6 @@ import { ReactNode, useContext, useEffect } from 'react' import { css } from '@emotion/css' import { PluginContext } from '../lib/PluginContext' -import { FlappingAlerts } from './FlappingAlerts' import { LostChangesProvider } from './LostChanges' import { LoadingPage } from './LoadingPage' import { StreamStatusOverlay } from './StreamStatusOverlay' @@ -84,7 +83,6 @@ export const LoadPluginData = (props: { children?: ReactNode }) => {
{isStreamIdle && } {isReconnecting && } - {props.children}
) : ( diff --git a/frontend/src/lib/PluginDataContext.tsx b/frontend/src/lib/PluginDataContext.tsx index 161588eef60..cf5ef9e1151 100644 --- a/frontend/src/lib/PluginDataContext.tsx +++ b/frontend/src/lib/PluginDataContext.tsx @@ -2,8 +2,6 @@ import { createContext, useState, useMemo, useCallback, Dispatch, SetStateAction } from 'react' // eslint-disable-next-line @typescript-eslint/no-restricted-imports import * as atoms from '../atoms' - -type FlappingEvent = atoms.FlappingEvent // eslint-disable-next-line @typescript-eslint/no-restricted-imports import * as recoil from 'recoil' // eslint-disable-next-line @typescript-eslint/no-restricted-imports @@ -26,12 +24,10 @@ export type PluginData = { startLoading: boolean isStreamIdle: boolean isReconnecting: boolean - flappingAlerts: FlappingEvent[] setLoadCompleted: Dispatch> setLoadStarted: Dispatch> setIsStreamIdle: Dispatch> setIsReconnecting: Dispatch> - setFlappingAlerts: Dispatch> mounted: boolean mount: () => void unmount: () => void @@ -49,12 +45,10 @@ export const defaultContext = { startLoading: false, isStreamIdle: false, isReconnecting: false, - flappingAlerts: [], setLoadCompleted: () => {}, setLoadStarted: () => {}, setIsStreamIdle: () => {}, setIsReconnecting: () => {}, - setFlappingAlerts: () => {}, mounted: false, mount: () => {}, unmount: () => {}, @@ -69,7 +63,6 @@ export const usePluginDataContextValue = () => { const [startLoading, setStartLoading] = useState(false) const [isStreamIdle, setIsStreamIdle] = useState(false) const [isReconnecting, setIsReconnecting] = useState(false) - const [flappingAlerts, setFlappingAlerts] = useState([]) const [mountCount, setMountCount] = useState(0) const backendUrl = getBackendUrl() @@ -88,29 +81,16 @@ export const usePluginDataContextValue = () => { startLoading, isStreamIdle, isReconnecting, - flappingAlerts, setLoadCompleted, setLoadStarted, setIsStreamIdle, setIsReconnecting, - setFlappingAlerts, mounted: mountCount > 0, mount, unmount, load: () => setStartLoading(true), }), - [ - backendUrl, - loadStarted, - loadCompleted, - startLoading, - isStreamIdle, - isReconnecting, - flappingAlerts, - mountCount, - mount, - unmount, - ] + [backendUrl, loadStarted, loadCompleted, startLoading, isStreamIdle, isReconnecting, mountCount, mount, unmount] ) return contextValue } diff --git a/frontend/src/resources/policy.ts b/frontend/src/resources/policy.ts index cd2483fc9fb..f0d5acf7d76 100644 --- a/frontend/src/resources/policy.ts +++ b/frontend/src/resources/policy.ts @@ -42,6 +42,7 @@ export interface Policy { } // This not from API, this will be added at console remediationResult?: REMEDIATION_ACTION | string + throttled?: boolean } export interface PolicyTemplate { diff --git a/frontend/src/routes/Governance/policies/Policies.tsx b/frontend/src/routes/Governance/policies/Policies.tsx index 84ba41828b6..b9adc8a23d4 100644 --- a/frontend/src/routes/Governance/policies/Policies.tsx +++ b/frontend/src/routes/Governance/policies/Policies.tsx @@ -125,7 +125,7 @@ export default function PoliciesPage() { () => [ { header: t('Name'), - cell: handleNameCell, + cell: (item) => handleNameCell(item, t), sort: 'policy.metadata.name', search: 'policy.metadata.name', id: 'name', diff --git a/frontend/src/routes/Governance/policies/PolicyTableCell.tsx b/frontend/src/routes/Governance/policies/PolicyTableCell.tsx index fbeef4678d5..d62affb9769 100644 --- a/frontend/src/routes/Governance/policies/PolicyTableCell.tsx +++ b/frontend/src/routes/Governance/policies/PolicyTableCell.tsx @@ -8,13 +8,14 @@ import { PolicySetList } from '../common/util' import { PolicyActionDropdown } from '../components/PolicyActionDropdown' import { AcmButton } from '../../../ui-components/AcmButton' import { AutomationDetailsSidebar } from '../components/AutomationDetailsSidebar' -import { ButtonVariant } from '@patternfly/react-core' +import { ButtonVariant, Flex, FlexItem, Icon, Tooltip } from '@patternfly/react-core' +import { ClockIcon } from '@patternfly/react-icons' import type { TFunction } from 'i18next' import AcmTimestamp from '../../../lib/AcmTimestamp' import { AcmVisitedLink } from '../../../ui-components' -export function handleNameCell(item: PolicyTableItem) { - return ( +export function handleNameCell(item: PolicyTableItem, t: TFunction) { + const nameLink = ( ) + + if (item.policy.throttled !== true) { + return nameLink + } + + return ( + + {nameLink} + + + + + + + + + ) } export function handleStatusCell(item: PolicyTableItem, t: TFunction, returnExportString?: boolean) { From 920b7d5c31dbb695fd3ad375e3750c41f0b911c5 Mon Sep 17 00:00:00 2001 From: John Swanke Date: Tue, 8 Sep 2026 16:15:01 -0400 Subject: [PATCH 07/12] check Signed-off-by: John Swanke --- backend/test/routes/events.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/backend/test/routes/events.test.ts b/backend/test/routes/events.test.ts index 52507299945..34bae5b931c 100644 --- a/backend/test/routes/events.test.ts +++ b/backend/test/routes/events.test.ts @@ -1658,9 +1658,7 @@ describe('events Route', () => { await Promise.all(Object.values(entry).map((e) => e.eventID)) } - let modifiedPushes = pushSpy.mock.calls.filter( - (call) => (call[0].data as { type?: string })?.type === 'MODIFIED' - ) + let modifiedPushes = pushSpy.mock.calls.filter((call) => (call[0].data as { type?: string })?.type === 'MODIFIED') expect(modifiedPushes.length).toBe(FLAP_THRESHOLD + 1) const periodicAt = base + FLAP_THRESHOLD * 100 + FLAP_COOLDOWN_MS From ad521bb2d40a96aa20ec21ee560f7772c1b05917 Mon Sep 17 00:00:00 2001 From: John Swanke Date: Fri, 11 Sep 2026 16:36:39 -0400 Subject: [PATCH 08/12] backup Signed-off-by: John Swanke --- backend/src/lib/server-side-events.ts | 6 +- backend/src/routes/events.ts | 253 +++++++++++++++++--------- backend/test/routes/events.test.ts | 141 +++++++++----- frontend/src/atoms.ts | 11 +- frontend/src/components/LoadData.tsx | 4 + 5 files changed, 285 insertions(+), 130 deletions(-) diff --git a/backend/src/lib/server-side-events.ts b/backend/src/lib/server-side-events.ts index 73840733644..9750eabf564 100644 --- a/backend/src/lib/server-side-events.ts +++ b/backend/src/lib/server-side-events.ts @@ -323,13 +323,15 @@ export class ServerSideEvents { parts.push(loaded) } - // remove START, SETTINGS and LOADED from events + // remove START, SETTINGS, THROTTLED and LOADED from events const start = parts.shift() const end = parts.pop() const inx = parts.findIndex(({ data }) => { return (data as { type?: 'SETTINGS' }).type === 'SETTINGS' }) const settings = parts.splice(inx, 1)[0] + const throttledEvents = parts.filter(({ data }) => (data as { type?: string }).type === 'THROTTLED') + parts = parts.filter(({ data }) => (data as { type?: string }).type !== 'THROTTLED') // separate resource by kind // we want to send the resources that populate the main console pages first @@ -397,7 +399,7 @@ export class ServerSideEvents { // send packets of resources // with resources that fill main console pages first let sentCount = 0 - const sending = [start, settings] + const sending = [start, settings, ...throttledEvents] do { sending.push(...clusters.splice(0, 200)) sending.push(...agents.splice(0, 200)) diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index 412d1529f94..bb35b75683f 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -35,7 +35,18 @@ export interface SettingsEvent { settings: Record } -type ServerSideEventData = WatchEvent | SettingsEvent | { type: 'START' | 'LOADED' | 'EOP' } +export type ThrottledResource = { + kind: string + namespace: string + name: string +} + +export interface ThrottledEvent { + type: 'THROTTLED' + resources: ThrottledResource[] +} + +type ServerSideEventData = WatchEvent | SettingsEvent | ThrottledEvent | { type: 'START' | 'LOADED' | 'EOP' } let requests: { cancel: () => void }[] = [] @@ -47,32 +58,46 @@ let requests: { cancel: () => void }[] = [] * 1) Overwhelm the event processing loop / liveliness probe * 2) Grow client event queues until the pod OOMs * - * Detection: if the same namespace/name Policy is modified more than FLAP_THRESHOLD (N) times - * within FLAP_WINDOW_MS (M), the resource is considered flapping. + * Detection: if status.compliant changes more than FLAP_THRESHOLD (N) times within + * FLAP_WINDOW_MS (M), the resource enters polling mode. * Only Policy resources are subject to this check. - * While flapping, the resource is cached/broadcast when flapping is first detected (marked with - * `throttled: true`) and then at most once per minute (FLAP_COOLDOWN_MS) while still flapping. - * Cache suppression continues for the full cooldown even if the detection window clears before - * the minute elapses, so re-entry into flapping does not bypass the interval. + * In polling mode, updates are suppressed (return true) except once per FLAP_COOLDOWN_MS (P). + * Polling mode ends when the policy spec changes or when a silence timer (P) expires with no calls. + * While in polling mode, `resource.throttled` is set after FLAP_SETTLING_MS (S) has elapsed since + * the tracker entry was first created; it is removed when polling mode ends. * * N is initially 5 times - * M is initially 5 seconds + * M is initially 1 minute * P is initially 1 minute + * S is initially 15 minutes */ -export const FLAP_THRESHOLD = 5 // N: modifications that trigger flapping detection -export const FLAP_WINDOW_MS = 5 * 1000 // M: sliding window for counting modifications -export const FLAP_COOLDOWN_MS = 60 * 1000 // P: min interval between cache updates while flapping - -interface FlapTrackerEntry { +export const FLAP_THRESHOLD = 5 // N: compliant changes within M that trigger polling mode +export const FLAP_WINDOW_MS = 60 * 1000 // M: sliding window for counting calls +export const FLAP_COOLDOWN_MS = 2 * 60 * 1000 // P: min interval between allowed updates while polling; silence to exit polling +//export const FLAP_SETTLING_MS = 15 * 60 * 1000 // S: grace period before marking resource.throttled +export const FLAP_SETTLING_MS = 3 * 60 * 1000 // S: grace period before marking resource.throttled +const THROTTLING_CHECK_INTERVAL = 60 * 1000 + +interface FlapTrackerEntry extends ThrottledResource { timestamps: number[] - throttled: boolean + throttled?: boolean lastCachedAt: number + settling: number + polling?: boolean + lastSpec?: string } const flapTracker: Record = {} +let flappingEventID: number | undefined +let lastFlappingResourceCount: number | undefined /** Clear flap tracker state. Used for test isolation. */ export function resetFlapTracker() { + if (flappingEventID) { + ServerSideEvents.removeEvent(flappingEventID) + flappingEventID = undefined + } + lastFlappingResourceCount = undefined for (const key in flapTracker) { delete flapTracker[key] } @@ -88,88 +113,146 @@ export function resourceFlapKey( return `${resource.kind}/${resource.metadata?.namespace ?? ''}/${resource.metadata?.name ?? ''}` } +function toThrottledResource( + resource: Pick & { metadata?: { namespace?: string; name?: string } } +): ThrottledResource { + return { + kind: resource.kind, + namespace: resource.metadata?.namespace ?? '', + name: resource.metadata?.name ?? '', + } +} + export function formatFlappingMessage(kind: string, namespace: string, name: string): string { const windowMinutes = Math.max(1, Math.round(FLAP_WINDOW_MS / 60_000)) const timesPerMinute = Math.max(1, Math.round(60_000 / FLAP_COOLDOWN_MS)) return `${kind} ${name} in namespace ${namespace} has been modified more than ${FLAP_THRESHOLD} times in the last ${windowMinutes} minutes. Verify this resource is configured correctly. Updates are being limited to ${timesPerMinute} times per minute.` } -/** - * Records a modification for flap detection on Policy resources and returns whether this - * update should be throttled (suppressed). Sets `resource.throttled` to true when flapping - * is detected. Non-Policy resources are never throttled. - */ -export function shouldThrottleResource( - resource: Pick & { metadata?: { namespace?: string; name?: string } }, - now = Date.now() -): boolean { - if (resource.kind !== 'Policy') { - return false - } +type ThrottleResource = Pick & { + metadata?: { namespace?: string; name?: string } + spec?: Record + status?: { compliant?: string } +} - const key = resourceFlapKey(resource) +export function refreshThrottleStatus(resource?: ThrottleResource, now = Date.now()): boolean { + let shouldThrottle = false - let entry = flapTracker[key] - if (!entry) { - entry = { timestamps: [], throttled: false, lastCachedAt: 0 } - flapTracker[key] = entry - } + if (resource) { + const key = resourceFlapKey(resource) + + let entry = flapTracker[key] + if (!entry) { + entry = { + ...toThrottledResource(resource), + timestamps: [], + lastCachedAt: 0, + settling: now, + polling: false, + } + flapTracker[key] = entry + } + const specKey = JSON.stringify(resource.spec ?? {}) + if (entry.lastSpec !== undefined && entry.lastSpec !== specKey) { + delete entry.polling + delete entry.throttled + } else { + entry.timestamps.push(now) + entry.timestamps = entry.timestamps.filter((t) => now - t <= FLAP_WINDOW_MS) + if (entry.timestamps.length > FLAP_THRESHOLD) { + entry.polling = true + } + } + entry.lastSpec = specKey - entry.timestamps.push(now) - entry.timestamps = entry.timestamps.filter((t) => now - t <= FLAP_WINDOW_MS) + if (entry.polling) { + // Allow one update per cooldown interval; suppress all others while polling. + if (entry.lastCachedAt === 0 || now - entry.lastCachedAt >= FLAP_COOLDOWN_MS) { + entry.lastCachedAt = now + } else { + shouldThrottle = true + } + } else { + entry.lastCachedAt = 0 + } + } - const isFlapping = entry.timestamps.length > FLAP_THRESHOLD - entry.throttled = isFlapping + const pollingEntries = Object.values(flapTracker).filter((e) => e.polling) + for (const entry of pollingEntries) { + if (entry.timestamps.length > 0) { + const lastCall = entry.timestamps[entry.timestamps.length - 1] + if (now - lastCall > FLAP_COOLDOWN_MS) { + entry.polling = false + entry.lastCachedAt = 0 + } + } - if (isFlapping) { - resource.throttled = true - } else { - delete resource.throttled + if (entry.polling && now - entry.settling > FLAP_SETTLING_MS) { + entry.throttled = true + } else { + delete entry.throttled + } } - const withinCooldown = entry.lastCachedAt > 0 && now - entry.lastCachedAt < FLAP_COOLDOWN_MS - if (withinCooldown) { - return true + const throttledResources = Object.values(flapTracker) + .filter((entry) => entry.throttled === true) + .map((entry): ThrottledResource => toThrottledResource(entry)) + + if (throttledResources.length !== lastFlappingResourceCount) { + lastFlappingResourceCount = throttledResources.length + void (async () => { + if (flappingEventID) { + ServerSideEvents.removeEvent(flappingEventID) + } + flappingEventID = await ServerSideEvents.pushEvent({ + data: { + type: 'THROTTLED', + resources: throttledResources, + } satisfies ThrottledEvent, + }) + })() } - if (isFlapping) { - entry.lastCachedAt = now + return shouldThrottle +} + +export function shouldThrottleResource(resource: ThrottleResource, now = Date.now()): boolean { + if (resource.kind !== 'Policy') { return false } - - entry.lastCachedAt = 0 - return false + return refreshThrottleStatus(resource, now) } -/** - * When TEST_THROTTLING=true, synthesize rapid Policy updates so flapping throttle can be verified - * without a misconfigured cluster resource. - */ -function startTestThrottling(): void { - if (process.env.NODE_ENV !== 'production' && process.env.TEST_THROTTLING !== 'true') return - - logger.warn({ msg: 'TEST_THROTTLING enabled — synthesizing flapping Policy updates' }) - let revision = 0 - const interval = setInterval(() => { - revision += 1 - const resource: IResource & { spec: { disabled: boolean } } = { - kind: 'Policy', - apiVersion: 'policy.open-cluster-management.io/v1', - metadata: { - name: 'test-flapping-policy', - namespace: 'default', - uid: 'test-flapping-policy-uid', - resourceVersion: String(revision), - }, - spec: { - disabled: false, - }, +let throttlingCheckTimer: NodeJS.Timeout | undefined + +function startRefreshThrottleStatus(): void { + if (throttlingCheckTimer) return + + throttlingCheckTimer = setInterval(() => { + if (stopping) { + if (throttlingCheckTimer) { + clearInterval(throttlingCheckTimer) + throttlingCheckTimer = undefined + } + return } - void cacheResource(resource, true).catch((err: unknown) => { - logger.error({ msg: 'TEST_THROTTLING cacheResource failed', error: errorToString(err) }) - }) - }, 200) - interval.unref() + try { + refreshThrottleStatus() + } catch (err: unknown) { + logger.error({ msg: 'throttling check failed', error: err }) + } + }, THROTTLING_CHECK_INTERVAL) + + throttlingCheckTimer.unref() + logger.info({ msg: 'throttling check started', interval: THROTTLING_CHECK_INTERVAL }) +} + +function stopRefreshThrottleStatus(): void { + if (throttlingCheckTimer) { + clearInterval(throttlingCheckTimer) + throttlingCheckTimer = undefined + logger.info({ msg: 'throttling check stopped' }) + } } export async function getKubeResources(kind: string, apiVersion: string) { @@ -477,7 +560,7 @@ const definitions: IWatchOptions[] = [ export function startWatching(): void { ServerSideEvents.eventFilter = eventFilter startAccessCacheCleanup() - startTestThrottling() + startRefreshThrottleStatus() for (const definition of definitions) { void listAndWatch(definition) @@ -703,6 +786,14 @@ export function createWatchEventProcessor(options: IWatchOptions, url: string, r throw err } pruneResources(options, [watchEvent.object]) + // Track flapping Policy updates but skip caching/broadcasting suppressed events. + if ( + (watchEvent.type === 'ADDED' || watchEvent.type === 'MODIFIED') && + shouldThrottleResource(watchEvent.object) + ) { + callback() + return + } switch (watchEvent.type) { case 'ADDED': case 'MODIFIED': @@ -950,7 +1041,6 @@ export async function cacheResource(resource: IResource, forwardEventsToClients } const latestExisting = cache[uid] if (latestExisting === existing) { - // Decide whether to replace the broadcast event after flapping throttle check below break } // if a deleteResource ran while we were awaiting, we will exit the loop because the resource is no longer existing @@ -958,11 +1048,6 @@ export async function cacheResource(resource: IResource, forwardEventsToClients existing = latestExisting } - // Skip caching/broadcasting for throttled updates except the transition update and once per minute. - if (shouldThrottleResource(resource)) { - return resource.metadata.resourceVersion - } - const shouldForward = forwardEventsToClients if (shouldForward && existing) { const eventID = await existing.eventID @@ -1039,6 +1124,7 @@ function eventFilter(token: string, serverSideEvent: ServerSideEvent { }) describe('flapping resource throttling', () => { + const compliantValues = ['Compliant', 'NonCompliant', 'Pending'] as const + + function policyWithCompliant(name: string, namespace: string, changeIndex: number) { + return { + kind: 'Policy', + metadata: { name, namespace }, + status: { compliant: compliantValues[changeIndex % compliantValues.length] }, + } + } + beforeEach(() => { resetFlapTracker() resetResourceCache() @@ -1525,6 +1537,27 @@ describe('events Route', () => { ) }) + it('should reset flap tracker when policy spec changes', () => { + const base = Date.now() + const policy = { + ...policyWithCompliant('spec-change', 'default', 0), + spec: { disabled: false }, + } + + for (let i = 0; i <= FLAP_THRESHOLD; i++) { + shouldThrottleResource({ ...policy, status: { compliant: compliantValues[i % compliantValues.length] } }, base + i) + } + expect(getFlapTracker()['Policy/default/spec-change'].polling).toBe(true) + + expect( + shouldThrottleResource({ ...policy, spec: { disabled: true } }, base + FLAP_THRESHOLD + 1) + ).toBe(false) + const entry = getFlapTracker()['Policy/default/spec-change'] + expect(entry.polling).toBeUndefined() + expect(entry.throttled).toBeUndefined() + expect(entry.lastSpec).toBe('{"disabled":true}') + }) + it('should never throttle non-Policy kinds', () => { const now = Date.now() for (let i = 0; i < FLAP_THRESHOLD + 10; i++) { @@ -1534,38 +1567,38 @@ describe('events Route', () => { } }) - it('should only set throttled on the resource when flapping is detected', () => { + it('should only set throttled on the tracker after settling while polling', async () => { const base = Date.now() - const policy: { kind: string; metadata: { name: string; namespace: string }; throttled?: boolean } = { - kind: 'Policy', - metadata: { name: 'policy-a', namespace: 'default' }, - } - for (let i = 0; i < FLAP_THRESHOLD; i++) { - shouldThrottleResource(policy, base + i) - expect(policy.throttled).toBeUndefined() + for (let i = 0; i <= FLAP_THRESHOLD; i++) { + shouldThrottleResource(policyWithCompliant('policy-a', 'default', i), base + i) } - shouldThrottleResource(policy, base + FLAP_THRESHOLD) - expect(policy.throttled).toBe(true) + jest.spyOn(Date, 'now').mockImplementation(() => base + FLAP_THRESHOLD) + await checkThrottleStatus() + expect(getFlapTracker()['Policy/default/policy-a'].throttled).toBeUndefined() - shouldThrottleResource(policy, base + FLAP_WINDOW_MS + FLAP_THRESHOLD + 10) - expect(policy.throttled).toBeUndefined() + const afterSettling = base + FLAP_SETTLING_MS + 1 + shouldThrottleResource(policyWithCompliant('policy-a', 'default', FLAP_THRESHOLD + 1), afterSettling - 1000) + jest.spyOn(Date, 'now').mockImplementation(() => afterSettling) + await checkThrottleStatus() + expect(getFlapTracker()['Policy/default/policy-a'].throttled).toBe(true) + + jest.restoreAllMocks() }) - it('should throttle Policy updates after more than N modifications within M seconds', () => { + it('should throttle Policy updates after more than N compliant changes within M seconds', () => { const base = Date.now() - const policy = { kind: 'Policy', metadata: { name: 'flappy', namespace: 'default' } } let notThrottledCount = 0 const end = base + (FLAP_THRESHOLD + 5) * 100 - for (let t = base; t <= end; t += 100) { - if (!shouldThrottleResource(policy, t)) { + for (let i = 0, t = base; t <= end; t += 100, i++) { + if (!shouldThrottleResource(policyWithCompliant('flappy', 'default', i), t)) { notThrottledCount += 1 } } - expect(getFlapTracker()['Policy/default/flappy'].throttled).toBe(true) + expect(getFlapTracker()['Policy/default/flappy'].polling).toBe(true) // First FLAP_THRESHOLD updates, plus the transition update when flapping is detected. // Further updates within the cooldown interval are throttled. expect(notThrottledCount).toBe(FLAP_THRESHOLD + 1) @@ -1573,52 +1606,60 @@ describe('events Route', () => { it('should allow caching again after FLAP_COOLDOWN_MS while still throttled', () => { const base = Date.now() - const policy = { kind: 'Policy', metadata: { name: 'periodic', namespace: 'default' } } for (let i = 0; i <= FLAP_THRESHOLD; i++) { - shouldThrottleResource(policy, base + i) + shouldThrottleResource(policyWithCompliant('periodic', 'default', i), base + i) } - expect(getFlapTracker()['Policy/default/periodic'].throttled).toBe(true) + expect(getFlapTracker()['Policy/default/periodic'].polling).toBe(true) - expect(shouldThrottleResource(policy, base + FLAP_THRESHOLD + 100)).toBe(true) + expect( + shouldThrottleResource(policyWithCompliant('periodic', 'default', FLAP_THRESHOLD), base + FLAP_THRESHOLD + 100) + ).toBe(true) const periodicAt = base + FLAP_THRESHOLD + FLAP_COOLDOWN_MS for (let i = 1; i <= FLAP_THRESHOLD; i++) { - shouldThrottleResource(policy, periodicAt - 1000 + i * 100) + shouldThrottleResource( + policyWithCompliant('periodic', 'default', FLAP_THRESHOLD + i), + periodicAt - 1000 + i * 100 + ) } - expect(shouldThrottleResource(policy, periodicAt)).toBe(false) + expect(shouldThrottleResource(policyWithCompliant('periodic', 'default', FLAP_THRESHOLD * 2 + 1), periodicAt)).toBe( + false + ) }) it('should continue suppressing caches after detection window clears until cooldown expires', () => { const base = Date.now() - const policy: { kind: string; metadata: { name: string; namespace: string }; throttled?: boolean } = { - kind: 'Policy', - metadata: { name: 'sticky', namespace: 'default' }, - } for (let i = 0; i <= FLAP_THRESHOLD; i++) { - shouldThrottleResource(policy, base + i) + shouldThrottleResource(policyWithCompliant('sticky', 'default', i), base + i) } const afterWindowClears = base + FLAP_WINDOW_MS + FLAP_THRESHOLD + 10 - expect(shouldThrottleResource(policy, afterWindowClears)).toBe(true) - expect(policy.throttled).toBeUndefined() + expect(shouldThrottleResource(policyWithCompliant('sticky', 'default', FLAP_THRESHOLD), afterWindowClears)).toBe( + true + ) + expect(getFlapTracker()['Policy/default/sticky'].polling).toBe(true) }) - it('should stop throttling when modifications fall back and cooldown expires', () => { + it('should stop throttling when modifications fall back and cooldown expires', async () => { const base = Date.now() - const policy = { kind: 'Policy', metadata: { name: 'recovering', namespace: 'ns1' } } for (let i = 0; i <= FLAP_THRESHOLD; i++) { - shouldThrottleResource(policy, base + i) + shouldThrottleResource(policyWithCompliant('recovering', 'ns1', i), base + i) } - expect(getFlapTracker()['Policy/ns1/recovering'].throttled).toBe(true) + expect(getFlapTracker()['Policy/ns1/recovering'].polling).toBe(true) const afterCooldown = base + FLAP_THRESHOLD + FLAP_COOLDOWN_MS + 1 - expect(shouldThrottleResource(policy, afterCooldown)).toBe(false) - expect(getFlapTracker()['Policy/ns1/recovering'].throttled).toBe(false) + expect(shouldThrottleResource(policyWithCompliant('recovering', 'ns1', FLAP_THRESHOLD), afterCooldown)).toBe(false) + + jest.spyOn(Date, 'now').mockImplementation(() => afterCooldown + FLAP_COOLDOWN_MS + 1) + await checkThrottleStatus() + expect(getFlapTracker()['Policy/ns1/recovering'].polling).toBe(false) + expect(getFlapTracker()['Policy/ns1/recovering'].throttled).toBeUndefined() expect(getFlapTracker()['Policy/ns1/recovering'].lastCachedAt).toBe(0) + jest.restoreAllMocks() }) it('should cache at most once per minute while a Policy is flapping', async () => { @@ -1628,7 +1669,7 @@ describe('events Route', () => { jest.spyOn(Date, 'now').mockImplementation(() => now) for (let i = 1; i <= FLAP_THRESHOLD + 1; i++) { - await cacheResource({ + const resource = { kind: 'Policy', apiVersion: 'policy.open-cluster-management.io/v1', metadata: { @@ -1637,12 +1678,16 @@ describe('events Route', () => { uid: 'flappy-uid', resourceVersion: String(i), }, - }) + status: { compliant: compliantValues[i % compliantValues.length] }, + } + if (!shouldThrottleResource(resource)) { + await cacheResource(resource) + } now += 100 } now += 100 - await cacheResource({ + const throttledResource = { kind: 'Policy', apiVersion: 'policy.open-cluster-management.io/v1', metadata: { @@ -1651,7 +1696,11 @@ describe('events Route', () => { uid: 'flappy-uid', resourceVersion: String(FLAP_THRESHOLD + 2), }, - }) + status: { compliant: compliantValues[(FLAP_THRESHOLD + 2) % compliantValues.length] }, + } + if (!shouldThrottleResource(throttledResource)) { + await cacheResource(throttledResource) + } await new Promise((resolve) => setTimeout(resolve, 0)) for (const entry of Object.values(getEventCache())) { @@ -1664,13 +1713,13 @@ describe('events Route', () => { const periodicAt = base + FLAP_THRESHOLD * 100 + FLAP_COOLDOWN_MS for (let i = 1; i <= FLAP_THRESHOLD; i++) { shouldThrottleResource( - { kind: 'Policy', metadata: { name: 'flappy-policy', namespace: 'default' } }, + policyWithCompliant('flappy-policy', 'default', FLAP_THRESHOLD + 2 + i), periodicAt - 1000 + i * 100 ) } now = periodicAt - await cacheResource({ + const periodicResource = { kind: 'Policy', apiVersion: 'policy.open-cluster-management.io/v1', metadata: { @@ -1679,7 +1728,11 @@ describe('events Route', () => { uid: 'flappy-uid', resourceVersion: String(FLAP_THRESHOLD * 2 + 3), }, - }) + status: { compliant: compliantValues[(FLAP_THRESHOLD * 2 + 3) % compliantValues.length] }, + } + if (!shouldThrottleResource(periodicResource)) { + await cacheResource(periodicResource) + } await new Promise((resolve) => setTimeout(resolve, 0)) for (const entry of Object.values(getEventCache())) { @@ -1692,7 +1745,7 @@ describe('events Route', () => { const resources = await getKubeResources('Policy', 'policy.open-cluster-management.io/v1') expect(resources).toHaveLength(1) expect(resources[0].metadata.resourceVersion).toBe(String(FLAP_THRESHOLD * 2 + 3)) - expect(resources[0].throttled).toBe(true) + expect(getFlapTracker()['Policy/default/flappy-policy'].polling).toBe(true) jest.restoreAllMocks() pushSpy.mockRestore() diff --git a/frontend/src/atoms.ts b/frontend/src/atoms.ts index c41a423cb1f..1cc50ea329d 100644 --- a/frontend/src/atoms.ts +++ b/frontend/src/atoms.ts @@ -203,7 +203,16 @@ export interface SettingsEvent { settings: Record } -export type ServerSideEventData = WatchEvent | SettingsEvent | { type: 'START' | 'LOADED' | 'EOP' } +export interface ThrottledEvent { + type: 'THROTTLED' + resources: { + kind: string + namespace: string + name: string + }[] +} + +export type ServerSideEventData = WatchEvent | SettingsEvent | ThrottledEvent | { type: 'START' | 'LOADED' | 'EOP' } export function usePolicies() { const policies = useRecoilValue(policiesState) diff --git a/frontend/src/components/LoadData.tsx b/frontend/src/components/LoadData.tsx index 6bf2b5fc982..1c342f773e4 100644 --- a/frontend/src/components/LoadData.tsx +++ b/frontend/src/components/LoadData.tsx @@ -562,6 +562,10 @@ export function LoadData(props: { children?: ReactNode }) { } setEventsLoaded(true) break + case 'THROTTLED': + // TODO: setThrottled(data.resources) + console.log('THROTTLED', data.resources) + break case 'SETTINGS': setSettings(data.settings) break From 4faec0ad085f4422bbe1a2f6db253d2ca005766c Mon Sep 17 00:00:00 2001 From: John Swanke Date: Tue, 15 Sep 2026 14:55:53 -0400 Subject: [PATCH 09/12] fixes Signed-off-by: John Swanke --- backend/src/lib/server-side-events.ts | 6 +- backend/src/routes/events.ts | 284 ++++++++---------- backend/test/routes/events.test.ts | 200 +++++------- frontend/public/locales/en/translation.json | 2 +- frontend/src/atoms.ts | 11 +- frontend/src/components/LoadData.tsx | 4 - .../routes/Governance/common/useCustom.tsx | 2 +- .../src/routes/Governance/common/util.tsx | 5 + 8 files changed, 203 insertions(+), 311 deletions(-) diff --git a/backend/src/lib/server-side-events.ts b/backend/src/lib/server-side-events.ts index 9750eabf564..73840733644 100644 --- a/backend/src/lib/server-side-events.ts +++ b/backend/src/lib/server-side-events.ts @@ -323,15 +323,13 @@ export class ServerSideEvents { parts.push(loaded) } - // remove START, SETTINGS, THROTTLED and LOADED from events + // remove START, SETTINGS and LOADED from events const start = parts.shift() const end = parts.pop() const inx = parts.findIndex(({ data }) => { return (data as { type?: 'SETTINGS' }).type === 'SETTINGS' }) const settings = parts.splice(inx, 1)[0] - const throttledEvents = parts.filter(({ data }) => (data as { type?: string }).type === 'THROTTLED') - parts = parts.filter(({ data }) => (data as { type?: string }).type !== 'THROTTLED') // separate resource by kind // we want to send the resources that populate the main console pages first @@ -399,7 +397,7 @@ export class ServerSideEvents { // send packets of resources // with resources that fill main console pages first let sentCount = 0 - const sending = [start, settings, ...throttledEvents] + const sending = [start, settings] do { sending.push(...clusters.splice(0, 200)) sending.push(...agents.splice(0, 200)) diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index bb35b75683f..72edd693ce2 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -35,69 +35,35 @@ export interface SettingsEvent { settings: Record } -export type ThrottledResource = { - kind: string - namespace: string - name: string -} - -export interface ThrottledEvent { - type: 'THROTTLED' - resources: ThrottledResource[] -} - -type ServerSideEventData = WatchEvent | SettingsEvent | ThrottledEvent | { type: 'START' | 'LOADED' | 'EOP' } +type ServerSideEventData = WatchEvent | SettingsEvent | { type: 'START' | 'LOADED' | 'EOP' } let requests: { cancel: () => void }[] = [] /** - * Flapping resource throttling - * - * When a watched kube resource updates erroneously at high frequency (especially Policies), - * unbounded cacheResource → ServerSideEvents.pushEvent calls can: - * 1) Overwhelm the event processing loop / liveliness probe - * 2) Grow client event queues until the pod OOMs - * - * Detection: if status.compliant changes more than FLAP_THRESHOLD (N) times within - * FLAP_WINDOW_MS (M), the resource enters polling mode. - * Only Policy resources are subject to this check. - * In polling mode, updates are suppressed (return true) except once per FLAP_COOLDOWN_MS (P). - * Polling mode ends when the policy spec changes or when a silence timer (P) expires with no calls. - * While in polling mode, `resource.throttled` is set after FLAP_SETTLING_MS (S) has elapsed since - * the tracker entry was first created; it is removed when polling mode ends. - * - * N is initially 5 times - * M is initially 1 minute - * P is initially 1 minute - * S is initially 15 minutes + * Policy flap throttling: limits cache/SSE churn when Policies update too often. + * More than FLAP_THRESHOLD updates in FLAP_WINDOW_MS (after FLAP_SETTLING_MS) sets `throttled` and + * allows at most one cached update per FLAP_COOLDOWN_MS. Throttle clears on spec change or after P + * with no updates (see startMonitoringThrottled). Overridable via FLAP_* / THROTTLING_CHECK_INTERVAL env vars. */ -export const FLAP_THRESHOLD = 5 // N: compliant changes within M that trigger polling mode -export const FLAP_WINDOW_MS = 60 * 1000 // M: sliding window for counting calls -export const FLAP_COOLDOWN_MS = 2 * 60 * 1000 // P: min interval between allowed updates while polling; silence to exit polling -//export const FLAP_SETTLING_MS = 15 * 60 * 1000 // S: grace period before marking resource.throttled -export const FLAP_SETTLING_MS = 3 * 60 * 1000 // S: grace period before marking resource.throttled -const THROTTLING_CHECK_INTERVAL = 60 * 1000 - -interface FlapTrackerEntry extends ThrottledResource { +export const FLAP_THRESHOLD = Number(process.env.FLAP_THRESHOLD) || 5 // N: updates within M that trigger throttling +export const FLAP_WINDOW_MS = Number(process.env.FLAP_WINDOW_MS) || 60 * 1000 // M: sliding window for counting calls +export const FLAP_COOLDOWN_MS = Number(process.env.FLAP_COOLDOWN_MS) || 60 * 1000 // P: min interval between allowed updates; silence to exit +export const FLAP_SETTLING_MS = Number(process.env.FLAP_SETTLING_MS) || 60 * 1000 // S: grace period before marking resource.throttled +const THROTTLING_CHECK_INTERVAL = Number(process.env.THROTTLING_CHECK_INTERVAL) || 60 * 1000 + +interface FlapTrackerEntry { timestamps: number[] - throttled?: boolean lastCachedAt: number settling: number - polling?: boolean + throttled?: boolean lastSpec?: string + resource?: string } const flapTracker: Record = {} -let flappingEventID: number | undefined -let lastFlappingResourceCount: number | undefined /** Clear flap tracker state. Used for test isolation. */ -export function resetFlapTracker() { - if (flappingEventID) { - ServerSideEvents.removeEvent(flappingEventID) - flappingEventID = undefined - } - lastFlappingResourceCount = undefined +export function resetFlapTracker(): void { for (const key in flapTracker) { delete flapTracker[key] } @@ -113,14 +79,8 @@ export function resourceFlapKey( return `${resource.kind}/${resource.metadata?.namespace ?? ''}/${resource.metadata?.name ?? ''}` } -function toThrottledResource( - resource: Pick & { metadata?: { namespace?: string; name?: string } } -): ThrottledResource { - return { - kind: resource.kind, - namespace: resource.metadata?.namespace ?? '', - name: resource.metadata?.name ?? '', - } +function resourceSpecKey(resource: IResource): string { + return JSON.stringify(get(resource, 'spec') ?? {}) } export function formatFlappingMessage(kind: string, namespace: string, name: string): string { @@ -129,129 +89,127 @@ export function formatFlappingMessage(kind: string, namespace: string, name: str return `${kind} ${name} in namespace ${namespace} has been modified more than ${FLAP_THRESHOLD} times in the last ${windowMinutes} minutes. Verify this resource is configured correctly. Updates are being limited to ${timesPerMinute} times per minute.` } -type ThrottleResource = Pick & { - metadata?: { namespace?: string; name?: string } - spec?: Record - status?: { compliant?: string } +export function formatFlappingRecoveredMessage(kind: string, namespace: string, name: string): string { + return `${kind} ${name} in namespace ${namespace} is no longer being throttled; policy updates will resume normally.` } -export function refreshThrottleStatus(resource?: ThrottleResource, now = Date.now()): boolean { - let shouldThrottle = false - - if (resource) { - const key = resourceFlapKey(resource) +// +// If a watched resource has too many updates pre minute +// put it into a polling mode where it just allows one update per minute +// +export function shouldThrottleResource(resource: IResource, now = Date.now()): boolean { + if (resource.kind !== 'Policy') { + return false + } - let entry = flapTracker[key] - if (!entry) { - entry = { - ...toThrottledResource(resource), - timestamps: [], - lastCachedAt: 0, - settling: now, - polling: false, - } - flapTracker[key] = entry + // every resource is tracked + const key = resourceFlapKey(resource) + let entry = flapTracker[key] + if (!entry) { + entry = { + timestamps: [], + lastCachedAt: 0, + settling: now, } - const specKey = JSON.stringify(resource.spec ?? {}) - if (entry.lastSpec !== undefined && entry.lastSpec !== specKey) { - delete entry.polling - delete entry.throttled - } else { - entry.timestamps.push(now) - entry.timestamps = entry.timestamps.filter((t) => now - t <= FLAP_WINDOW_MS) - if (entry.timestamps.length > FLAP_THRESHOLD) { - entry.polling = true + flapTracker[key] = entry + } + + // if resource's spec has changed, immediately remove from polling mode + const specKey = resourceSpecKey(resource) + if (entry.lastSpec !== undefined && entry.lastSpec !== specKey) { + delete entry.resource + delete entry.throttled + delete entry.lastSpec + return false + } else { + // else determine if resource is flapping if it updates more then FLAP_THRESHOLD in FLAP_WINDOW_MS ms + entry.timestamps.push(now) + entry.timestamps = entry.timestamps.filter((t) => now - t <= FLAP_WINDOW_MS) + if (entry.timestamps.length > FLAP_THRESHOLD) { + // when a resource is first created, it might flap at first + // so allow a settling time before actually throttling + if (now - entry.settling > FLAP_SETTLING_MS) { + if (!entry.throttled) { + logger.warn({ + msg: formatFlappingMessage( + resource.kind, + resource.metadata?.namespace ?? '', + resource.metadata?.name ?? '' + ), + }) + } + entry.resource = JSON.stringify(resource) + resource.throttled = true + entry.throttled = true } } - entry.lastSpec = specKey + } + entry.lastSpec = specKey - if (entry.polling) { - // Allow one update per cooldown interval; suppress all others while polling. - if (entry.lastCachedAt === 0 || now - entry.lastCachedAt >= FLAP_COOLDOWN_MS) { - entry.lastCachedAt = now - } else { - shouldThrottle = true - } + // if resource is in polling mode, allow one update per cooldown interval; suppress all others while polling. + if (entry.throttled) { + // Allow one update per cooldown interval; suppress all others while polling. + if (entry.lastCachedAt === 0 || now - entry.lastCachedAt >= FLAP_COOLDOWN_MS) { + entry.lastCachedAt = now } else { - entry.lastCachedAt = 0 + return true } + } else { + entry.lastCachedAt = 0 } - const pollingEntries = Object.values(flapTracker).filter((e) => e.polling) - for (const entry of pollingEntries) { + return false +} + +// +// Periodically check resources that are throttled to +// see if they are still flapping and if not, reset +// +let monitoringThrottledTimer: NodeJS.Timeout | undefined + +export async function checkThrottleStatus(now = Date.now()): Promise { + const throttledEntries = Object.values(flapTracker).filter((e) => e.throttled) + for (const entry of throttledEntries) { if (entry.timestamps.length > 0) { const lastCall = entry.timestamps[entry.timestamps.length - 1] if (now - lastCall > FLAP_COOLDOWN_MS) { - entry.polling = false + const resource = entry.resource ? (JSON.parse(entry.resource) as IResource) : undefined + if (resource) { + await cacheResource(resource, true) + logger.warn({ + msg: formatFlappingRecoveredMessage( + resource.kind, + resource.metadata?.namespace ?? '', + resource.metadata?.name ?? '' + ), + }) + } + delete entry.resource + entry.throttled = false entry.lastCachedAt = 0 } } - - if (entry.polling && now - entry.settling > FLAP_SETTLING_MS) { - entry.throttled = true - } else { - delete entry.throttled - } - } - - const throttledResources = Object.values(flapTracker) - .filter((entry) => entry.throttled === true) - .map((entry): ThrottledResource => toThrottledResource(entry)) - - if (throttledResources.length !== lastFlappingResourceCount) { - lastFlappingResourceCount = throttledResources.length - void (async () => { - if (flappingEventID) { - ServerSideEvents.removeEvent(flappingEventID) - } - flappingEventID = await ServerSideEvents.pushEvent({ - data: { - type: 'THROTTLED', - resources: throttledResources, - } satisfies ThrottledEvent, - }) - })() - } - - return shouldThrottle -} - -export function shouldThrottleResource(resource: ThrottleResource, now = Date.now()): boolean { - if (resource.kind !== 'Policy') { - return false } - return refreshThrottleStatus(resource, now) } -let throttlingCheckTimer: NodeJS.Timeout | undefined +function startMonitoringThrottled(): void { + if (monitoringThrottledTimer) return -function startRefreshThrottleStatus(): void { - if (throttlingCheckTimer) return - - throttlingCheckTimer = setInterval(() => { - if (stopping) { - if (throttlingCheckTimer) { - clearInterval(throttlingCheckTimer) - throttlingCheckTimer = undefined - } - return - } - try { - refreshThrottleStatus() - } catch (err: unknown) { + monitoringThrottledTimer = setInterval(() => { + void checkThrottleStatus().catch((err: unknown) => { logger.error({ msg: 'throttling check failed', error: err }) - } + }) }, THROTTLING_CHECK_INTERVAL) - throttlingCheckTimer.unref() + monitoringThrottledTimer.unref() logger.info({ msg: 'throttling check started', interval: THROTTLING_CHECK_INTERVAL }) } -function stopRefreshThrottleStatus(): void { - if (throttlingCheckTimer) { - clearInterval(throttlingCheckTimer) - throttlingCheckTimer = undefined - logger.info({ msg: 'throttling check stopped' }) +function stopMonitoringThrottled(): void { + if (monitoringThrottledTimer) { + clearInterval(monitoringThrottledTimer) + monitoringThrottledTimer = undefined + logger.info({ msg: 'monitoring throttled stopped' }) } } @@ -560,7 +518,7 @@ const definitions: IWatchOptions[] = [ export function startWatching(): void { ServerSideEvents.eventFilter = eventFilter startAccessCacheCleanup() - startRefreshThrottleStatus() + startMonitoringThrottled() for (const definition of definitions) { void listAndWatch(definition) @@ -1039,26 +997,21 @@ export async function cacheResource(resource: IResource, forwardEventsToClients ) { return resource.metadata.resourceVersion } + const eventID = await existing.eventID const latestExisting = cache[uid] if (latestExisting === existing) { + // if no other cacheResource call updated the cache while we were awaiting, we can replace the cache entry and event + if (eventID > 0) ServerSideEvents.removeEvent(eventID) break } // if a deleteResource ran while we were awaiting, we will exit the loop because the resource is no longer existing // if another cacheResource call updated the cache while we were awaiting, we will check again if the resourceVersion is the same existing = latestExisting } - - const shouldForward = forwardEventsToClients - if (shouldForward && existing) { - const eventID = await existing.eventID - // if no other cacheResource call updated the cache while we were awaiting, we can replace the cache entry and event - if (cache[uid] === existing && eventID > 0) ServerSideEvents.removeEvent(eventID) - } - const compressed = deflateResource(resource, eventDict) - const eventID = shouldForward + const eventID = forwardEventsToClients ? compressed.then((compressed) => ServerSideEvents.pushEvent({ data: { type: 'MODIFIED', object: compressed } })) - : (existing?.eventID ?? NO_BROADCAST_EVENT_ID) + : NO_BROADCAST_EVENT_ID cache[uid] = { compressed, eventID } if (resource.kind === 'ManagedCluster') { @@ -1124,7 +1077,6 @@ function eventFilter(token: string, serverSideEvent: ServerSideEvent { describe('flapping resource throttling', () => { const compliantValues = ['Compliant', 'NonCompliant', 'Pending'] as const + const policyApiVersion = 'policy.open-cluster-management.io/v1' - function policyWithCompliant(name: string, namespace: string, changeIndex: number) { + function policyWithCompliant(name: string, namespace: string, changeIndex: number): IResource { return { kind: 'Policy', - metadata: { name, namespace }, + apiVersion: policyApiVersion, + metadata: { name, namespace, uid: `${namespace}-${name}` }, status: { compliant: compliantValues[changeIndex % compliantValues.length] }, + } as IResource + } + + function throttlePolicyAt(name: string, namespace: string, atTime: number) { + shouldThrottleResource(policyWithCompliant(name, namespace, 0), atTime - FLAP_SETTLING_MS - 100) + for (let i = 0; i <= FLAP_THRESHOLD; i++) { + shouldThrottleResource(policyWithCompliant(name, namespace, i + 1), atTime - (FLAP_THRESHOLD - i) * 100) } + expect(getFlapTracker()[`Policy/${namespace}/${name}`].throttled).toBe(true) } beforeEach(() => { @@ -1542,164 +1552,123 @@ describe('events Route', () => { const policy = { ...policyWithCompliant('spec-change', 'default', 0), spec: { disabled: false }, - } + } as IResource - for (let i = 0; i <= FLAP_THRESHOLD; i++) { - shouldThrottleResource({ ...policy, status: { compliant: compliantValues[i % compliantValues.length] } }, base + i) - } - expect(getFlapTracker()['Policy/default/spec-change'].polling).toBe(true) + throttlePolicyAt('spec-change', 'default', base + FLAP_SETTLING_MS + FLAP_THRESHOLD + 1) expect( - shouldThrottleResource({ ...policy, spec: { disabled: true } }, base + FLAP_THRESHOLD + 1) + shouldThrottleResource({ ...policy, spec: { disabled: true } } as IResource, base + FLAP_SETTLING_MS + FLAP_THRESHOLD + 2) ).toBe(false) const entry = getFlapTracker()['Policy/default/spec-change'] - expect(entry.polling).toBeUndefined() expect(entry.throttled).toBeUndefined() - expect(entry.lastSpec).toBe('{"disabled":true}') + expect(entry.lastSpec).toBeUndefined() }) it('should never throttle non-Policy kinds', () => { const now = Date.now() for (let i = 0; i < FLAP_THRESHOLD + 10; i++) { expect( - shouldThrottleResource({ kind: 'ManagedCluster', metadata: { name: 'cluster-a', namespace: '' } }, now + i) + shouldThrottleResource( + { + kind: 'ManagedCluster', + apiVersion: 'cluster.open-cluster-management.io/v1', + metadata: { name: 'cluster-a', namespace: '' }, + }, + now + i + ) ).toBe(false) } }) - it('should only set throttled on the tracker after settling while polling', async () => { + it('should only set throttled on the tracker after settling while polling', () => { const base = Date.now() for (let i = 0; i <= FLAP_THRESHOLD; i++) { shouldThrottleResource(policyWithCompliant('policy-a', 'default', i), base + i) + expect(getFlapTracker()['Policy/default/policy-a'].throttled).toBeUndefined() } - jest.spyOn(Date, 'now').mockImplementation(() => base + FLAP_THRESHOLD) - await checkThrottleStatus() - expect(getFlapTracker()['Policy/default/policy-a'].throttled).toBeUndefined() - const afterSettling = base + FLAP_SETTLING_MS + 1 - shouldThrottleResource(policyWithCompliant('policy-a', 'default', FLAP_THRESHOLD + 1), afterSettling - 1000) - jest.spyOn(Date, 'now').mockImplementation(() => afterSettling) - await checkThrottleStatus() + shouldThrottleResource(policyWithCompliant('policy-a', 'default', FLAP_THRESHOLD + 1), afterSettling) expect(getFlapTracker()['Policy/default/policy-a'].throttled).toBe(true) - - jest.restoreAllMocks() }) it('should throttle Policy updates after more than N compliant changes within M seconds', () => { - const base = Date.now() - - let notThrottledCount = 0 - const end = base + (FLAP_THRESHOLD + 5) * 100 - for (let i = 0, t = base; t <= end; t += 100, i++) { - if (!shouldThrottleResource(policyWithCompliant('flappy', 'default', i), t)) { - notThrottledCount += 1 - } - } + const atTime = Date.now() + throttlePolicyAt('flappy', 'default', atTime) - expect(getFlapTracker()['Policy/default/flappy'].polling).toBe(true) - // First FLAP_THRESHOLD updates, plus the transition update when flapping is detected. - // Further updates within the cooldown interval are throttled. - expect(notThrottledCount).toBe(FLAP_THRESHOLD + 1) + expect(shouldThrottleResource(policyWithCompliant('flappy', 'default', 99), atTime + 100)).toBe(true) + expect(shouldThrottleResource(policyWithCompliant('flappy', 'default', 100), atTime + FLAP_COOLDOWN_MS)).toBe( + false + ) }) it('should allow caching again after FLAP_COOLDOWN_MS while still throttled', () => { const base = Date.now() - - for (let i = 0; i <= FLAP_THRESHOLD; i++) { - shouldThrottleResource(policyWithCompliant('periodic', 'default', i), base + i) - } - expect(getFlapTracker()['Policy/default/periodic'].polling).toBe(true) + const throttledAt = base + FLAP_SETTLING_MS + FLAP_THRESHOLD * 100 + throttlePolicyAt('periodic', 'default', throttledAt) expect( - shouldThrottleResource(policyWithCompliant('periodic', 'default', FLAP_THRESHOLD), base + FLAP_THRESHOLD + 100) - ).toBe(true) - - const periodicAt = base + FLAP_THRESHOLD + FLAP_COOLDOWN_MS - for (let i = 1; i <= FLAP_THRESHOLD; i++) { shouldThrottleResource( - policyWithCompliant('periodic', 'default', FLAP_THRESHOLD + i), - periodicAt - 1000 + i * 100 + policyWithCompliant('periodic', 'default', FLAP_THRESHOLD + 2), + throttledAt + 100 ) - } + ).toBe(true) - expect(shouldThrottleResource(policyWithCompliant('periodic', 'default', FLAP_THRESHOLD * 2 + 1), periodicAt)).toBe( - false - ) + const periodicAt = throttledAt + FLAP_COOLDOWN_MS + expect( + shouldThrottleResource(policyWithCompliant('periodic', 'default', FLAP_THRESHOLD + 3), periodicAt) + ).toBe(false) }) it('should continue suppressing caches after detection window clears until cooldown expires', () => { const base = Date.now() + const throttledAt = base + FLAP_SETTLING_MS + FLAP_THRESHOLD * 100 + throttlePolicyAt('sticky', 'default', throttledAt) - for (let i = 0; i <= FLAP_THRESHOLD; i++) { - shouldThrottleResource(policyWithCompliant('sticky', 'default', i), base + i) - } - - const afterWindowClears = base + FLAP_WINDOW_MS + FLAP_THRESHOLD + 10 - expect(shouldThrottleResource(policyWithCompliant('sticky', 'default', FLAP_THRESHOLD), afterWindowClears)).toBe( - true - ) - expect(getFlapTracker()['Policy/default/sticky'].polling).toBe(true) + expect( + shouldThrottleResource(policyWithCompliant('sticky', 'default', FLAP_THRESHOLD + 2), throttledAt + 100) + ).toBe(true) + expect(getFlapTracker()['Policy/default/sticky'].throttled).toBe(true) }) it('should stop throttling when modifications fall back and cooldown expires', async () => { const base = Date.now() + const throttledAt = base + FLAP_SETTLING_MS + FLAP_THRESHOLD * 100 + throttlePolicyAt('recovering', 'ns1', throttledAt) - for (let i = 0; i <= FLAP_THRESHOLD; i++) { - shouldThrottleResource(policyWithCompliant('recovering', 'ns1', i), base + i) - } - expect(getFlapTracker()['Policy/ns1/recovering'].polling).toBe(true) - - const afterCooldown = base + FLAP_THRESHOLD + FLAP_COOLDOWN_MS + 1 - expect(shouldThrottleResource(policyWithCompliant('recovering', 'ns1', FLAP_THRESHOLD), afterCooldown)).toBe(false) - - jest.spyOn(Date, 'now').mockImplementation(() => afterCooldown + FLAP_COOLDOWN_MS + 1) - await checkThrottleStatus() - expect(getFlapTracker()['Policy/ns1/recovering'].polling).toBe(false) - expect(getFlapTracker()['Policy/ns1/recovering'].throttled).toBeUndefined() + const afterCooldown = throttledAt + FLAP_COOLDOWN_MS + 1 + await checkThrottleStatus(afterCooldown) + expect(getFlapTracker()['Policy/ns1/recovering'].throttled).toBe(false) expect(getFlapTracker()['Policy/ns1/recovering'].lastCachedAt).toBe(0) - jest.restoreAllMocks() }) it('should cache at most once per minute while a Policy is flapping', async () => { const pushSpy = jest.spyOn(ServerSideEvents, 'pushEvent') const base = 1_000_000_000_000 - let now = base - jest.spyOn(Date, 'now').mockImplementation(() => now) + const throttledAt = base + FLAP_SETTLING_MS + FLAP_THRESHOLD * 100 + let revision = 0 - for (let i = 1; i <= FLAP_THRESHOLD + 1; i++) { - const resource = { + const makeResource = (): IResource => + ({ kind: 'Policy', - apiVersion: 'policy.open-cluster-management.io/v1', + apiVersion: policyApiVersion, metadata: { name: 'flappy-policy', namespace: 'default', uid: 'flappy-uid', - resourceVersion: String(i), + resourceVersion: String(++revision), }, - status: { compliant: compliantValues[i % compliantValues.length] }, - } - if (!shouldThrottleResource(resource)) { + status: { compliant: compliantValues[revision % compliantValues.length] }, + }) as IResource + + throttlePolicyAt('flappy-policy', 'default', throttledAt) + + for (let i = 0; i <= 2; i++) { + const resource = makeResource() + if (!shouldThrottleResource(resource, throttledAt + 100 + i)) { await cacheResource(resource) } - now += 100 - } - - now += 100 - const throttledResource = { - kind: 'Policy', - apiVersion: 'policy.open-cluster-management.io/v1', - metadata: { - name: 'flappy-policy', - namespace: 'default', - uid: 'flappy-uid', - resourceVersion: String(FLAP_THRESHOLD + 2), - }, - status: { compliant: compliantValues[(FLAP_THRESHOLD + 2) % compliantValues.length] }, - } - if (!shouldThrottleResource(throttledResource)) { - await cacheResource(throttledResource) } await new Promise((resolve) => setTimeout(resolve, 0)) @@ -1708,29 +1677,11 @@ describe('events Route', () => { } let modifiedPushes = pushSpy.mock.calls.filter((call) => (call[0].data as { type?: string })?.type === 'MODIFIED') - expect(modifiedPushes.length).toBe(FLAP_THRESHOLD + 1) - - const periodicAt = base + FLAP_THRESHOLD * 100 + FLAP_COOLDOWN_MS - for (let i = 1; i <= FLAP_THRESHOLD; i++) { - shouldThrottleResource( - policyWithCompliant('flappy-policy', 'default', FLAP_THRESHOLD + 2 + i), - periodicAt - 1000 + i * 100 - ) - } + const pushesAfterFlapping = modifiedPushes.length - now = periodicAt - const periodicResource = { - kind: 'Policy', - apiVersion: 'policy.open-cluster-management.io/v1', - metadata: { - name: 'flappy-policy', - namespace: 'default', - uid: 'flappy-uid', - resourceVersion: String(FLAP_THRESHOLD * 2 + 3), - }, - status: { compliant: compliantValues[(FLAP_THRESHOLD * 2 + 3) % compliantValues.length] }, - } - if (!shouldThrottleResource(periodicResource)) { + const periodicAt = throttledAt + FLAP_COOLDOWN_MS + const periodicResource = makeResource() + if (!shouldThrottleResource(periodicResource, periodicAt)) { await cacheResource(periodicResource) } @@ -1740,14 +1691,13 @@ describe('events Route', () => { } modifiedPushes = pushSpy.mock.calls.filter((call) => (call[0].data as { type?: string })?.type === 'MODIFIED') - expect(modifiedPushes.length).toBe(FLAP_THRESHOLD + 2) + expect(modifiedPushes.length).toBe(pushesAfterFlapping + 1) - const resources = await getKubeResources('Policy', 'policy.open-cluster-management.io/v1') + const resources = await getKubeResources('Policy', policyApiVersion) expect(resources).toHaveLength(1) - expect(resources[0].metadata.resourceVersion).toBe(String(FLAP_THRESHOLD * 2 + 3)) - expect(getFlapTracker()['Policy/default/flappy-policy'].polling).toBe(true) + expect(resources[0].metadata.resourceVersion).toBe(String(revision)) + expect(getFlapTracker()['Policy/default/flappy-policy'].throttled).toBe(true) - jest.restoreAllMocks() pushSpy.mockRestore() }) }) diff --git a/frontend/public/locales/en/translation.json b/frontend/public/locales/en/translation.json index 524c9fa3efe..f9d460d2966 100644 --- a/frontend/public/locales/en/translation.json +++ b/frontend/public/locales/en/translation.json @@ -2603,7 +2603,7 @@ "policy.table.actions.inform": "Inform", "policy.table.actions.informing": "Informing", "policy.table.throttled": "Updates throttled", - "policy.table.throttled.tooltip": "This policy is updating too frequently. Policy updates are therefore being limited to once per minute. Verify this policy is configured correctly", + "policy.table.throttled.tooltip": "This policy's compliance is changing too frequently. See Policy Conflicts panel in Overview", "policy.tableHeader.name": "Policy name", "policy.violations_one": "{{count}} policy with violations", "policy.violations_other": "{{count}} policies with violations", diff --git a/frontend/src/atoms.ts b/frontend/src/atoms.ts index 1cc50ea329d..e75042e001f 100644 --- a/frontend/src/atoms.ts +++ b/frontend/src/atoms.ts @@ -203,16 +203,7 @@ export interface SettingsEvent { settings: Record } -export interface ThrottledEvent { - type: 'THROTTLED' - resources: { - kind: string - namespace: string - name: string - }[] -} - -export type ServerSideEventData = WatchEvent | SettingsEvent | ThrottledEvent | { type: 'START' | 'LOADED' | 'EOP' } +export type ServerSideEventData = WatchEvent | SettingsEvent | { type: 'START' | 'LOADED' } export function usePolicies() { const policies = useRecoilValue(policiesState) diff --git a/frontend/src/components/LoadData.tsx b/frontend/src/components/LoadData.tsx index 1c342f773e4..6bf2b5fc982 100644 --- a/frontend/src/components/LoadData.tsx +++ b/frontend/src/components/LoadData.tsx @@ -562,10 +562,6 @@ export function LoadData(props: { children?: ReactNode }) { } setEventsLoaded(true) break - case 'THROTTLED': - // TODO: setThrottled(data.resources) - console.log('THROTTLED', data.resources) - break case 'SETTINGS': setSettings(data.settings) break diff --git a/frontend/src/routes/Governance/common/useCustom.tsx b/frontend/src/routes/Governance/common/useCustom.tsx index e099a5dd339..9c82381bae8 100644 --- a/frontend/src/routes/Governance/common/useCustom.tsx +++ b/frontend/src/routes/Governance/common/useCustom.tsx @@ -18,7 +18,7 @@ export function useAddRemediationPolicies() { (p: Policy) => p.metadata.name === `${policyNamespace}.${policyName}` ) const result = cloneDeep(p) - result.remediationResult = getPolicyRemediation(p, matchedPropagated) + result.remediationResult = getPolicyRemediation(result, matchedPropagated) return result }) return resultPolicies diff --git a/frontend/src/routes/Governance/common/util.tsx b/frontend/src/routes/Governance/common/util.tsx index 0c185e58462..0d36b9a0d92 100644 --- a/frontend/src/routes/Governance/common/util.tsx +++ b/frontend/src/routes/Governance/common/util.tsx @@ -473,6 +473,11 @@ export function getPolicyRemediation(policy: Policy | undefined, propagatedPolic if (!policy) { return '' } + if (propagatedPolicies.some((propaPolicy) => propaPolicy.throttled === true)) { + policy.throttled = true + } else { + delete policy.throttled + } const templates = policy.spec['policy-templates'] let rootRA = policy.spec.remediationAction || '' let remediationAggregation = '' From dce553116dd762ffa21014dbb134b3d036d17dd2 Mon Sep 17 00:00:00 2001 From: John Swanke Date: Tue, 15 Sep 2026 16:41:27 -0400 Subject: [PATCH 10/12] text Signed-off-by: John Swanke --- frontend/public/locales/en/translation.json | 4 +- .../routes/Governance/overview/Overview.tsx | 99 ++++++++++++++++++- 2 files changed, 101 insertions(+), 2 deletions(-) diff --git a/frontend/public/locales/en/translation.json b/frontend/public/locales/en/translation.json index f9d460d2966..d07dc339283 100644 --- a/frontend/public/locales/en/translation.json +++ b/frontend/public/locales/en/translation.json @@ -2541,6 +2541,7 @@ "policies.unknown_other": "{{count}} policies with unknown status", "Policy": "Policy", "Policy annotations": "Policy annotations", + "Policy conflicts": "Policy conflicts", "Policy automation": "Policy automation", "Policy automation content": "Policy automation content", "Policy automation created": "Policy automation created", @@ -2603,7 +2604,8 @@ "policy.table.actions.inform": "Inform", "policy.table.actions.informing": "Informing", "policy.table.throttled": "Updates throttled", - "policy.table.throttled.tooltip": "This policy's compliance is changing too frequently. See Policy Conflicts panel in Overview", + "policy.table.throttled.tooltip": "This policy's compliance is changing too frequently. See Policy conflicts panel in Overview", + "policy.overview.conflicts.tooltip": "The policies in this list are changing their compliances too rapidly. This can happen if:\n\n• A user policy may have a defect.\n\n• Two or more policies are trying to enforce different values for the same resource.\n\nUntil this is resolved, these policy updates will be throttled to once a minute.", "policy.tableHeader.name": "Policy name", "policy.violations_one": "{{count}} policy with violations", "policy.violations_other": "{{count}} policies with violations", diff --git a/frontend/src/routes/Governance/overview/Overview.tsx b/frontend/src/routes/Governance/overview/Overview.tsx index d39f2e15e7c..b534c3bb987 100644 --- a/frontend/src/routes/Governance/overview/Overview.tsx +++ b/frontend/src/routes/Governance/overview/Overview.tsx @@ -8,10 +8,13 @@ import { ExpandableSection, Icon, PageSection, + Popover, + PopoverPosition, Stack, Tooltip, } from '@patternfly/react-core' import { CheckCircleIcon, ExclamationCircleIcon, ExclamationTriangleIcon } from '@patternfly/react-icons' +import { generatePath } from 'react-router' import { Fragment, useCallback, useContext, useMemo, useState } from 'react' import { AcmMasonry } from '../../../components/AcmMasonry' import { useTranslation } from '../../../lib/acm-i18next' @@ -26,7 +29,16 @@ import { import { ClusterPolicySummarySidebar } from './ClusterPolicySummarySidebar' import { useClusterViolationSummaryMap } from './ClusterViolationSummary' import { PolicySetViolationsCard } from './PolicySetViolationSummary' -import { PolicyViolationsCard, usePolicyViolationSummary, ViolationSummary } from './PolicyViolationSummary' +import { + PolicyViolationsCard, + usePolicyClusterViolationSummaryMap, + usePolicyViolationSummary, + ViolationSummary, +} from './PolicyViolationSummary' +import { useAddRemediationPolicies } from '../common/useCustom' +import { NavigationPath } from '../../../NavigationPath' +import { AcmVisitedLink } from '../../../ui-components' +import { ClusterPolicyViolationIcons2 } from '../components/ClusterPolicyViolations' import { SecurityGroupPolicySummarySidebar } from './SecurityGroupPolicySummarySidebar' import keyBy from 'lodash/keyBy' import type { TFunction } from 'i18next' @@ -73,6 +85,7 @@ export default function GovernanceOverview() { + @@ -396,6 +409,90 @@ function ClustersCard() { ) } +const policyConflictsPopoverBody = (t: TFunction) => ( + {t('policy.overview.conflicts.tooltip')} +) + +function PolicyConflictsCard() { + const { t } = useTranslation() + const policies = useAddRemediationPolicies() + const throttledPolicies = useMemo( + () => + policies + .filter((policy) => policy.throttled === true) + .sort((a, b) => compareStrings(a.metadata.name, b.metadata.name)), + [policies] + ) + const policyClusterViolationSummaryMap = usePolicyClusterViolationSummaryMap(throttledPolicies) + + if (throttledPolicies.length === 0) { + return null + } + + return ( +
+ + + + {t('Policy conflicts')} + + + + + + +
+ {throttledPolicies.map((policy) => { + const key = `${policy.metadata.namespace}.${policy.metadata.name}` + const clusterViolationSummary = policyClusterViolationSummaryMap[policy.metadata.uid ?? ''] + const policyDetailsPath = generatePath(NavigationPath.policyDetails, { + namespace: policy.metadata.namespace!, + name: policy.metadata.name!, + }) + const policyResultsPath = generatePath(NavigationPath.policyDetailsResults, { + namespace: policy.metadata.namespace ?? '', + name: policy.metadata.name ?? '', + }) + return ( + + + {policy.metadata.name} + + {clusterViolationSummary.compliant || + clusterViolationSummary.noncompliant || + clusterViolationSummary.pending || + clusterViolationSummary.unknown ? ( + + ) : ( + - + )} + + ) + })} +
+
+
+
+ ) +} + function renderClusterList( clusterList: { cluster: ManagedCluster; violations: ViolationSummary }[], onClick: (cluster: ManagedCluster, compliance: string) => void, From f09ee8210a77c35641299ab18f10f9aa699673f9 Mon Sep 17 00:00:00 2001 From: John Swanke Date: Tue, 15 Sep 2026 16:45:36 -0400 Subject: [PATCH 11/12] fix Signed-off-by: John Swanke --- backend/test/routes/events.test.ts | 16 ++++++++-------- frontend/public/locales/en/translation.json | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/backend/test/routes/events.test.ts b/backend/test/routes/events.test.ts index 3ebaf8709e1..bd759c13817 100644 --- a/backend/test/routes/events.test.ts +++ b/backend/test/routes/events.test.ts @@ -1557,7 +1557,10 @@ describe('events Route', () => { throttlePolicyAt('spec-change', 'default', base + FLAP_SETTLING_MS + FLAP_THRESHOLD + 1) expect( - shouldThrottleResource({ ...policy, spec: { disabled: true } } as IResource, base + FLAP_SETTLING_MS + FLAP_THRESHOLD + 2) + shouldThrottleResource( + { ...policy, spec: { disabled: true } } as IResource, + base + FLAP_SETTLING_MS + FLAP_THRESHOLD + 2 + ) ).toBe(false) const entry = getFlapTracker()['Policy/default/spec-change'] expect(entry.throttled).toBeUndefined() @@ -1609,16 +1612,13 @@ describe('events Route', () => { throttlePolicyAt('periodic', 'default', throttledAt) expect( - shouldThrottleResource( - policyWithCompliant('periodic', 'default', FLAP_THRESHOLD + 2), - throttledAt + 100 - ) + shouldThrottleResource(policyWithCompliant('periodic', 'default', FLAP_THRESHOLD + 2), throttledAt + 100) ).toBe(true) const periodicAt = throttledAt + FLAP_COOLDOWN_MS - expect( - shouldThrottleResource(policyWithCompliant('periodic', 'default', FLAP_THRESHOLD + 3), periodicAt) - ).toBe(false) + expect(shouldThrottleResource(policyWithCompliant('periodic', 'default', FLAP_THRESHOLD + 3), periodicAt)).toBe( + false + ) }) it('should continue suppressing caches after detection window clears until cooldown expires', () => { diff --git a/frontend/public/locales/en/translation.json b/frontend/public/locales/en/translation.json index 78c79adebd7..aa43ba5ddc1 100644 --- a/frontend/public/locales/en/translation.json +++ b/frontend/public/locales/en/translation.json @@ -2573,7 +2573,6 @@ "policies.unknown_other": "{{count}} policies with unknown status", "Policy": "Policy", "Policy annotations": "Policy annotations", - "Policy conflicts": "Policy conflicts", "Policy automation": "Policy automation", "Policy automation content": "Policy automation content", "Policy automation created": "Policy automation created", @@ -2581,6 +2580,7 @@ "Policy automation steps": "Policy automation steps", "Policy cluster violations": "Policy cluster violations", "Policy cluster violations chart": "Policy cluster violations chart", + "Policy conflicts": "Policy conflicts", "Policy content": "Policy content", "Policy created": "Policy created", "Policy details": "Policy details", @@ -2627,6 +2627,7 @@ "policy.modal.warning.pruneParameter": "Some policies have the Prune parameter set.", "policy.modal.warning.pruneParameter.deleteMessage": "Deleting this policy might delete some related objects on the managed cluster(s).", "policy.modal.warning.pruneParameter.disableMessage": "Disabling this policy might delete some related objects on the managed cluster(s).", + "policy.overview.conflicts.tooltip": "The policies in this list are changing their compliances too rapidly. This can happen if:\n\n• A user policy may have a defect.\n\n• Two or more policies are trying to enforce different values for the same resource.\n\nUntil this is resolved, these policy updates will be throttled to once a minute.", "policy.table.actionGroup.status": "Status", "policy.table.actionGroup.status.disabled": "Disabled", "policy.table.actionGroup.status.enabled": "Enabled", @@ -2641,7 +2642,6 @@ "policy.table.actions.informing": "Informing", "policy.table.throttled": "Updates throttled", "policy.table.throttled.tooltip": "This policy's compliance is changing too frequently. See Policy conflicts panel in Overview", - "policy.overview.conflicts.tooltip": "The policies in this list are changing their compliances too rapidly. This can happen if:\n\n• A user policy may have a defect.\n\n• Two or more policies are trying to enforce different values for the same resource.\n\nUntil this is resolved, these policy updates will be throttled to once a minute.", "policy.tableHeader.name": "Policy name", "policy.violations_one": "{{count}} policy with violations", "policy.violations_other": "{{count}} policies with violations", From 77464c3f4948d46c0959f09b8da10577cd12e493 Mon Sep 17 00:00:00 2001 From: John Swanke Date: Tue, 15 Sep 2026 22:17:18 -0400 Subject: [PATCH 12/12] purge Signed-off-by: John Swanke --- backend/src/routes/events.ts | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/backend/src/routes/events.ts b/backend/src/routes/events.ts index 2e483d3f0ac..b3edbcf5f58 100644 --- a/backend/src/routes/events.ts +++ b/backend/src/routes/events.ts @@ -67,12 +67,13 @@ export const FLAP_THRESHOLD = Number(process.env.FLAP_THRESHOLD) || 5 // N: upda export const FLAP_WINDOW_MS = Number(process.env.FLAP_WINDOW_MS) || 60 * 1000 // M: sliding window for counting calls export const FLAP_COOLDOWN_MS = Number(process.env.FLAP_COOLDOWN_MS) || 60 * 1000 // P: min interval between allowed updates; silence to exit export const FLAP_SETTLING_MS = Number(process.env.FLAP_SETTLING_MS) || 60 * 1000 // S: grace period before marking resource.throttled +const FLAP_TRACKER_TTL_MS = 12 * 60 * 60 * 1000 // drop tracker entry 12h after resource first seen (emerged) const THROTTLING_CHECK_INTERVAL = Number(process.env.THROTTLING_CHECK_INTERVAL) || 60 * 1000 interface FlapTrackerEntry { timestamps: number[] lastCachedAt: number - settling: number + emerged: number throttled?: boolean lastSpec?: string resource?: string @@ -127,7 +128,7 @@ export function shouldThrottleResource(resource: IResource, now = Date.now()): b entry = { timestamps: [], lastCachedAt: 0, - settling: now, + emerged: now, } flapTracker[key] = entry } @@ -146,7 +147,7 @@ export function shouldThrottleResource(resource: IResource, now = Date.now()): b if (entry.timestamps.length > FLAP_THRESHOLD) { // when a resource is first created, it might flap at first // so allow a settling time before actually throttling - if (now - entry.settling > FLAP_SETTLING_MS) { + if (now - entry.emerged > FLAP_SETTLING_MS) { if (!entry.throttled) { logger.warn({ msg: formatFlappingMessage( @@ -186,8 +187,15 @@ export function shouldThrottleResource(resource: IResource, now = Date.now()): b let monitoringThrottledTimer: NodeJS.Timeout | undefined export async function checkThrottleStatus(now = Date.now()): Promise { - const throttledEntries = Object.values(flapTracker).filter((e) => e.throttled) - for (const entry of throttledEntries) { + for (const key of Object.keys(flapTracker)) { + const entry = flapTracker[key] + if (now - entry.emerged > FLAP_TRACKER_TTL_MS) { + delete flapTracker[key] + continue + } + if (!entry.throttled) { + continue + } if (entry.timestamps.length > 0) { const lastCall = entry.timestamps[entry.timestamps.length - 1] if (now - lastCall > FLAP_COOLDOWN_MS) {