From d4d47142f5b6d9e3d44290edd918b1961b72deae Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 16 Apr 2026 17:00:27 -0400 Subject: [PATCH 1/7] feat(autoconfig): add wizard config for harvester period and max files --- locales/en/common.json | 6 + .../ContainerSelectionStep.tsx | 2 +- .../DeploymentLabelActionModal.tsx | 273 ++++++++---------- .../HarvesterConfigStep.tsx | 107 ++++++- .../LogLevelConfigStep.tsx | 2 +- .../DeploymentLabelAction/ReviewStep.tsx | 14 +- .../{envVarUtils.ts => utils.ts} | 67 +++-- .../ContainerSelectionStep.test.tsx | 12 +- .../{envVarUtils.test.ts => utils.test.ts} | 166 +++++------ 9 files changed, 364 insertions(+), 285 deletions(-) rename src/openshift/actions/DeploymentLabelAction/{envVarUtils.ts => utils.ts} (53%) rename src/openshift/test/actions/DeploymentLabelAction/{envVarUtils.test.ts => utils.test.ts} (59%) diff --git a/locales/en/common.json b/locales/en/common.json index c06919fe..fc0388b1 100644 --- a/locales/en/common.json +++ b/locales/en/common.json @@ -55,6 +55,10 @@ "DEPLOYMENT_ACTION_HARVESTER_TEMPLATE_NONE_DESC": "No automatic JFR harvesting", "DEPLOYMENT_ACTION_HARVESTER_TEMPLATE_CONTINUOUS_DESC": "Continuous JFR recording with periodic harvesting", "DEPLOYMENT_ACTION_HARVESTER_TEMPLATE_PROFILING_DESC": "Profiling template for performance analysis", + "DEPLOYMENT_ACTION_HARVESTER_PERIOD_LABEL": "Harvester Period:", + "DEPLOYMENT_ACTION_HARVESTER_PERIOD_HELPER": "How often to push JFR data to Cryostat (default: 15 minutes)", + "DEPLOYMENT_ACTION_HARVESTER_MAX_FILES_LABEL": "Harvester Max Files:", + "DEPLOYMENT_ACTION_HARVESTER_MAX_FILES_HELPER": "Maximum number of JFR files to retain locally (default: 4 files)", "DEPLOYMENT_ACTION_HARVESTER_EXIT_MAX_AGE_LABEL": "Harvester Exit Max Age:", "DEPLOYMENT_ACTION_HARVESTER_EXIT_MAX_AGE_HELPER": "Maximum age of data retained in the local recording file before it is pushed to Cryostat on shutdown (default: 5 minutes)", "DEPLOYMENT_ACTION_HARVESTER_EXIT_MAX_SIZE_LABEL": "Harvester Exit Max Size:", @@ -70,6 +74,8 @@ "DEPLOYMENT_ACTION_REVIEW_CONTAINER": "Selected Container", "DEPLOYMENT_ACTION_REVIEW_JAVA_OPTS_VAR": "Java Options Variable", "DEPLOYMENT_ACTION_REVIEW_HARVESTER": "Harvester Template", + "DEPLOYMENT_ACTION_REVIEW_HARVESTER_PERIOD": "Harvester Period", + "DEPLOYMENT_ACTION_REVIEW_HARVESTER_MAX_FILES": "Harvester Max Files", "DEPLOYMENT_ACTION_REVIEW_HARVESTER_EXIT_MAX_AGE": "Harvester Exit Max Age", "DEPLOYMENT_ACTION_REVIEW_HARVESTER_EXIT_MAX_SIZE": "Harvester Exit Max Size", "DEPLOYMENT_ACTION_REVIEW_LOG_LEVEL": "Log Level", diff --git a/src/openshift/actions/DeploymentLabelAction/ContainerSelectionStep.tsx b/src/openshift/actions/DeploymentLabelAction/ContainerSelectionStep.tsx index cc3c2b82..05854574 100644 --- a/src/openshift/actions/DeploymentLabelAction/ContainerSelectionStep.tsx +++ b/src/openshift/actions/DeploymentLabelAction/ContainerSelectionStep.tsx @@ -26,7 +26,7 @@ import { DescriptionListDescription, } from '@patternfly/react-core'; import * as React from 'react'; -import { Container, getAgentConfig, formatAgentConfig, LogLevel } from './envVarUtils'; +import { Container, getAgentConfig, formatAgentConfig, LogLevel } from './utils'; interface ContainerSelectionStepProps { containers: Container[]; diff --git a/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx b/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx index de9f959c..0f2bbb62 100644 --- a/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx +++ b/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx @@ -38,16 +38,7 @@ import { import * as React from 'react'; import { ContainerSelectionStep } from './ContainerSelectionStep'; -import { - Container, - HARVESTER_TEMPLATES, - LOG_LEVELS, - HarvesterTemplate, - LogLevel, - AGENT_ENV_VARS, - getAgentConfig, - getEnvVarIndex, -} from './envVarUtils'; +import { Container, HARVESTER_TEMPLATES, LOG_LEVELS, HarvesterTemplate, LogLevel, parseDuration } from './utils'; import { HarvesterConfigStep } from './HarvesterConfigStep'; import { InstanceSelectionStep } from './InstanceSelectionStep'; import { JavaOptsConfigStep } from './JavaOptsConfigStep'; @@ -67,6 +58,8 @@ interface WizardFormData { selectedContainerName: string; javaOptsVar: string; harvesterTemplate: HarvesterTemplate; + harvesterPeriodMs: number; + harvesterMaxFiles: number; harvesterExitMaxAgeMs: number; harvesterExitMaxSizeB: number; logLevel: LogLevel; @@ -80,6 +73,8 @@ const formDefaults: WizardFormData = { selectedContainerName: '', javaOptsVar: 'JAVA_TOOL_OPTIONS', harvesterTemplate: HARVESTER_TEMPLATES.CONTINUOUS, + harvesterPeriodMs: 900000, + harvesterMaxFiles: 4, harvesterExitMaxAgeMs: 300000, harvesterExitMaxSizeB: 20971520, logLevel: LOG_LEVELS.OFF, @@ -130,10 +125,11 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, }); const containers: Container[] = React.useMemo(() => { + const deploymentLabels = resource.spec?.template?.metadata?.labels || {}; return (resource.spec?.template?.spec?.containers || []).map((container: any) => ({ name: container.name, image: container.image, - env: container.env || [], + labels: deploymentLabels, })); }, [resource]); @@ -157,10 +153,15 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, React.useEffect(() => { if (containers.length > 0) { const firstContainer = containers[0]; - const agentConfig = getAgentConfig(firstContainer); const deploymentLabels = resource.spec?.template.metadata.labels; const logLevelFromLabel = (deploymentLabels?.['cryostat.io/log-level'] as LogLevel) || LOG_LEVELS.OFF; const javaOptsVarFromLabel = deploymentLabels?.['cryostat.io/java-options-var'] || 'JAVA_TOOL_OPTIONS'; + const harvesterTemplateFromLabel = + (deploymentLabels?.['cryostat.io/harvester-template'] as HarvesterTemplate) || HARVESTER_TEMPLATES.CONTINUOUS; + const harvesterPeriodFromLabel = deploymentLabels?.['cryostat.io/harvester-period']; + const harvesterMaxFilesFromLabel = deploymentLabels?.['cryostat.io/harvester-max-files']; + const harvesterExitMaxAgeFromLabel = deploymentLabels?.['cryostat.io/harvester-exit-max-age']; + const harvesterExitMaxSizeFromLabel = deploymentLabels?.['cryostat.io/harvester-exit-max-size']; setFormData((prev) => { const newData = { @@ -168,9 +169,11 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, selectedContainerIndex: 0, selectedContainerName: firstContainer.name, javaOptsVar: javaOptsVarFromLabel, - harvesterTemplate: agentConfig?.harvesterTemplate || HARVESTER_TEMPLATES.CONTINUOUS, - harvesterExitMaxAgeMs: agentConfig?.harvesterExitMaxAgeMs || 300000, - harvesterExitMaxSizeB: agentConfig?.harvesterExitMaxSizeB || 20971520, + harvesterTemplate: harvesterTemplateFromLabel, + harvesterPeriodMs: parseDuration(harvesterPeriodFromLabel, 900000), + harvesterMaxFiles: harvesterMaxFilesFromLabel ? parseInt(harvesterMaxFilesFromLabel, 10) : 4, + harvesterExitMaxAgeMs: parseDuration(harvesterExitMaxAgeFromLabel, 300000), + harvesterExitMaxSizeB: harvesterExitMaxSizeFromLabel ? parseInt(harvesterExitMaxSizeFromLabel, 10) : 20971520, logLevel: logLevelFromLabel, }; setInitialFormData(newData); @@ -251,56 +254,6 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, }); } - function generateEnvVarPatches(containerIndex: number): Patch[] { - const patches: Patch[] = []; - const container = containers[containerIndex]; - const basePath = `/spec/template/spec/containers/${containerIndex}/env`; - - const envVarUpdates = [ - { name: AGENT_ENV_VARS.HARVESTER_TEMPLATE, value: formData.harvesterTemplate }, - { name: AGENT_ENV_VARS.HARVESTER_EXIT_MAX_AGE_MS, value: formData.harvesterExitMaxAgeMs.toString() }, - { name: AGENT_ENV_VARS.HARVESTER_EXIT_MAX_SIZE_B, value: formData.harvesterExitMaxSizeB.toString() }, - ]; - - if (!container.env || container.env.length === 0) { - patches.push({ - op: 'add', - path: basePath, - value: [], - }); - } - - for (const envVar of envVarUpdates) { - const existingIndex = getEnvVarIndex(container, envVar.name); - - if (existingIndex !== -1) { - if (envVar.value) { - patches.push({ - op: 'replace', - path: `${basePath}/${existingIndex}/value`, - value: envVar.value, - }); - } else { - patches.push({ - op: 'remove', - path: `${basePath}/${existingIndex}`, - }); - } - } else if (envVar.value) { - patches.push({ - op: 'add', - path: `${basePath}/-`, - value: { - name: envVar.name, - value: envVar.value, - }, - }); - } - } - - return patches; - } - function addMetadataLabels(instance: K8sResourceCommon) { const patches: Patch[] = [ { @@ -323,11 +276,33 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, path: '/spec/template/metadata/labels/cryostat.io~1java-options-var', value: formData.javaOptsVar, }, + { + op: 'replace', + path: '/spec/template/metadata/labels/cryostat.io~1harvester-template', + value: formData.harvesterTemplate, + }, + { + op: 'replace', + path: '/spec/template/metadata/labels/cryostat.io~1harvester-period', + value: `${formData.harvesterPeriodMs}ms`, + }, + { + op: 'replace', + path: '/spec/template/metadata/labels/cryostat.io~1harvester-max-files', + value: formData.harvesterMaxFiles.toString(), + }, + { + op: 'replace', + path: '/spec/template/metadata/labels/cryostat.io~1harvester-exit-max-age', + value: `${formData.harvesterExitMaxAgeMs}ms`, + }, + { + op: 'replace', + path: '/spec/template/metadata/labels/cryostat.io~1harvester-exit-max-size', + value: formData.harvesterExitMaxSizeB.toString(), + }, ]; - const envVarPatches = generateEnvVarPatches(formData.selectedContainerIndex); - patches.push(...envVarPatches); - patchResource(patches); } @@ -360,33 +335,34 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, path: '/spec/template/metadata/labels/cryostat.io~1java-options-var', }); } - - // Also remove environment variables from the selected container - const container = containers[formData.selectedContainerIndex]; - const basePath = `/spec/template/spec/containers/${formData.selectedContainerIndex}/env`; - - const envVarsToRemove = [ - AGENT_ENV_VARS.HARVESTER_TEMPLATE, - AGENT_ENV_VARS.HARVESTER_EXIT_MAX_AGE_MS, - AGENT_ENV_VARS.HARVESTER_EXIT_MAX_SIZE_B, - ]; - - // Collect indices and sort in descending order to avoid index shifting issues - const indicesToRemove: number[] = []; - for (const envVarName of envVarsToRemove) { - const existingIndex = getEnvVarIndex(container, envVarName); - if (existingIndex !== -1) { - indicesToRemove.push(existingIndex); - } + if (deploymentLabels?.['cryostat.io/harvester-template']) { + patches.push({ + op: 'remove', + path: '/spec/template/metadata/labels/cryostat.io~1harvester-template', + }); } - - // Sort descending so we remove from highest index first - indicesToRemove.sort((a, b) => b - a); - - for (const index of indicesToRemove) { + if (deploymentLabels?.['cryostat.io/harvester-period']) { patches.push({ op: 'remove', - path: `${basePath}/${index}`, + path: '/spec/template/metadata/labels/cryostat.io~1harvester-period', + }); + } + if (deploymentLabels?.['cryostat.io/harvester-max-files']) { + patches.push({ + op: 'remove', + path: '/spec/template/metadata/labels/cryostat.io~1harvester-max-files', + }); + } + if (deploymentLabels?.['cryostat.io/harvester-exit-max-age']) { + patches.push({ + op: 'remove', + path: '/spec/template/metadata/labels/cryostat.io~1harvester-exit-max-age', + }); + } + if (deploymentLabels?.['cryostat.io/harvester-exit-max-size']) { + patches.push({ + op: 'remove', + path: '/spec/template/metadata/labels/cryostat.io~1harvester-exit-max-size', }); } @@ -399,6 +375,8 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, const hasInstanceChanged = formSelectValue !== initialValue || formData.cryostatInstance !== initialValue; const hasJavaOptsChanged = formData.javaOptsVar !== initialFormData.javaOptsVar; const hasHarvesterChanged = formData.harvesterTemplate !== initialFormData.harvesterTemplate; + const hasHarvesterPeriodChanged = formData.harvesterPeriodMs !== initialFormData.harvesterPeriodMs; + const hasHarvesterMaxFilesChanged = formData.harvesterMaxFiles !== initialFormData.harvesterMaxFiles; const hasHarvesterExitMaxAgeChanged = formData.harvesterExitMaxAgeMs !== initialFormData.harvesterExitMaxAgeMs; const hasHarvesterExitMaxSizeChanged = formData.harvesterExitMaxSizeB !== initialFormData.harvesterExitMaxSizeB; const hasLogLevelChanged = formData.logLevel !== initialFormData.logLevel; @@ -406,6 +384,8 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, hasInstanceChanged || hasJavaOptsChanged || hasHarvesterChanged || + hasHarvesterPeriodChanged || + hasHarvesterMaxFilesChanged || hasHarvesterExitMaxAgeChanged || hasHarvesterExitMaxSizeChanged || hasLogLevelChanged; @@ -440,6 +420,8 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, selectedContainerName: containers[0]?.name || '', javaOptsVar: 'JAVA_TOOL_OPTIONS', harvesterTemplate: HARVESTER_TEMPLATES.CONTINUOUS, + harvesterPeriodMs: 900000, + harvesterMaxFiles: 4, harvesterExitMaxAgeMs: 300000, harvesterExitMaxSizeB: 20971520, logLevel: LOG_LEVELS.OFF, @@ -461,6 +443,31 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, path: '/spec/template/metadata/labels/cryostat.io~1namespace', value: cryostatInstance.metadata?.namespace, }, + { + op: 'replace', + path: '/spec/template/metadata/labels/cryostat.io~1harvester-template', + value: quickRegisterData.harvesterTemplate, + }, + { + op: 'replace', + path: '/spec/template/metadata/labels/cryostat.io~1harvester-period', + value: `${quickRegisterData.harvesterPeriodMs}ms`, + }, + { + op: 'replace', + path: '/spec/template/metadata/labels/cryostat.io~1harvester-max-files', + value: quickRegisterData.harvesterMaxFiles.toString(), + }, + { + op: 'replace', + path: '/spec/template/metadata/labels/cryostat.io~1harvester-exit-max-age', + value: `${quickRegisterData.harvesterExitMaxAgeMs}ms`, + }, + { + op: 'replace', + path: '/spec/template/metadata/labels/cryostat.io~1harvester-exit-max-size', + value: quickRegisterData.harvesterExitMaxSizeB.toString(), + }, ]; if (cryostatInstance.metadata?.labels?.['cryostat.io/log-level']) { patches.push({ @@ -474,9 +481,6 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, path: '/spec/template/metadata/labels/cryostat.io~1java-options-var', }); } - - const envVarPatches = generateEnvVarPatchesForData(quickRegisterData); - patches.push(...envVarPatches); patchResource(patches); } @@ -487,71 +491,28 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, } }; - function generateEnvVarPatchesForData(data: WizardFormData): Patch[] { - const patches: Patch[] = []; - const container = containers[data.selectedContainerIndex]; - const basePath = `/spec/template/spec/containers/${data.selectedContainerIndex}/env`; - - const envVarUpdates = [ - { name: AGENT_ENV_VARS.HARVESTER_TEMPLATE, value: data.harvesterTemplate }, - { name: AGENT_ENV_VARS.HARVESTER_EXIT_MAX_AGE_MS, value: data.harvesterExitMaxAgeMs.toString() }, - { name: AGENT_ENV_VARS.HARVESTER_EXIT_MAX_SIZE_B, value: data.harvesterExitMaxSizeB.toString() }, - ]; - - if (!container.env || container.env.length === 0) { - patches.push({ - op: 'add', - path: basePath, - value: [], - }); - } - - for (const envVar of envVarUpdates) { - const existingIndex = getEnvVarIndex(container, envVar.name); - - if (existingIndex !== -1) { - if (envVar.value) { - patches.push({ - op: 'replace', - path: `${basePath}/${existingIndex}/value`, - value: envVar.value, - }); - } else { - patches.push({ - op: 'remove', - path: `${basePath}/${existingIndex}`, - }); - } - } else if (envVar.value) { - patches.push({ - op: 'add', - path: `${basePath}/-`, - value: { - name: envVar.name, - value: envVar.value, - }, - }); - } - } - - return patches; - } - const handleContainerChange = (index: number) => { const container = containers[index]; - const agentConfig = getAgentConfig(container); const deploymentLabels = resource.spec?.template.metadata.labels; const logLevelFromLabel = (deploymentLabels?.['cryostat.io/log-level'] as LogLevel) || LOG_LEVELS.OFF; const javaOptsVarFromLabel = deploymentLabels?.['cryostat.io/java-options-var'] || 'JAVA_TOOL_OPTIONS'; + const harvesterTemplateFromLabel = + (deploymentLabels?.['cryostat.io/harvester-template'] as HarvesterTemplate) || HARVESTER_TEMPLATES.CONTINUOUS; + const harvesterPeriodFromLabel = deploymentLabels?.['cryostat.io/harvester-period']; + const harvesterMaxFilesFromLabel = deploymentLabels?.['cryostat.io/harvester-max-files']; + const harvesterExitMaxAgeFromLabel = deploymentLabels?.['cryostat.io/harvester-exit-max-age']; + const harvesterExitMaxSizeFromLabel = deploymentLabels?.['cryostat.io/harvester-exit-max-size']; setFormData((prev) => ({ ...prev, selectedContainerIndex: index, selectedContainerName: container.name, javaOptsVar: javaOptsVarFromLabel, - harvesterTemplate: agentConfig?.harvesterTemplate || HARVESTER_TEMPLATES.CONTINUOUS, - harvesterExitMaxAgeMs: agentConfig?.harvesterExitMaxAgeMs || 300000, - harvesterExitMaxSizeB: agentConfig?.harvesterExitMaxSizeB || 20971520, + harvesterTemplate: harvesterTemplateFromLabel, + harvesterPeriodMs: parseDuration(harvesterPeriodFromLabel, 900000), + harvesterMaxFiles: harvesterMaxFilesFromLabel ? parseInt(harvesterMaxFilesFromLabel, 10) : 4, + harvesterExitMaxAgeMs: parseDuration(harvesterExitMaxAgeFromLabel, 300000), + harvesterExitMaxSizeB: harvesterExitMaxSizeFromLabel ? parseInt(harvesterExitMaxSizeFromLabel, 10) : 20971520, logLevel: logLevelFromLabel, })); }; @@ -560,10 +521,18 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, setFormData((prev) => ({ ...prev, javaOptsVar: value })); }; - const handleHarvesterChange = (template: HarvesterTemplate, maxAge: number, maxSize: number) => { + const handleHarvesterChange = ( + template: HarvesterTemplate, + periodMs: number, + maxFiles: number, + maxAge: number, + maxSize: number, + ) => { setFormData((prev) => ({ ...prev, harvesterTemplate: template, + harvesterPeriodMs: periodMs, + harvesterMaxFiles: maxFiles, harvesterExitMaxAgeMs: maxAge, harvesterExitMaxSizeB: maxSize, })); @@ -666,6 +635,8 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, > = ({ kind, selectedContainer={selectedContainer} javaOptsVar={formData.javaOptsVar} harvesterTemplate={formData.harvesterTemplate} + harvesterPeriodMs={formData.harvesterPeriodMs} + harvesterMaxFiles={formData.harvesterMaxFiles} harvesterExitMaxAgeMs={formData.harvesterExitMaxAgeMs} harvesterExitMaxSizeB={formData.harvesterExitMaxSizeB} logLevel={formData.logLevel} diff --git a/src/openshift/actions/DeploymentLabelAction/HarvesterConfigStep.tsx b/src/openshift/actions/DeploymentLabelAction/HarvesterConfigStep.tsx index 981c50cf..cf9205a1 100644 --- a/src/openshift/actions/DeploymentLabelAction/HarvesterConfigStep.tsx +++ b/src/openshift/actions/DeploymentLabelAction/HarvesterConfigStep.tsx @@ -28,20 +28,24 @@ import { InputGroupItem, } from '@patternfly/react-core'; import * as React from 'react'; -import { HARVESTER_TEMPLATES, HarvesterTemplate } from './envVarUtils'; +import { HARVESTER_TEMPLATES, HarvesterTemplate } from './utils'; type TimeUnit = 'ms' | 's' | 'm' | 'h'; type SizeUnit = 'B' | 'KiB' | 'MiB' | 'GiB'; interface HarvesterConfigStepProps { harvesterTemplate: HarvesterTemplate; + harvesterPeriodMs: number; + harvesterMaxFiles: number; harvesterExitMaxAgeMs: number; harvesterExitMaxSizeB: number; - onChange: (template: HarvesterTemplate, maxAge: number, maxSize: number) => void; + onChange: (template: HarvesterTemplate, periodMs: number, maxFiles: number, maxAge: number, maxSize: number) => void; } export const HarvesterConfigStep: React.FC = ({ harvesterTemplate, + harvesterPeriodMs, + harvesterMaxFiles, harvesterExitMaxAgeMs, harvesterExitMaxSizeB, onChange, @@ -53,8 +57,6 @@ export const HarvesterConfigStep: React.FC = ({ const timeUnits: Array<{ divisor: number; unit: TimeUnit }> = [ { divisor: 60 * 60 * 1000, unit: 'h' }, { divisor: 60 * 1000, unit: 'm' }, - { divisor: 1000, unit: 's' }, - { divisor: 1, unit: 'ms' }, ]; for (const { divisor, unit } of timeUnits) { @@ -62,7 +64,8 @@ export const HarvesterConfigStep: React.FC = ({ return { value: ms / divisor, unit }; } } - return { value: ms, unit: 'ms' }; + // Default to minutes for period display + return { value: ms / (60 * 1000), unit: 'm' }; }; // Convert bytes to display value and unit @@ -82,10 +85,14 @@ export const HarvesterConfigStep: React.FC = ({ return { value: bytes, unit: 'B' }; }; + const [periodDisplay, setPeriodDisplay] = React.useState(() => getTimeDisplayValue(harvesterPeriodMs)); const [timeDisplay, setTimeDisplay] = React.useState(() => getTimeDisplayValue(harvesterExitMaxAgeMs)); const [sizeDisplay, setSizeDisplay] = React.useState(() => getSizeDisplayValue(harvesterExitMaxSizeB)); - // Update display when props change + React.useEffect(() => { + setPeriodDisplay(getTimeDisplayValue(harvesterPeriodMs)); + }, [harvesterPeriodMs]); + React.useEffect(() => { setTimeDisplay(getTimeDisplayValue(harvesterExitMaxAgeMs)); }, [harvesterExitMaxAgeMs]); @@ -95,7 +102,7 @@ export const HarvesterConfigStep: React.FC = ({ }, [harvesterExitMaxSizeB]); const handleTemplateChange = (template: HarvesterTemplate) => { - onChange(template, harvesterExitMaxAgeMs, harvesterExitMaxSizeB); + onChange(template, harvesterPeriodMs, harvesterMaxFiles, harvesterExitMaxAgeMs, harvesterExitMaxSizeB); }; const convertTimeToMs = (value: number, unit: TimeUnit): number => { @@ -104,10 +111,8 @@ export const HarvesterConfigStep: React.FC = ({ return value * 60 * 60 * 1000; case 'm': return value * 60 * 1000; - case 's': - return value * 1000; default: - return value; + return value * 60 * 1000; // Default to minutes } }; @@ -124,30 +129,47 @@ export const HarvesterConfigStep: React.FC = ({ } }; + const handlePeriodValueChange = (value: number) => { + setPeriodDisplay((prev) => ({ ...prev, value })); + const ms = convertTimeToMs(value, periodDisplay.unit); + onChange(harvesterTemplate, ms, harvesterMaxFiles, harvesterExitMaxAgeMs, harvesterExitMaxSizeB); + }; + + const handlePeriodUnitChange = (_event: React.FormEvent, unit: string) => { + const newUnit = unit as TimeUnit; + setPeriodDisplay((prev) => ({ ...prev, unit: newUnit })); + const ms = convertTimeToMs(periodDisplay.value, newUnit); + onChange(harvesterTemplate, ms, harvesterMaxFiles, harvesterExitMaxAgeMs, harvesterExitMaxSizeB); + }; + + const handleMaxFilesChange = (value: number) => { + onChange(harvesterTemplate, harvesterPeriodMs, value, harvesterExitMaxAgeMs, harvesterExitMaxSizeB); + }; + const handleTimeValueChange = (value: number) => { setTimeDisplay((prev) => ({ ...prev, value })); const ms = convertTimeToMs(value, timeDisplay.unit); - onChange(harvesterTemplate, ms, harvesterExitMaxSizeB); + onChange(harvesterTemplate, harvesterPeriodMs, harvesterMaxFiles, ms, harvesterExitMaxSizeB); }; const handleTimeUnitChange = (_event: React.FormEvent, unit: string) => { const newUnit = unit as TimeUnit; setTimeDisplay((prev) => ({ ...prev, unit: newUnit })); const ms = convertTimeToMs(timeDisplay.value, newUnit); - onChange(harvesterTemplate, ms, harvesterExitMaxSizeB); + onChange(harvesterTemplate, harvesterPeriodMs, harvesterMaxFiles, ms, harvesterExitMaxSizeB); }; const handleSizeValueChange = (value: number) => { setSizeDisplay((prev) => ({ ...prev, value })); const bytes = convertSizeToBytes(value, sizeDisplay.unit); - onChange(harvesterTemplate, harvesterExitMaxAgeMs, bytes); + onChange(harvesterTemplate, harvesterPeriodMs, harvesterMaxFiles, harvesterExitMaxAgeMs, bytes); }; const handleSizeUnitChange = (_event: React.FormEvent, unit: string) => { const newUnit = unit as SizeUnit; setSizeDisplay((prev) => ({ ...prev, unit: newUnit })); const bytes = convertSizeToBytes(sizeDisplay.value, newUnit); - onChange(harvesterTemplate, harvesterExitMaxAgeMs, bytes); + onChange(harvesterTemplate, harvesterPeriodMs, harvesterMaxFiles, harvesterExitMaxAgeMs, bytes); }; return ( @@ -178,6 +200,63 @@ export const HarvesterConfigStep: React.FC = ({ onChange={() => handleTemplateChange(HARVESTER_TEMPLATES.PROFILING)} /> + + + + handlePeriodValueChange(Math.max(0, periodDisplay.value - 1))} + onPlus={() => handlePeriodValueChange(periodDisplay.value + 1)} + onChange={(event) => { + const value = Number((event.target as HTMLInputElement).value); + if (!isNaN(value) && value >= 0) { + handlePeriodValueChange(value); + } + }} + min={0} + widthChars={10} + /> + + + + + + + + + + + {t('DEPLOYMENT_ACTION_HARVESTER_PERIOD_HELPER')} + + + + + handleMaxFilesChange(Math.max(1, harvesterMaxFiles - 1))} + onPlus={() => handleMaxFilesChange(harvesterMaxFiles + 1)} + onChange={(event) => { + const value = Number((event.target as HTMLInputElement).value); + if (!isNaN(value) && value >= 1) { + handleMaxFilesChange(value); + } + }} + min={1} + widthChars={10} + /> + + + {t('DEPLOYMENT_ACTION_HARVESTER_MAX_FILES_HELPER')} + + + diff --git a/src/openshift/actions/DeploymentLabelAction/LogLevelConfigStep.tsx b/src/openshift/actions/DeploymentLabelAction/LogLevelConfigStep.tsx index 0423692a..341a9bf9 100644 --- a/src/openshift/actions/DeploymentLabelAction/LogLevelConfigStep.tsx +++ b/src/openshift/actions/DeploymentLabelAction/LogLevelConfigStep.tsx @@ -16,7 +16,7 @@ import { useCryostatTranslation } from '@i18n/i18nextUtil'; import { Form, FormGroup, Radio } from '@patternfly/react-core'; import * as React from 'react'; -import { LOG_LEVELS, LogLevel } from './envVarUtils'; +import { LOG_LEVELS, LogLevel } from './utils'; interface LogLevelConfigStepProps { logLevel: LogLevel; diff --git a/src/openshift/actions/DeploymentLabelAction/ReviewStep.tsx b/src/openshift/actions/DeploymentLabelAction/ReviewStep.tsx index d31ee8d4..04e37000 100644 --- a/src/openshift/actions/DeploymentLabelAction/ReviewStep.tsx +++ b/src/openshift/actions/DeploymentLabelAction/ReviewStep.tsx @@ -23,12 +23,14 @@ import { DescriptionListDescription, } from '@patternfly/react-core'; import * as React from 'react'; -import { Container, HarvesterTemplate, LogLevel } from './envVarUtils'; +import { Container, HarvesterTemplate, LogLevel } from './utils'; interface ReviewStepProps { selectedInstance: K8sResourceKind | null; selectedContainer: Container | null; harvesterTemplate: HarvesterTemplate; + harvesterPeriodMs: number; + harvesterMaxFiles: number; harvesterExitMaxAgeMs: number; harvesterExitMaxSizeB: number; logLevel: LogLevel; @@ -39,6 +41,8 @@ export const ReviewStep: React.FC = ({ selectedInstance, selectedContainer, harvesterTemplate, + harvesterPeriodMs, + harvesterMaxFiles, harvesterExitMaxAgeMs, harvesterExitMaxSizeB, logLevel, @@ -77,6 +81,14 @@ export const ReviewStep: React.FC = ({ {t('DEPLOYMENT_ACTION_REVIEW_HARVESTER')} {getHarvesterDisplayName(harvesterTemplate)} + + {t('DEPLOYMENT_ACTION_REVIEW_HARVESTER_PERIOD')} + {formatDuration(harvesterPeriodMs, 1)} + + + {t('DEPLOYMENT_ACTION_REVIEW_HARVESTER_MAX_FILES')} + {harvesterMaxFiles} + {t('DEPLOYMENT_ACTION_REVIEW_HARVESTER_EXIT_MAX_AGE')} {formatDuration(harvesterExitMaxAgeMs, 1)} diff --git a/src/openshift/actions/DeploymentLabelAction/envVarUtils.ts b/src/openshift/actions/DeploymentLabelAction/utils.ts similarity index 53% rename from src/openshift/actions/DeploymentLabelAction/envVarUtils.ts rename to src/openshift/actions/DeploymentLabelAction/utils.ts index 6572fcfb..a16e3907 100644 --- a/src/openshift/actions/DeploymentLabelAction/envVarUtils.ts +++ b/src/openshift/actions/DeploymentLabelAction/utils.ts @@ -14,26 +14,12 @@ * limitations under the License. */ -export interface EnvVar { - name: string; - value?: string; - valueFrom?: unknown; -} - export interface Container { name: string; image: string; - env?: EnvVar[]; + labels?: Record; } -export const AGENT_ENV_VARS = { - HARVESTER_TEMPLATE: 'CRYOSTAT_AGENT_HARVESTER_TEMPLATE', - HARVESTER_PERIOD_MS: 'CRYOSTAT_AGENT_HARVESTER_PERIOD_MS', - HARVESTER_MAX_FILES: 'CRYOSTAT_AGENT_HARVESTER_MAX_FILES', - HARVESTER_EXIT_MAX_AGE_MS: 'CRYOSTAT_AGENT_HARVESTER_EXIT_MAX_AGE_MS', - HARVESTER_EXIT_MAX_SIZE_B: 'CRYOSTAT_AGENT_HARVESTER_EXIT_MAX_SIZE_B', -} as const; - export const HARVESTER_TEMPLATES = { NONE: '', CONTINUOUS: 'Continuous', @@ -54,27 +40,52 @@ export type LogLevel = (typeof LOG_LEVELS)[keyof typeof LOG_LEVELS]; export interface AgentConfig { harvesterTemplate: HarvesterTemplate; + harvesterPeriodMs: number; + harvesterMaxFiles: number; harvesterExitMaxAgeMs: number; harvesterExitMaxSizeB: number; } -export function findEnvVar(container: Container, envVarName: string): EnvVar | undefined { - return container.env?.find((env) => env.name === envVarName); +export function parseDuration(duration: string | undefined, defaultValue: number): number { + if (!duration) return defaultValue; + const match = duration.match(/^(\d+)(ms|s|m|h)?$/); + if (!match) return defaultValue; + const value = parseInt(match[1], 10); + const unit = match[2] || 'ms'; + switch (unit) { + case 'h': + return value * 60 * 60 * 1000; + case 'm': + return value * 60 * 1000; + case 's': + return value * 1000; + default: + return value; + } } export function getAgentConfig(container: Container): AgentConfig | null { - const harvesterTemplateVar = findEnvVar(container, AGENT_ENV_VARS.HARVESTER_TEMPLATE); - const harvesterExitMaxAgeVar = findEnvVar(container, AGENT_ENV_VARS.HARVESTER_EXIT_MAX_AGE_MS); - const harvesterExitMaxSizeVar = findEnvVar(container, AGENT_ENV_VARS.HARVESTER_EXIT_MAX_SIZE_B); + const labels = container.labels; + if (!labels) { + return null; + } + + const harvesterTemplate = labels['cryostat.io/harvester-template']; + const harvesterPeriod = labels['cryostat.io/harvester-period']; + const harvesterMaxFiles = labels['cryostat.io/harvester-max-files']; + const harvesterExitMaxAge = labels['cryostat.io/harvester-exit-max-age']; + const harvesterExitMaxSize = labels['cryostat.io/harvester-exit-max-size']; - if (!harvesterTemplateVar && !harvesterExitMaxAgeVar && !harvesterExitMaxSizeVar) { + if (!harvesterTemplate && !harvesterPeriod && !harvesterMaxFiles && !harvesterExitMaxAge && !harvesterExitMaxSize) { return null; } return { - harvesterTemplate: (harvesterTemplateVar?.value as HarvesterTemplate) || HARVESTER_TEMPLATES.NONE, - harvesterExitMaxAgeMs: harvesterExitMaxAgeVar?.value ? parseInt(harvesterExitMaxAgeVar.value, 10) : 300000, - harvesterExitMaxSizeB: harvesterExitMaxSizeVar?.value ? parseInt(harvesterExitMaxSizeVar.value, 10) : 20971520, + harvesterTemplate: (harvesterTemplate as HarvesterTemplate) || HARVESTER_TEMPLATES.NONE, + harvesterPeriodMs: parseDuration(harvesterPeriod, 900000), + harvesterMaxFiles: harvesterMaxFiles ? parseInt(harvesterMaxFiles, 10) : 4, + harvesterExitMaxAgeMs: parseDuration(harvesterExitMaxAge, 300000), + harvesterExitMaxSizeB: harvesterExitMaxSize ? parseInt(harvesterExitMaxSize, 10) : 20971520, }; } @@ -96,11 +107,3 @@ export function formatAgentConfig(config: AgentConfig | null, logLevel?: LogLeve return parts.length > 0 ? parts.join(', ') : 'None'; } - -export function getEnvVarIndex(container: Container, envVarName: string): number { - return container.env?.findIndex((env) => env.name === envVarName) ?? -1; -} - -export function hasEnvVar(container: Container, envVarName: string): boolean { - return getEnvVarIndex(container, envVarName) !== -1; -} diff --git a/src/openshift/test/actions/DeploymentLabelAction/ContainerSelectionStep.test.tsx b/src/openshift/test/actions/DeploymentLabelAction/ContainerSelectionStep.test.tsx index 2c3b97d8..dfbb0c61 100644 --- a/src/openshift/test/actions/DeploymentLabelAction/ContainerSelectionStep.test.tsx +++ b/src/openshift/test/actions/DeploymentLabelAction/ContainerSelectionStep.test.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ import { ContainerSelectionStep } from '@console-plugin/actions/DeploymentLabelAction/ContainerSelectionStep'; -import { Container, AGENT_ENV_VARS, LOG_LEVELS } from '@console-plugin/actions/DeploymentLabelAction/envVarUtils'; +import { Container, LOG_LEVELS } from '@console-plugin/actions/DeploymentLabelAction/utils'; import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import '@testing-library/jest-dom'; @@ -32,7 +32,7 @@ describe('ContainerSelectionStep', () => { { name: 'app-container', image: 'quay.io/app:latest', - env: [], + labels: {}, }, ]; @@ -40,17 +40,19 @@ describe('ContainerSelectionStep', () => { { name: 'app-container', image: 'quay.io/app:latest', - env: [{ name: AGENT_ENV_VARS.HARVESTER_TEMPLATE, value: 'Continuous' }], + labels: { + 'cryostat.io/harvester-template': 'Continuous', + }, }, { name: 'sidecar-container', image: 'quay.io/sidecar:v1', - env: [], + labels: {}, }, { name: 'worker-container', image: 'quay.io/worker:latest', - env: [], + labels: {}, }, ]; diff --git a/src/openshift/test/actions/DeploymentLabelAction/envVarUtils.test.ts b/src/openshift/test/actions/DeploymentLabelAction/utils.test.ts similarity index 59% rename from src/openshift/test/actions/DeploymentLabelAction/envVarUtils.test.ts rename to src/openshift/test/actions/DeploymentLabelAction/utils.test.ts index e931a605..a157fac6 100644 --- a/src/openshift/test/actions/DeploymentLabelAction/envVarUtils.test.ts +++ b/src/openshift/test/actions/DeploymentLabelAction/utils.test.ts @@ -15,78 +15,100 @@ */ import { Container, - AGENT_ENV_VARS, HARVESTER_TEMPLATES, LOG_LEVELS, - findEnvVar, getAgentConfig, formatAgentConfig, - getEnvVarIndex, - hasEnvVar, -} from '@console-plugin/actions/DeploymentLabelAction/envVarUtils'; +} from '@console-plugin/actions/DeploymentLabelAction/utils'; describe('envVarUtils', () => { const mockContainerWithConfig: Container = { name: 'app-container', image: 'quay.io/app:latest', - env: [ - { name: AGENT_ENV_VARS.HARVESTER_TEMPLATE, value: 'Continuous' }, - { name: 'OTHER_VAR', value: 'other-value' }, - ], + labels: { + 'cryostat.io/harvester-template': 'Continuous', + }, }; const mockContainerWithoutConfig: Container = { name: 'app-container', image: 'quay.io/app:latest', - env: [{ name: 'OTHER_VAR', value: 'other-value' }], + labels: {}, }; - const mockContainerNoEnv: Container = { + const mockContainerNoLabels: Container = { name: 'app-container', image: 'quay.io/app:latest', }; - describe('findEnvVar', () => { - it('should find an existing environment variable', () => { - const result = findEnvVar(mockContainerWithConfig, AGENT_ENV_VARS.HARVESTER_TEMPLATE); - expect(result).toEqual({ name: AGENT_ENV_VARS.HARVESTER_TEMPLATE, value: 'Continuous' }); - }); - - it('should return undefined for non-existent environment variable', () => { - const result = findEnvVar(mockContainerWithoutConfig, AGENT_ENV_VARS.HARVESTER_TEMPLATE); - expect(result).toBeUndefined(); - }); - - it('should return undefined for container without env array', () => { - const result = findEnvVar(mockContainerNoEnv, AGENT_ENV_VARS.HARVESTER_TEMPLATE); - expect(result).toBeUndefined(); - }); - }); - describe('getAgentConfig', () => { - it('should return agent config when env vars are present', () => { + it('should return agent config when labels are present', () => { const result = getAgentConfig(mockContainerWithConfig); expect(result).toEqual({ harvesterTemplate: 'Continuous', + harvesterPeriodMs: 900000, + harvesterMaxFiles: 4, harvesterExitMaxAgeMs: 300000, harvesterExitMaxSizeB: 20971520, }); }); - it('should return null when no agent env vars are present', () => { + it('should return null when no agent labels are present', () => { const result = getAgentConfig(mockContainerWithoutConfig); expect(result).toBeNull(); }); - it('should return config with defaults when only one env var is present', () => { + it('should return config with defaults when only one label is present', () => { const containerWithPartialConfig: Container = { name: 'app-container', image: 'quay.io/app:latest', - env: [{ name: AGENT_ENV_VARS.HARVESTER_TEMPLATE, value: 'Profiling' }], + labels: { + 'cryostat.io/harvester-template': 'Profiling', + }, }; const result = getAgentConfig(containerWithPartialConfig); expect(result).toEqual({ harvesterTemplate: 'Profiling', + harvesterPeriodMs: 900000, + harvesterMaxFiles: 4, + harvesterExitMaxAgeMs: 300000, + harvesterExitMaxSizeB: 20971520, + }); + }); + + it('should parse custom harvester period when specified', () => { + const containerWithCustomPeriod: Container = { + name: 'app-container', + image: 'quay.io/app:latest', + labels: { + 'cryostat.io/harvester-template': 'Continuous', + 'cryostat.io/harvester-period': '600000ms', + }, + }; + const result = getAgentConfig(containerWithCustomPeriod); + expect(result).toEqual({ + harvesterTemplate: 'Continuous', + harvesterPeriodMs: 600000, + harvesterMaxFiles: 4, + harvesterExitMaxAgeMs: 300000, + harvesterExitMaxSizeB: 20971520, + }); + }); + + it('should parse custom harvester max files when specified', () => { + const containerWithCustomMaxFiles: Container = { + name: 'app-container', + image: 'quay.io/app:latest', + labels: { + 'cryostat.io/harvester-template': 'Continuous', + 'cryostat.io/harvester-max-files': '8', + }, + }; + const result = getAgentConfig(containerWithCustomMaxFiles); + expect(result).toEqual({ + harvesterTemplate: 'Continuous', + harvesterPeriodMs: 900000, + harvesterMaxFiles: 8, harvesterExitMaxAgeMs: 300000, harvesterExitMaxSizeB: 20971520, }); @@ -96,14 +118,16 @@ describe('envVarUtils', () => { const containerWithCustomAge: Container = { name: 'app-container', image: 'quay.io/app:latest', - env: [ - { name: AGENT_ENV_VARS.HARVESTER_TEMPLATE, value: 'Continuous' }, - { name: AGENT_ENV_VARS.HARVESTER_EXIT_MAX_AGE_MS, value: '60000' }, - ], + labels: { + 'cryostat.io/harvester-template': 'Continuous', + 'cryostat.io/harvester-exit-max-age': '60000ms', + }, }; const result = getAgentConfig(containerWithCustomAge); expect(result).toEqual({ harvesterTemplate: 'Continuous', + harvesterPeriodMs: 900000, + harvesterMaxFiles: 4, harvesterExitMaxAgeMs: 60000, harvesterExitMaxSizeB: 20971520, }); @@ -113,14 +137,16 @@ describe('envVarUtils', () => { const containerWithCustomSize: Container = { name: 'app-container', image: 'quay.io/app:latest', - env: [ - { name: AGENT_ENV_VARS.HARVESTER_TEMPLATE, value: 'Continuous' }, - { name: AGENT_ENV_VARS.HARVESTER_EXIT_MAX_SIZE_B, value: '52428800' }, - ], + labels: { + 'cryostat.io/harvester-template': 'Continuous', + 'cryostat.io/harvester-exit-max-size': '52428800', + }, }; const result = getAgentConfig(containerWithCustomSize); expect(result).toEqual({ harvesterTemplate: 'Continuous', + harvesterPeriodMs: 900000, + harvesterMaxFiles: 4, harvesterExitMaxAgeMs: 300000, harvesterExitMaxSizeB: 52428800, }); @@ -130,22 +156,26 @@ describe('envVarUtils', () => { const containerWithAllCustom: Container = { name: 'app-container', image: 'quay.io/app:latest', - env: [ - { name: AGENT_ENV_VARS.HARVESTER_TEMPLATE, value: 'Profiling' }, - { name: AGENT_ENV_VARS.HARVESTER_EXIT_MAX_AGE_MS, value: '45000' }, - { name: AGENT_ENV_VARS.HARVESTER_EXIT_MAX_SIZE_B, value: '31457280' }, - ], + labels: { + 'cryostat.io/harvester-template': 'Profiling', + 'cryostat.io/harvester-period': '1200000ms', + 'cryostat.io/harvester-max-files': '6', + 'cryostat.io/harvester-exit-max-age': '45000ms', + 'cryostat.io/harvester-exit-max-size': '31457280', + }, }; const result = getAgentConfig(containerWithAllCustom); expect(result).toEqual({ harvesterTemplate: 'Profiling', + harvesterPeriodMs: 1200000, + harvesterMaxFiles: 6, harvesterExitMaxAgeMs: 45000, harvesterExitMaxSizeB: 31457280, }); }); - it('should return null for container without env array', () => { - const result = getAgentConfig(mockContainerNoEnv); + it('should return null for container without labels', () => { + const result = getAgentConfig(mockContainerNoLabels); expect(result).toBeNull(); }); }); @@ -154,6 +184,8 @@ describe('envVarUtils', () => { it('should format config with harvester and log level', () => { const config = { harvesterTemplate: HARVESTER_TEMPLATES.CONTINUOUS, + harvesterPeriodMs: 900000, + harvesterMaxFiles: 4, harvesterExitMaxAgeMs: 300000, harvesterExitMaxSizeB: 20971520, }; @@ -164,6 +196,8 @@ describe('envVarUtils', () => { it('should format config with only harvester template', () => { const config = { harvesterTemplate: HARVESTER_TEMPLATES.PROFILING, + harvesterPeriodMs: 900000, + harvesterMaxFiles: 4, harvesterExitMaxAgeMs: 300000, harvesterExitMaxSizeB: 20971520, }; @@ -179,6 +213,8 @@ describe('envVarUtils', () => { it('should format config with custom Java opts var', () => { const config = { harvesterTemplate: HARVESTER_TEMPLATES.CONTINUOUS, + harvesterPeriodMs: 900000, + harvesterMaxFiles: 4, harvesterExitMaxAgeMs: 300000, harvesterExitMaxSizeB: 20971520, }; @@ -194,6 +230,8 @@ describe('envVarUtils', () => { it('should return "None" for empty config', () => { const config = { harvesterTemplate: '' as any, + harvesterPeriodMs: 900000, + harvesterMaxFiles: 4, harvesterExitMaxAgeMs: 300000, harvesterExitMaxSizeB: 20971520, }; @@ -201,38 +239,4 @@ describe('envVarUtils', () => { expect(result).toBe('None'); }); }); - - describe('getEnvVarIndex', () => { - it('should return correct index for existing env var', () => { - const result = getEnvVarIndex(mockContainerWithConfig, AGENT_ENV_VARS.HARVESTER_TEMPLATE); - expect(result).toBe(0); - }); - - it('should return -1 for non-existent env var', () => { - const result = getEnvVarIndex(mockContainerWithoutConfig, AGENT_ENV_VARS.HARVESTER_TEMPLATE); - expect(result).toBe(-1); - }); - - it('should return -1 for container without env array', () => { - const result = getEnvVarIndex(mockContainerNoEnv, AGENT_ENV_VARS.HARVESTER_TEMPLATE); - expect(result).toBe(-1); - }); - }); - - describe('hasEnvVar', () => { - it('should return true for existing env var', () => { - const result = hasEnvVar(mockContainerWithConfig, AGENT_ENV_VARS.HARVESTER_TEMPLATE); - expect(result).toBe(true); - }); - - it('should return false for non-existent env var', () => { - const result = hasEnvVar(mockContainerWithoutConfig, AGENT_ENV_VARS.HARVESTER_TEMPLATE); - expect(result).toBe(false); - }); - - it('should return false for container without env array', () => { - const result = hasEnvVar(mockContainerNoEnv, AGENT_ENV_VARS.HARVESTER_TEMPLATE); - expect(result).toBe(false); - }); - }); }); From e7d8975598573a5b4ae49b1b696d047c294dbe49 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 16 Apr 2026 17:10:10 -0400 Subject: [PATCH 2/7] refactor to extract constants --- .../DeploymentDecorator.tsx | 13 +- .../DeploymentLabelActionModal.tsx | 119 ++++++++++-------- .../actions/DeploymentLabelAction/utils.ts | 24 +++- 3 files changed, 90 insertions(+), 66 deletions(-) diff --git a/src/openshift/actions/DeploymentLabelAction/DeploymentDecorator.tsx b/src/openshift/actions/DeploymentLabelAction/DeploymentDecorator.tsx index bdd5a97a..db34664a 100644 --- a/src/openshift/actions/DeploymentLabelAction/DeploymentDecorator.tsx +++ b/src/openshift/actions/DeploymentLabelAction/DeploymentDecorator.tsx @@ -17,6 +17,7 @@ import CryostatIcon from '@console-plugin/assets/CryostatIcon'; import { k8sGet, K8sResourceKind, useK8sModel, useK8sWatchResource } from '@openshift-console/dynamic-plugin-sdk'; import { Node } from '@patternfly/react-topology'; import * as React from 'react'; +import { AGENT_LABEL_KEYS } from './utils'; type DeploymentDecoratorProps = { element: Node; @@ -50,7 +51,7 @@ export const DeploymentDecorator: React.FC = ({ elemen React.useEffect(() => { if (deploymentLoaded && cryostatsLoaded) { const deploymentLabels = deployment.spec?.template.metadata.labels; - if (deploymentLabels && deploymentLabels['cryostat.io/name'] && deploymentLabels['cryostat.io/namespace']) { + if (deploymentLabels && deploymentLabels[AGENT_LABEL_KEYS.NAME] && deploymentLabels[AGENT_LABEL_KEYS.NAMESPACE]) { setIsRegistered(true); } else { setIsRegistered(false); @@ -61,16 +62,16 @@ export const DeploymentDecorator: React.FC = ({ elemen React.useEffect(() => { if (deploymentLoaded && cryostatsLoaded && isRegistered) { const labels = deployment.spec?.template.metadata.labels; - if (labels && labels['cryostat.io/name'] && labels['cryostat.io/namespace']) { + if (labels && labels[AGENT_LABEL_KEYS.NAME] && labels[AGENT_LABEL_KEYS.NAMESPACE]) { cryostats.forEach((cryostat) => { if ( - cryostat.metadata?.name === labels['cryostat.io/name'] && - cryostat.metadata?.namespace === labels['cryostat.io/namespace'] + cryostat.metadata?.name === labels[AGENT_LABEL_KEYS.NAME] && + cryostat.metadata?.namespace === labels[AGENT_LABEL_KEYS.NAMESPACE] ) { k8sGet({ model: routeModel, - name: labels['cryostat.io/name'], - ns: labels['cryostat.io/namespace'], + name: labels[AGENT_LABEL_KEYS.NAME], + ns: labels[AGENT_LABEL_KEYS.NAMESPACE], }) .catch(() => '') .then( diff --git a/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx b/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx index 0f2bbb62..079056e6 100644 --- a/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx +++ b/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx @@ -38,7 +38,15 @@ import { import * as React from 'react'; import { ContainerSelectionStep } from './ContainerSelectionStep'; -import { Container, HARVESTER_TEMPLATES, LOG_LEVELS, HarvesterTemplate, LogLevel, parseDuration } from './utils'; +import { + Container, + HARVESTER_TEMPLATES, + LOG_LEVELS, + HarvesterTemplate, + LogLevel, + parseDuration, + AGENT_LABEL_KEYS, +} from './utils'; import { HarvesterConfigStep } from './HarvesterConfigStep'; import { InstanceSelectionStep } from './InstanceSelectionStep'; import { JavaOptsConfigStep } from './JavaOptsConfigStep'; @@ -138,8 +146,8 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, return; } const deploymentLabels = resource.spec?.template.metadata.labels; - const name = deploymentLabels['cryostat.io/name']; - const namespace = deploymentLabels['cryostat.io/namespace']; + const name = deploymentLabels[AGENT_LABEL_KEYS.NAME]; + const namespace = deploymentLabels[AGENT_LABEL_KEYS.NAMESPACE]; for (let i = 0; i < cryostats.length; i++) { if (cryostats[i].metadata?.name === name && cryostats[i].metadata?.namespace === namespace) { setFormSelectValue(i.toString()); @@ -154,14 +162,15 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, if (containers.length > 0) { const firstContainer = containers[0]; const deploymentLabels = resource.spec?.template.metadata.labels; - const logLevelFromLabel = (deploymentLabels?.['cryostat.io/log-level'] as LogLevel) || LOG_LEVELS.OFF; - const javaOptsVarFromLabel = deploymentLabels?.['cryostat.io/java-options-var'] || 'JAVA_TOOL_OPTIONS'; + const logLevelFromLabel = (deploymentLabels?.[AGENT_LABEL_KEYS.LOG_LEVEL] as LogLevel) || LOG_LEVELS.OFF; + const javaOptsVarFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.JAVA_OPTIONS_VAR] || 'JAVA_TOOL_OPTIONS'; const harvesterTemplateFromLabel = - (deploymentLabels?.['cryostat.io/harvester-template'] as HarvesterTemplate) || HARVESTER_TEMPLATES.CONTINUOUS; - const harvesterPeriodFromLabel = deploymentLabels?.['cryostat.io/harvester-period']; - const harvesterMaxFilesFromLabel = deploymentLabels?.['cryostat.io/harvester-max-files']; - const harvesterExitMaxAgeFromLabel = deploymentLabels?.['cryostat.io/harvester-exit-max-age']; - const harvesterExitMaxSizeFromLabel = deploymentLabels?.['cryostat.io/harvester-exit-max-size']; + (deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_TEMPLATE] as HarvesterTemplate) || + HARVESTER_TEMPLATES.CONTINUOUS; + const harvesterPeriodFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_PERIOD]; + const harvesterMaxFilesFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_MAX_FILES]; + const harvesterExitMaxAgeFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_AGE]; + const harvesterExitMaxSizeFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_SIZE]; setFormData((prev) => { const newData = { @@ -258,47 +267,47 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, const patches: Patch[] = [ { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1name', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.NAME.replace('/', '~1')}`, value: instance.metadata?.name, }, { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1namespace', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.NAMESPACE.replace('/', '~1')}`, value: instance.metadata?.namespace, }, { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1log-level', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.LOG_LEVEL.replace('/', '~1')}`, value: formData.logLevel, }, { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1java-options-var', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.JAVA_OPTIONS_VAR.replace('/', '~1')}`, value: formData.javaOptsVar, }, { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-template', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_TEMPLATE.replace('/', '~1')}`, value: formData.harvesterTemplate, }, { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-period', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_PERIOD.replace('/', '~1')}`, value: `${formData.harvesterPeriodMs}ms`, }, { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-max-files', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_MAX_FILES.replace('/', '~1')}`, value: formData.harvesterMaxFiles.toString(), }, { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-exit-max-age', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_AGE.replace('/', '~1')}`, value: `${formData.harvesterExitMaxAgeMs}ms`, }, { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-exit-max-size', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_SIZE.replace('/', '~1')}`, value: formData.harvesterExitMaxSizeB.toString(), }, ]; @@ -311,58 +320,58 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, const deploymentLabels = resource.spec?.template.metadata.labels; // Only remove labels that exist - if (deploymentLabels?.['cryostat.io/name']) { + if (deploymentLabels?.[AGENT_LABEL_KEYS.NAME]) { patches.push({ op: 'remove', - path: '/spec/template/metadata/labels/cryostat.io~1name', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.NAME.replace('/', '~1')}`, }); } - if (deploymentLabels?.['cryostat.io/namespace']) { + if (deploymentLabels?.[AGENT_LABEL_KEYS.NAMESPACE]) { patches.push({ op: 'remove', - path: '/spec/template/metadata/labels/cryostat.io~1namespace', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.NAMESPACE.replace('/', '~1')}`, }); } - if (deploymentLabels?.['cryostat.io/log-level']) { + if (deploymentLabels?.[AGENT_LABEL_KEYS.LOG_LEVEL]) { patches.push({ op: 'remove', - path: '/spec/template/metadata/labels/cryostat.io~1log-level', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.LOG_LEVEL.replace('/', '~1')}`, }); } - if (deploymentLabels?.['cryostat.io/java-options-var']) { + if (deploymentLabels?.[AGENT_LABEL_KEYS.JAVA_OPTIONS_VAR]) { patches.push({ op: 'remove', - path: '/spec/template/metadata/labels/cryostat.io~1java-options-var', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.JAVA_OPTIONS_VAR.replace('/', '~1')}`, }); } - if (deploymentLabels?.['cryostat.io/harvester-template']) { + if (deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_TEMPLATE]) { patches.push({ op: 'remove', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-template', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_TEMPLATE.replace('/', '~1')}`, }); } - if (deploymentLabels?.['cryostat.io/harvester-period']) { + if (deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_PERIOD]) { patches.push({ op: 'remove', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-period', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_PERIOD.replace('/', '~1')}`, }); } - if (deploymentLabels?.['cryostat.io/harvester-max-files']) { + if (deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_MAX_FILES]) { patches.push({ op: 'remove', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-max-files', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_MAX_FILES.replace('/', '~1')}`, }); } - if (deploymentLabels?.['cryostat.io/harvester-exit-max-age']) { + if (deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_AGE]) { patches.push({ op: 'remove', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-exit-max-age', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_AGE.replace('/', '~1')}`, }); } - if (deploymentLabels?.['cryostat.io/harvester-exit-max-size']) { + if (deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_SIZE]) { patches.push({ op: 'remove', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-exit-max-size', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_SIZE.replace('/', '~1')}`, }); } @@ -435,50 +444,50 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, const patches: Patch[] = [ { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1name', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.NAME.replace('/', '~1')}`, value: cryostatInstance.metadata?.name, }, { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1namespace', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.NAMESPACE.replace('/', '~1')}`, value: cryostatInstance.metadata?.namespace, }, { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-template', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_TEMPLATE.replace('/', '~1')}`, value: quickRegisterData.harvesterTemplate, }, { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-period', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_PERIOD.replace('/', '~1')}`, value: `${quickRegisterData.harvesterPeriodMs}ms`, }, { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-max-files', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_MAX_FILES.replace('/', '~1')}`, value: quickRegisterData.harvesterMaxFiles.toString(), }, { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-exit-max-age', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_AGE.replace('/', '~1')}`, value: `${quickRegisterData.harvesterExitMaxAgeMs}ms`, }, { op: 'replace', - path: '/spec/template/metadata/labels/cryostat.io~1harvester-exit-max-size', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_SIZE.replace('/', '~1')}`, value: quickRegisterData.harvesterExitMaxSizeB.toString(), }, ]; - if (cryostatInstance.metadata?.labels?.['cryostat.io/log-level']) { + if (cryostatInstance.metadata?.labels?.[AGENT_LABEL_KEYS.LOG_LEVEL]) { patches.push({ op: 'remove', - path: '/spec/template/metadata/labels/cryostat.io~1log-level', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.LOG_LEVEL.replace('/', '~1')}`, }); } - if (cryostatInstance.metadata?.labels?.['cryostat.io/java-options-var']) { + if (cryostatInstance.metadata?.labels?.[AGENT_LABEL_KEYS.JAVA_OPTIONS_VAR]) { patches.push({ op: 'remove', - path: '/spec/template/metadata/labels/cryostat.io~1java-options-var', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.JAVA_OPTIONS_VAR.replace('/', '~1')}`, }); } patchResource(patches); @@ -494,14 +503,14 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, const handleContainerChange = (index: number) => { const container = containers[index]; const deploymentLabels = resource.spec?.template.metadata.labels; - const logLevelFromLabel = (deploymentLabels?.['cryostat.io/log-level'] as LogLevel) || LOG_LEVELS.OFF; - const javaOptsVarFromLabel = deploymentLabels?.['cryostat.io/java-options-var'] || 'JAVA_TOOL_OPTIONS'; + const logLevelFromLabel = (deploymentLabels?.[AGENT_LABEL_KEYS.LOG_LEVEL] as LogLevel) || LOG_LEVELS.OFF; + const javaOptsVarFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.JAVA_OPTIONS_VAR] || 'JAVA_TOOL_OPTIONS'; const harvesterTemplateFromLabel = - (deploymentLabels?.['cryostat.io/harvester-template'] as HarvesterTemplate) || HARVESTER_TEMPLATES.CONTINUOUS; - const harvesterPeriodFromLabel = deploymentLabels?.['cryostat.io/harvester-period']; - const harvesterMaxFilesFromLabel = deploymentLabels?.['cryostat.io/harvester-max-files']; - const harvesterExitMaxAgeFromLabel = deploymentLabels?.['cryostat.io/harvester-exit-max-age']; - const harvesterExitMaxSizeFromLabel = deploymentLabels?.['cryostat.io/harvester-exit-max-size']; + (deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_TEMPLATE] as HarvesterTemplate) || HARVESTER_TEMPLATES.CONTINUOUS; + const harvesterPeriodFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_PERIOD]; + const harvesterMaxFilesFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_MAX_FILES]; + const harvesterExitMaxAgeFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_AGE]; + const harvesterExitMaxSizeFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_SIZE]; setFormData((prev) => ({ ...prev, diff --git a/src/openshift/actions/DeploymentLabelAction/utils.ts b/src/openshift/actions/DeploymentLabelAction/utils.ts index a16e3907..cf9628f0 100644 --- a/src/openshift/actions/DeploymentLabelAction/utils.ts +++ b/src/openshift/actions/DeploymentLabelAction/utils.ts @@ -14,6 +14,20 @@ * limitations under the License. */ +export const AGENT_AUTOCONFIG_LABEL_PREFIX = 'cryostat.io/'; + +export const AGENT_LABEL_KEYS = { + NAME: `${AGENT_AUTOCONFIG_LABEL_PREFIX}name`, + NAMESPACE: `${AGENT_AUTOCONFIG_LABEL_PREFIX}namespace`, + LOG_LEVEL: `${AGENT_AUTOCONFIG_LABEL_PREFIX}log-level`, + JAVA_OPTIONS_VAR: `${AGENT_AUTOCONFIG_LABEL_PREFIX}java-options-var`, + HARVESTER_TEMPLATE: `${AGENT_AUTOCONFIG_LABEL_PREFIX}harvester-template`, + HARVESTER_PERIOD: `${AGENT_AUTOCONFIG_LABEL_PREFIX}harvester-period`, + HARVESTER_MAX_FILES: `${AGENT_AUTOCONFIG_LABEL_PREFIX}harvester-max-files`, + HARVESTER_EXIT_MAX_AGE: `${AGENT_AUTOCONFIG_LABEL_PREFIX}harvester-exit-max-age`, + HARVESTER_EXIT_MAX_SIZE: `${AGENT_AUTOCONFIG_LABEL_PREFIX}harvester-exit-max-size`, +} as const; + export interface Container { name: string; image: string; @@ -70,11 +84,11 @@ export function getAgentConfig(container: Container): AgentConfig | null { return null; } - const harvesterTemplate = labels['cryostat.io/harvester-template']; - const harvesterPeriod = labels['cryostat.io/harvester-period']; - const harvesterMaxFiles = labels['cryostat.io/harvester-max-files']; - const harvesterExitMaxAge = labels['cryostat.io/harvester-exit-max-age']; - const harvesterExitMaxSize = labels['cryostat.io/harvester-exit-max-size']; + const harvesterTemplate = labels[AGENT_LABEL_KEYS.HARVESTER_TEMPLATE]; + const harvesterPeriod = labels[AGENT_LABEL_KEYS.HARVESTER_PERIOD]; + const harvesterMaxFiles = labels[AGENT_LABEL_KEYS.HARVESTER_MAX_FILES]; + const harvesterExitMaxAge = labels[AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_AGE]; + const harvesterExitMaxSize = labels[AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_SIZE]; if (!harvesterTemplate && !harvesterPeriod && !harvesterMaxFiles && !harvesterExitMaxAge && !harvesterExitMaxSize) { return null; From 0ca38fd7f6a8673f17779584a6d4721035133e67 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 16 Apr 2026 17:34:27 -0400 Subject: [PATCH 3/7] support callback-port --- locales/en/common.json | 6 + .../DeploymentLabelAction/AgentConfigStep.tsx | 104 ++++++++++++++++++ .../DeploymentLabelActionModal.tsx | 55 +++++++-- .../JavaOptsConfigStep.tsx | 46 -------- .../DeploymentLabelAction/ReviewStep.tsx | 8 ++ .../actions/DeploymentLabelAction/utils.ts | 1 + 6 files changed, 166 insertions(+), 54 deletions(-) create mode 100644 src/openshift/actions/DeploymentLabelAction/AgentConfigStep.tsx delete mode 100644 src/openshift/actions/DeploymentLabelAction/JavaOptsConfigStep.tsx diff --git a/locales/en/common.json b/locales/en/common.json index fc0388b1..1ac5d8d2 100644 --- a/locales/en/common.json +++ b/locales/en/common.json @@ -46,8 +46,13 @@ "DEPLOYMENT_ACTION_CONTAINER_NO_CONFIG": "None", "DEPLOYMENT_ACTION_CONTAINER_SINGLE_AUTO_SELECTED": "Single container automatically selected", "DEPLOYMENT_ACTION_CONTAINER_MUST_SELECT_ONE": "You must select exactly one container to instrument", + "DEPLOYMENT_ACTION_WIZARD_STEP_AGENT_CONFIG": "Agent Configuration", + "DEPLOYMENT_ACTION_WIZARD_STEP_AGENT_CONFIG_DESC": "Configure Agent settings", "DEPLOYMENT_ACTION_JAVA_OPTS_VAR_LABEL": "Java Options Environment Variable:", "DEPLOYMENT_ACTION_JAVA_OPTS_VAR_HELPER": "The environment variable name used to pass Java options to the JVM (default: JAVA_TOOL_OPTIONS)", + "DEPLOYMENT_ACTION_CALLBACK_PORT_LABEL": "Callback Port:", + "DEPLOYMENT_ACTION_CALLBACK_PORT_HELPER": "HTTP port for the Cryostat Agent (leave empty for Operator default)", + "DEPLOYMENT_ACTION_CALLBACK_PORT_PLACEHOLDER": "", "DEPLOYMENT_ACTION_HARVESTER_TEMPLATE_LABEL": "Harvester Template:", "DEPLOYMENT_ACTION_HARVESTER_TEMPLATE_NONE": "None", "DEPLOYMENT_ACTION_HARVESTER_TEMPLATE_CONTINUOUS": "Continuous", @@ -73,6 +78,7 @@ "DEPLOYMENT_ACTION_REVIEW_INSTANCE": "Cryostat Instance", "DEPLOYMENT_ACTION_REVIEW_CONTAINER": "Selected Container", "DEPLOYMENT_ACTION_REVIEW_JAVA_OPTS_VAR": "Java Options Variable", + "DEPLOYMENT_ACTION_REVIEW_CALLBACK_PORT": "Callback Port", "DEPLOYMENT_ACTION_REVIEW_HARVESTER": "Harvester Template", "DEPLOYMENT_ACTION_REVIEW_HARVESTER_PERIOD": "Harvester Period", "DEPLOYMENT_ACTION_REVIEW_HARVESTER_MAX_FILES": "Harvester Max Files", diff --git a/src/openshift/actions/DeploymentLabelAction/AgentConfigStep.tsx b/src/openshift/actions/DeploymentLabelAction/AgentConfigStep.tsx new file mode 100644 index 00000000..a5c595de --- /dev/null +++ b/src/openshift/actions/DeploymentLabelAction/AgentConfigStep.tsx @@ -0,0 +1,104 @@ +/* + * Copyright The Cryostat Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { useCryostatTranslation } from '@i18n/i18nextUtil'; +import { + Form, + FormGroup, + TextInput, + NumberInput, + FormHelperText, + HelperText, + HelperTextItem, +} from '@patternfly/react-core'; +import * as React from 'react'; + +interface AgentConfigStepProps { + javaOptsVar: string; + callbackPort?: number; + onJavaOptsVarChange: (value: string) => void; + onCallbackPortChange: (value: number | undefined) => void; +} + +export const AgentConfigStep: React.FC = ({ + javaOptsVar, + callbackPort, + onJavaOptsVarChange, + onCallbackPortChange, +}) => { + const { t } = useCryostatTranslation(); + + const handleCallbackPortChange = (event: React.FormEvent) => { + const value = (event.target as HTMLInputElement).value; + if (value === '') { + onCallbackPortChange(undefined); + } else { + const numValue = Number(value); + if (!isNaN(numValue) && numValue > 0) { + onCallbackPortChange(numValue); + } + } + }; + + const handleCallbackPortMinus = () => { + if (callbackPort !== undefined && callbackPort > 1) { + onCallbackPortChange(callbackPort - 1); + } + }; + + const handleCallbackPortPlus = () => { + if (callbackPort !== undefined) { + onCallbackPortChange(callbackPort + 1); + } else { + onCallbackPortChange(1); + } + }; + + return ( +
+ + onJavaOptsVarChange(value)} + placeholder="JAVA_TOOL_OPTIONS" + /> + + + {t('DEPLOYMENT_ACTION_JAVA_OPTS_VAR_HELPER')} + + + + + + + + {t('DEPLOYMENT_ACTION_CALLBACK_PORT_HELPER')} + + + +
+ ); +}; diff --git a/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx b/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx index 079056e6..020c20af 100644 --- a/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx +++ b/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx @@ -37,7 +37,12 @@ import { } from '@patternfly/react-core'; import * as React from 'react'; +import { AgentConfigStep } from './AgentConfigStep'; import { ContainerSelectionStep } from './ContainerSelectionStep'; +import { HarvesterConfigStep } from './HarvesterConfigStep'; +import { InstanceSelectionStep } from './InstanceSelectionStep'; +import { LogLevelConfigStep } from './LogLevelConfigStep'; +import { ReviewStep } from './ReviewStep'; import { Container, HARVESTER_TEMPLATES, @@ -47,11 +52,6 @@ import { parseDuration, AGENT_LABEL_KEYS, } from './utils'; -import { HarvesterConfigStep } from './HarvesterConfigStep'; -import { InstanceSelectionStep } from './InstanceSelectionStep'; -import { JavaOptsConfigStep } from './JavaOptsConfigStep'; -import { LogLevelConfigStep } from './LogLevelConfigStep'; -import { ReviewStep } from './ReviewStep'; interface CryostatModalProps { kind: K8sModel; @@ -65,6 +65,7 @@ interface WizardFormData { selectedContainerIndex: number; selectedContainerName: string; javaOptsVar: string; + callbackPort?: number; harvesterTemplate: HarvesterTemplate; harvesterPeriodMs: number; harvesterMaxFiles: number; @@ -80,6 +81,7 @@ const formDefaults: WizardFormData = { selectedContainerIndex: 0, selectedContainerName: '', javaOptsVar: 'JAVA_TOOL_OPTIONS', + callbackPort: undefined, harvesterTemplate: HARVESTER_TEMPLATES.CONTINUOUS, harvesterPeriodMs: 900000, harvesterMaxFiles: 4, @@ -164,6 +166,7 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, const deploymentLabels = resource.spec?.template.metadata.labels; const logLevelFromLabel = (deploymentLabels?.[AGENT_LABEL_KEYS.LOG_LEVEL] as LogLevel) || LOG_LEVELS.OFF; const javaOptsVarFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.JAVA_OPTIONS_VAR] || 'JAVA_TOOL_OPTIONS'; + const callbackPortFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.CALLBACK_PORT]; const harvesterTemplateFromLabel = (deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_TEMPLATE] as HarvesterTemplate) || HARVESTER_TEMPLATES.CONTINUOUS; @@ -178,6 +181,7 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, selectedContainerIndex: 0, selectedContainerName: firstContainer.name, javaOptsVar: javaOptsVarFromLabel, + callbackPort: callbackPortFromLabel ? parseInt(callbackPortFromLabel, 10) : undefined, harvesterTemplate: harvesterTemplateFromLabel, harvesterPeriodMs: parseDuration(harvesterPeriodFromLabel, 900000), harvesterMaxFiles: harvesterMaxFilesFromLabel ? parseInt(harvesterMaxFilesFromLabel, 10) : 4, @@ -312,6 +316,14 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, }, ]; + if (formData.callbackPort !== undefined) { + patches.push({ + op: 'replace', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.CALLBACK_PORT.replace('/', '~1')}`, + value: formData.callbackPort.toString(), + }); + } + patchResource(patches); } @@ -344,6 +356,12 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.JAVA_OPTIONS_VAR.replace('/', '~1')}`, }); } + if (deploymentLabels?.[AGENT_LABEL_KEYS.CALLBACK_PORT]) { + patches.push({ + op: 'remove', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.CALLBACK_PORT.replace('/', '~1')}`, + }); + } if (deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_TEMPLATE]) { patches.push({ op: 'remove', @@ -383,6 +401,7 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, const hasInstanceChanged = formSelectValue !== initialValue || formData.cryostatInstance !== initialValue; const hasJavaOptsChanged = formData.javaOptsVar !== initialFormData.javaOptsVar; + const hasCallbackPortChanged = formData.callbackPort !== initialFormData.callbackPort; const hasHarvesterChanged = formData.harvesterTemplate !== initialFormData.harvesterTemplate; const hasHarvesterPeriodChanged = formData.harvesterPeriodMs !== initialFormData.harvesterPeriodMs; const hasHarvesterMaxFilesChanged = formData.harvesterMaxFiles !== initialFormData.harvesterMaxFiles; @@ -392,6 +411,7 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, const hasAnyChange = hasInstanceChanged || hasJavaOptsChanged || + hasCallbackPortChanged || hasHarvesterChanged || hasHarvesterPeriodChanged || hasHarvesterMaxFilesChanged || @@ -428,6 +448,7 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, selectedContainerIndex: 0, selectedContainerName: containers[0]?.name || '', javaOptsVar: 'JAVA_TOOL_OPTIONS', + callbackPort: undefined, harvesterTemplate: HARVESTER_TEMPLATES.CONTINUOUS, harvesterPeriodMs: 900000, harvesterMaxFiles: 4, @@ -490,6 +511,12 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.JAVA_OPTIONS_VAR.replace('/', '~1')}`, }); } + if (cryostatInstance.metadata?.labels?.[AGENT_LABEL_KEYS.CALLBACK_PORT]) { + patches.push({ + op: 'remove', + path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.CALLBACK_PORT.replace('/', '~1')}`, + }); + } patchResource(patches); } @@ -505,6 +532,7 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, const deploymentLabels = resource.spec?.template.metadata.labels; const logLevelFromLabel = (deploymentLabels?.[AGENT_LABEL_KEYS.LOG_LEVEL] as LogLevel) || LOG_LEVELS.OFF; const javaOptsVarFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.JAVA_OPTIONS_VAR] || 'JAVA_TOOL_OPTIONS'; + const callbackPortFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.CALLBACK_PORT]; const harvesterTemplateFromLabel = (deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_TEMPLATE] as HarvesterTemplate) || HARVESTER_TEMPLATES.CONTINUOUS; const harvesterPeriodFromLabel = deploymentLabels?.[AGENT_LABEL_KEYS.HARVESTER_PERIOD]; @@ -517,6 +545,7 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, selectedContainerIndex: index, selectedContainerName: container.name, javaOptsVar: javaOptsVarFromLabel, + callbackPort: callbackPortFromLabel ? parseInt(callbackPortFromLabel, 10) : undefined, harvesterTemplate: harvesterTemplateFromLabel, harvesterPeriodMs: parseDuration(harvesterPeriodFromLabel, 900000), harvesterMaxFiles: harvesterMaxFilesFromLabel ? parseInt(harvesterMaxFilesFromLabel, 10) : 4, @@ -530,6 +559,10 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, setFormData((prev) => ({ ...prev, javaOptsVar: value })); }; + const handleCallbackPortChange = (value: number | undefined) => { + setFormData((prev) => ({ ...prev, callbackPort: value })); + }; + const handleHarvesterChange = ( template: HarvesterTemplate, periodMs: number, @@ -627,13 +660,18 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, /> - + = ({ kind, selectedInstance={selectedInstance} selectedContainer={selectedContainer} javaOptsVar={formData.javaOptsVar} + callbackPort={formData.callbackPort} harvesterTemplate={formData.harvesterTemplate} harvesterPeriodMs={formData.harvesterPeriodMs} harvesterMaxFiles={formData.harvesterMaxFiles} diff --git a/src/openshift/actions/DeploymentLabelAction/JavaOptsConfigStep.tsx b/src/openshift/actions/DeploymentLabelAction/JavaOptsConfigStep.tsx deleted file mode 100644 index 1aff4997..00000000 --- a/src/openshift/actions/DeploymentLabelAction/JavaOptsConfigStep.tsx +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright The Cryostat Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import { useCryostatTranslation } from '@i18n/i18nextUtil'; -import { Form, FormGroup, TextInput, FormHelperText, HelperText, HelperTextItem } from '@patternfly/react-core'; -import * as React from 'react'; - -interface JavaOptsConfigStepProps { - javaOptsVar: string; - onChange: (value: string) => void; -} - -export const JavaOptsConfigStep: React.FC = ({ javaOptsVar, onChange }) => { - const { t } = useCryostatTranslation(); - - return ( -
- - onChange(value)} - placeholder="JAVA_TOOL_OPTIONS" - /> - - - {t('DEPLOYMENT_ACTION_JAVA_OPTS_VAR_HELPER')} - - - -
- ); -}; diff --git a/src/openshift/actions/DeploymentLabelAction/ReviewStep.tsx b/src/openshift/actions/DeploymentLabelAction/ReviewStep.tsx index 04e37000..70f5c364 100644 --- a/src/openshift/actions/DeploymentLabelAction/ReviewStep.tsx +++ b/src/openshift/actions/DeploymentLabelAction/ReviewStep.tsx @@ -35,6 +35,7 @@ interface ReviewStepProps { harvesterExitMaxSizeB: number; logLevel: LogLevel; javaOptsVar: string; + callbackPort?: number; } export const ReviewStep: React.FC = ({ @@ -47,6 +48,7 @@ export const ReviewStep: React.FC = ({ harvesterExitMaxSizeB, logLevel, javaOptsVar, + callbackPort, }) => { const { t } = useCryostatTranslation(); @@ -77,6 +79,12 @@ export const ReviewStep: React.FC = ({ {t('DEPLOYMENT_ACTION_REVIEW_JAVA_OPTS_VAR')} {javaOptsVar} + + {t('DEPLOYMENT_ACTION_REVIEW_CALLBACK_PORT')} + + {callbackPort !== undefined ? callbackPort.toString() : ''} + + {t('DEPLOYMENT_ACTION_REVIEW_HARVESTER')} {getHarvesterDisplayName(harvesterTemplate)} diff --git a/src/openshift/actions/DeploymentLabelAction/utils.ts b/src/openshift/actions/DeploymentLabelAction/utils.ts index cf9628f0..cf6a12a9 100644 --- a/src/openshift/actions/DeploymentLabelAction/utils.ts +++ b/src/openshift/actions/DeploymentLabelAction/utils.ts @@ -21,6 +21,7 @@ export const AGENT_LABEL_KEYS = { NAMESPACE: `${AGENT_AUTOCONFIG_LABEL_PREFIX}namespace`, LOG_LEVEL: `${AGENT_AUTOCONFIG_LABEL_PREFIX}log-level`, JAVA_OPTIONS_VAR: `${AGENT_AUTOCONFIG_LABEL_PREFIX}java-options-var`, + CALLBACK_PORT: `${AGENT_AUTOCONFIG_LABEL_PREFIX}callback-port`, HARVESTER_TEMPLATE: `${AGENT_AUTOCONFIG_LABEL_PREFIX}harvester-template`, HARVESTER_PERIOD: `${AGENT_AUTOCONFIG_LABEL_PREFIX}harvester-period`, HARVESTER_MAX_FILES: `${AGENT_AUTOCONFIG_LABEL_PREFIX}harvester-max-files`, From 1ef61152fee0c76e600793018c9e54c42e8e834f Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 17 Apr 2026 09:18:37 -0400 Subject: [PATCH 4/7] format durations more nicely in labels --- .../DeploymentLabelActionModal.tsx | 9 +++++---- .../actions/DeploymentLabelAction/utils.ts | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx b/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx index 020c20af..ee39d916 100644 --- a/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx +++ b/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx @@ -50,6 +50,7 @@ import { HarvesterTemplate, LogLevel, parseDuration, + formatDurationForLabel, AGENT_LABEL_KEYS, } from './utils'; @@ -297,7 +298,7 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, { op: 'replace', path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_PERIOD.replace('/', '~1')}`, - value: `${formData.harvesterPeriodMs}ms`, + value: formatDurationForLabel(formData.harvesterPeriodMs), }, { op: 'replace', @@ -307,7 +308,7 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, { op: 'replace', path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_AGE.replace('/', '~1')}`, - value: `${formData.harvesterExitMaxAgeMs}ms`, + value: formatDurationForLabel(formData.harvesterExitMaxAgeMs), }, { op: 'replace', @@ -481,7 +482,7 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, { op: 'replace', path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_PERIOD.replace('/', '~1')}`, - value: `${quickRegisterData.harvesterPeriodMs}ms`, + value: formatDurationForLabel(quickRegisterData.harvesterPeriodMs), }, { op: 'replace', @@ -491,7 +492,7 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, { op: 'replace', path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_AGE.replace('/', '~1')}`, - value: `${quickRegisterData.harvesterExitMaxAgeMs}ms`, + value: formatDurationForLabel(quickRegisterData.harvesterExitMaxAgeMs), }, { op: 'replace', diff --git a/src/openshift/actions/DeploymentLabelAction/utils.ts b/src/openshift/actions/DeploymentLabelAction/utils.ts index cf6a12a9..d6516959 100644 --- a/src/openshift/actions/DeploymentLabelAction/utils.ts +++ b/src/openshift/actions/DeploymentLabelAction/utils.ts @@ -79,6 +79,25 @@ export function parseDuration(duration: string | undefined, defaultValue: number } } +export function formatDurationForLabel(durationMs: number): string { + const hours = durationMs / (60 * 60 * 1000); + if (Number.isInteger(hours) && hours >= 1) { + return `${hours}h`; + } + + const minutes = durationMs / (60 * 1000); + if (Number.isInteger(minutes) && minutes >= 1) { + return `${minutes}m`; + } + + const seconds = durationMs / 1000; + if (Number.isInteger(seconds) && seconds >= 1) { + return `${seconds}s`; + } + + return `${durationMs}ms`; +} + export function getAgentConfig(container: Container): AgentConfig | null { const labels = container.labels; if (!labels) { From 7290b32c8b1cd07bc4b554fdcfde91d1cd9d4293 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 17 Apr 2026 09:44:36 -0400 Subject: [PATCH 5/7] format byte sizes more nicely in labels --- .../DeploymentLabelActionModal.tsx | 5 +++-- .../actions/DeploymentLabelAction/utils.ts | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx b/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx index ee39d916..e6876051 100644 --- a/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx +++ b/src/openshift/actions/DeploymentLabelAction/DeploymentLabelActionModal.tsx @@ -51,6 +51,7 @@ import { LogLevel, parseDuration, formatDurationForLabel, + formatByteSizeForLabel, AGENT_LABEL_KEYS, } from './utils'; @@ -313,7 +314,7 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, { op: 'replace', path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_SIZE.replace('/', '~1')}`, - value: formData.harvesterExitMaxSizeB.toString(), + value: formatByteSizeForLabel(formData.harvesterExitMaxSizeB), }, ]; @@ -497,7 +498,7 @@ export const DeploymentLabelActionModal: React.FC = ({ kind, { op: 'replace', path: `/spec/template/metadata/labels/${AGENT_LABEL_KEYS.HARVESTER_EXIT_MAX_SIZE.replace('/', '~1')}`, - value: quickRegisterData.harvesterExitMaxSizeB.toString(), + value: formatByteSizeForLabel(quickRegisterData.harvesterExitMaxSizeB), }, ]; if (cryostatInstance.metadata?.labels?.[AGENT_LABEL_KEYS.LOG_LEVEL]) { diff --git a/src/openshift/actions/DeploymentLabelAction/utils.ts b/src/openshift/actions/DeploymentLabelAction/utils.ts index d6516959..aaca97c9 100644 --- a/src/openshift/actions/DeploymentLabelAction/utils.ts +++ b/src/openshift/actions/DeploymentLabelAction/utils.ts @@ -98,6 +98,25 @@ export function formatDurationForLabel(durationMs: number): string { return `${durationMs}ms`; } +export function formatByteSizeForLabel(bytes: number): string { + const gi = bytes / (1024 * 1024 * 1024); + if (Number.isInteger(gi) && gi >= 1) { + return `${gi}Gi`; + } + + const mi = bytes / (1024 * 1024); + if (Number.isInteger(mi) && mi >= 1) { + return `${mi}Mi`; + } + + const ki = bytes / 1024; + if (Number.isInteger(ki) && ki >= 1) { + return `${ki}Ki`; + } + + return `${bytes}`; +} + export function getAgentConfig(container: Container): AgentConfig | null { const labels = container.labels; if (!labels) { From b671d240f3d162422063fd8a2b726c13d55856ba Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 17 Apr 2026 09:44:42 -0400 Subject: [PATCH 6/7] tests --- .../DeploymentLabelActionModal.test.tsx | 136 ++++++++++++++++ .../DeploymentLabelAction/utils.test.ts | 148 ++++++++++++++++++ 2 files changed, 284 insertions(+) diff --git a/src/openshift/test/actions/DeploymentLabelAction/DeploymentLabelActionModal.test.tsx b/src/openshift/test/actions/DeploymentLabelAction/DeploymentLabelActionModal.test.tsx index 4b5fff70..bcdb6df3 100644 --- a/src/openshift/test/actions/DeploymentLabelAction/DeploymentLabelActionModal.test.tsx +++ b/src/openshift/test/actions/DeploymentLabelAction/DeploymentLabelActionModal.test.tsx @@ -220,4 +220,140 @@ describe('DeploymentLabelActionModal', () => { expect(screen.getByText('DEPLOYMENT_ACTION_QUICK_REGISTER')).toBeInTheDocument(); }); }); + + it('should include callback port label when set in wizard', async () => { + renderModal(mockDeploymentWithoutLabels); + + const select = screen.getByLabelText('Cryostat Instance Selection'); + await userEvent.selectOptions(select, '0'); + await waitFor(() => { + expect(select).toHaveValue('0'); + }); + + // Navigate through wizard steps to Agent Configuration + let nextButton = screen.queryByRole('button', { name: /next/i }); + while (nextButton) { + await userEvent.click(nextButton); + // Wait a bit for the step to change + await new Promise((resolve) => setTimeout(resolve, 100)); + + // Check if we're on the Agent Configuration step by looking for the callback port field + const callbackPortInput = screen.queryByRole('spinbutton', { name: '' }); + if (callbackPortInput) { + // Found the callback port field, set its value + await userEvent.clear(callbackPortInput); + await userEvent.type(callbackPortInput, '8080'); + break; + } + + nextButton = screen.queryByRole('button', { name: /next/i }); + } + + // Continue to finish + nextButton = screen.queryByRole('button', { name: /next/i }); + while (nextButton) { + await userEvent.click(nextButton); + nextButton = screen.queryByRole('button', { name: /next/i }); + } + + const finishButton = screen.queryByRole('button', { name: /finish|submit/i }); + if (finishButton) { + await userEvent.click(finishButton); + expect(k8sPatchResource).toHaveBeenCalled(); + + const callArgs = (k8sPatchResource as jest.Mock).mock.calls[0][0]; + const patches = callArgs.patches; + + // Verify callback port patch is included + expect(patches).toEqual( + expect.arrayContaining([ + { + op: 'replace', + path: '/spec/template/metadata/labels/cryostat.io~1callback-port', + value: '8080', + }, + ]), + ); + } + }); + + it('should NOT include callback port label in Quick Register', async () => { + renderModal(mockDeploymentWithoutLabels); + + const select = screen.getByLabelText('Cryostat Instance Selection'); + await userEvent.selectOptions(select, '0'); + await waitFor(() => { + expect(select).toHaveValue('0'); + }); + + // Click Quick Register button + const quickRegisterButton = screen.getByText('DEPLOYMENT_ACTION_QUICK_REGISTER'); + await userEvent.click(quickRegisterButton); + + await waitFor(() => { + expect(k8sPatchResource).toHaveBeenCalled(); + }); + + const callArgs = (k8sPatchResource as jest.Mock).mock.calls[0][0]; + const patches = callArgs.patches; + + // Verify callback port patch is NOT included + const hasCallbackPortPatch = patches.some((p) => p.path?.includes('callback-port')); + expect(hasCallbackPortPatch).toBe(false); + }); + + it('should remove callback port label when deployment has it and user selects empty option', async () => { + const deploymentWithCallbackPort = { + ...mockDeploymentWithLabels, + spec: { + ...mockDeploymentWithLabels.spec, + template: { + ...mockDeploymentWithLabels.spec?.template, + metadata: { + ...mockDeploymentWithLabels.spec?.template?.metadata, + labels: { + ...mockDeploymentWithLabels.spec?.template?.metadata?.labels, + 'cryostat.io/callback-port': '8080', + }, + }, + }, + }, + }; + + renderModal(deploymentWithCallbackPort); + + const selectElement = screen.getByLabelText('Cryostat Instance Selection'); + expect(selectElement).toHaveValue('0'); + + await userEvent.selectOptions(selectElement, '-1'); + await waitFor(() => { + expect(selectElement).toHaveValue('-1'); + }); + + const nextButtons = screen.queryAllByRole('button', { name: /next/i }); + if (nextButtons.length > 0) { + for (const button of nextButtons) { + await userEvent.click(button); + } + } + + const finishButton = screen.queryByRole('button', { name: /finish|submit/i }); + if (finishButton) { + await userEvent.click(finishButton); + expect(k8sPatchResource).toHaveBeenCalled(); + + const callArgs = (k8sPatchResource as jest.Mock).mock.calls[0][0]; + const patches = callArgs.patches; + + // Verify callback port removal patch is included + expect(patches).toEqual( + expect.arrayContaining([ + { + op: 'remove', + path: '/spec/template/metadata/labels/cryostat.io~1callback-port', + }, + ]), + ); + } + }); }); diff --git a/src/openshift/test/actions/DeploymentLabelAction/utils.test.ts b/src/openshift/test/actions/DeploymentLabelAction/utils.test.ts index a157fac6..cd79fca5 100644 --- a/src/openshift/test/actions/DeploymentLabelAction/utils.test.ts +++ b/src/openshift/test/actions/DeploymentLabelAction/utils.test.ts @@ -19,6 +19,9 @@ import { LOG_LEVELS, getAgentConfig, formatAgentConfig, + formatDurationForLabel, + formatByteSizeForLabel, + parseDuration, } from '@console-plugin/actions/DeploymentLabelAction/utils'; describe('envVarUtils', () => { @@ -239,4 +242,149 @@ describe('envVarUtils', () => { expect(result).toBe('None'); }); }); + + describe('parseDuration', () => { + it('should parse duration with hours unit', () => { + expect(parseDuration('2h', 0)).toBe(2 * 60 * 60 * 1000); + }); + + it('should parse duration with minutes unit', () => { + expect(parseDuration('15m', 0)).toBe(15 * 60 * 1000); + }); + + it('should parse duration with seconds unit', () => { + expect(parseDuration('30s', 0)).toBe(30 * 1000); + }); + + it('should parse duration with milliseconds unit', () => { + expect(parseDuration('500ms', 0)).toBe(500); + }); + + it('should parse duration without unit as milliseconds', () => { + expect(parseDuration('1000', 0)).toBe(1000); + }); + + it('should return default value for undefined duration', () => { + expect(parseDuration(undefined, 12345)).toBe(12345); + }); + + it('should return default value for invalid duration format', () => { + expect(parseDuration('invalid', 12345)).toBe(12345); + }); + + it('should return default value for empty string', () => { + expect(parseDuration('', 12345)).toBe(12345); + }); + }); + + describe('formatDurationForLabel', () => { + it('should format duration in hours when value divides evenly', () => { + expect(formatDurationForLabel(3600000)).toBe('1h'); + expect(formatDurationForLabel(7200000)).toBe('2h'); + expect(formatDurationForLabel(10800000)).toBe('3h'); + }); + + it('should format duration in minutes when value divides evenly', () => { + expect(formatDurationForLabel(60000)).toBe('1m'); + expect(formatDurationForLabel(900000)).toBe('15m'); + expect(formatDurationForLabel(1800000)).toBe('30m'); + }); + + it('should format duration in seconds when value divides evenly', () => { + expect(formatDurationForLabel(1000)).toBe('1s'); + expect(formatDurationForLabel(5000)).toBe('5s'); + expect(formatDurationForLabel(30000)).toBe('30s'); + }); + + it('should format duration in milliseconds when value does not divide evenly', () => { + expect(formatDurationForLabel(1500)).toBe('1500ms'); + expect(formatDurationForLabel(999)).toBe('999ms'); + expect(formatDurationForLabel(12345)).toBe('12345ms'); + }); + + it('should prefer larger units over smaller units', () => { + // 1 hour should be formatted as 1h, not 60m + expect(formatDurationForLabel(3600000)).toBe('1h'); + // 1 minute should be formatted as 1m, not 60s + expect(formatDurationForLabel(60000)).toBe('1m'); + }); + + it('should handle zero duration', () => { + expect(formatDurationForLabel(0)).toBe('0ms'); + }); + + it('should handle very large durations', () => { + expect(formatDurationForLabel(86400000)).toBe('24h'); // 24 hours + }); + + it('should format default harvester period correctly', () => { + expect(formatDurationForLabel(900000)).toBe('15m'); + }); + + it('should format default harvester exit max age correctly', () => { + expect(formatDurationForLabel(300000)).toBe('5m'); + }); + }); + + describe('formatByteSizeForLabel', () => { + it('should format size in Gi when value divides evenly', () => { + expect(formatByteSizeForLabel(1073741824)).toBe('1Gi'); + expect(formatByteSizeForLabel(2147483648)).toBe('2Gi'); + expect(formatByteSizeForLabel(5368709120)).toBe('5Gi'); + }); + + it('should format size in Mi when value divides evenly', () => { + expect(formatByteSizeForLabel(1048576)).toBe('1Mi'); + expect(formatByteSizeForLabel(20971520)).toBe('20Mi'); + expect(formatByteSizeForLabel(52428800)).toBe('50Mi'); + }); + + it('should format size in Ki when value divides evenly', () => { + expect(formatByteSizeForLabel(1024)).toBe('1Ki'); + expect(formatByteSizeForLabel(2048)).toBe('2Ki'); + expect(formatByteSizeForLabel(10240)).toBe('10Ki'); + }); + + it('should format size in bytes when value does not divide evenly', () => { + expect(formatByteSizeForLabel(500)).toBe('500'); + expect(formatByteSizeForLabel(1023)).toBe('1023'); + expect(formatByteSizeForLabel(1025)).toBe('1025'); + }); + + it('should prefer larger units over smaller units', () => { + // 1 GiB should be formatted as 1Gi, not 1024Mi + expect(formatByteSizeForLabel(1073741824)).toBe('1Gi'); + // 1 MiB should be formatted as 1Mi, not 1024Ki + expect(formatByteSizeForLabel(1048576)).toBe('1Mi'); + }); + + it('should handle zero size', () => { + expect(formatByteSizeForLabel(0)).toBe('0'); + }); + + it('should handle very large sizes', () => { + expect(formatByteSizeForLabel(10737418240)).toBe('10Gi'); // 10 GiB + }); + + it('should format default harvester exit max size correctly', () => { + expect(formatByteSizeForLabel(20971520)).toBe('20Mi'); + }); + + it('should use Kubernetes resource format (no B suffix)', () => { + // Verify the format matches k8s resource quantities + expect(formatByteSizeForLabel(1073741824)).not.toContain('B'); + expect(formatByteSizeForLabel(1048576)).not.toContain('B'); + expect(formatByteSizeForLabel(1024)).not.toContain('B'); + }); + + it('should handle fractional MiB that do not divide evenly', () => { + // 1.5 MiB in bytes (1572864) should not format as Mi + expect(formatByteSizeForLabel(1572864)).toBe('1536Ki'); + }); + + it('should handle fractional GiB that do not divide evenly', () => { + // 1.5 GiB in bytes (1610612736) should not format as Gi + expect(formatByteSizeForLabel(1610612736)).toBe('1536Mi'); + }); + }); }); From edbbc219fcea7b4bbbeb02cbede20bddf0693f64 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 27 Apr 2026 08:35:05 -0400 Subject: [PATCH 7/7] parse byte sizes correctly --- .../actions/DeploymentLabelAction/utils.ts | 20 +++++- .../DeploymentLabelAction/utils.test.ts | 66 +++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/src/openshift/actions/DeploymentLabelAction/utils.ts b/src/openshift/actions/DeploymentLabelAction/utils.ts index aaca97c9..8fa33c1d 100644 --- a/src/openshift/actions/DeploymentLabelAction/utils.ts +++ b/src/openshift/actions/DeploymentLabelAction/utils.ts @@ -79,6 +79,24 @@ export function parseDuration(duration: string | undefined, defaultValue: number } } +export function parseByteSize(size: string | undefined, defaultValue: number): number { + if (!size) return defaultValue; + const match = size.match(/^(\d+)(Gi|Mi|Ki)?$/); + if (!match) return defaultValue; + const value = parseInt(match[1], 10); + const unit = match[2]; + switch (unit) { + case 'Gi': + return value * 1024 * 1024 * 1024; + case 'Mi': + return value * 1024 * 1024; + case 'Ki': + return value * 1024; + default: + return value; + } +} + export function formatDurationForLabel(durationMs: number): string { const hours = durationMs / (60 * 60 * 1000); if (Number.isInteger(hours) && hours >= 1) { @@ -138,7 +156,7 @@ export function getAgentConfig(container: Container): AgentConfig | null { harvesterPeriodMs: parseDuration(harvesterPeriod, 900000), harvesterMaxFiles: harvesterMaxFiles ? parseInt(harvesterMaxFiles, 10) : 4, harvesterExitMaxAgeMs: parseDuration(harvesterExitMaxAge, 300000), - harvesterExitMaxSizeB: harvesterExitMaxSize ? parseInt(harvesterExitMaxSize, 10) : 20971520, + harvesterExitMaxSizeB: parseByteSize(harvesterExitMaxSize, 20971520), }; } diff --git a/src/openshift/test/actions/DeploymentLabelAction/utils.test.ts b/src/openshift/test/actions/DeploymentLabelAction/utils.test.ts index cd79fca5..ad9f9b77 100644 --- a/src/openshift/test/actions/DeploymentLabelAction/utils.test.ts +++ b/src/openshift/test/actions/DeploymentLabelAction/utils.test.ts @@ -22,6 +22,7 @@ import { formatDurationForLabel, formatByteSizeForLabel, parseDuration, + parseByteSize, } from '@console-plugin/actions/DeploymentLabelAction/utils'; describe('envVarUtils', () => { @@ -155,6 +156,25 @@ describe('envVarUtils', () => { }); }); + it('should parse harvester exit max size with Kubernetes quantity suffix', () => { + const containerWithKubernetesSize: Container = { + name: 'app-container', + image: 'quay.io/app:latest', + labels: { + 'cryostat.io/harvester-template': 'Continuous', + 'cryostat.io/harvester-exit-max-size': '20Mi', + }, + }; + const result = getAgentConfig(containerWithKubernetesSize); + expect(result).toEqual({ + harvesterTemplate: 'Continuous', + harvesterPeriodMs: 900000, + harvesterMaxFiles: 4, + harvesterExitMaxAgeMs: 300000, + harvesterExitMaxSizeB: 20971520, // 20 * 1024 * 1024 + }); + }); + it('should parse all custom values when specified', () => { const containerWithAllCustom: Container = { name: 'app-container', @@ -387,4 +407,50 @@ describe('envVarUtils', () => { expect(formatByteSizeForLabel(1610612736)).toBe('1536Mi'); }); }); + + describe('parseByteSize', () => { + it('should parse size with Gi unit', () => { + expect(parseByteSize('2Gi', 0)).toBe(2 * 1024 * 1024 * 1024); + }); + + it('should parse size with Mi unit', () => { + expect(parseByteSize('20Mi', 0)).toBe(20 * 1024 * 1024); + }); + + it('should parse size with Ki unit', () => { + expect(parseByteSize('512Ki', 0)).toBe(512 * 1024); + }); + + it('should parse size without unit as bytes', () => { + expect(parseByteSize('1024', 0)).toBe(1024); + }); + + it('should return default value for undefined size', () => { + expect(parseByteSize(undefined, 12345)).toBe(12345); + }); + + it('should return default value for invalid size format', () => { + expect(parseByteSize('invalid', 12345)).toBe(12345); + }); + + it('should return default value for empty string', () => { + expect(parseByteSize('', 12345)).toBe(12345); + }); + + it('should parse default harvester exit max size correctly', () => { + expect(parseByteSize('20Mi', 0)).toBe(20971520); + }); + + it('should handle size with only numeric value', () => { + expect(parseByteSize('52428800', 0)).toBe(52428800); + }); + + it('should parse 1Gi correctly', () => { + expect(parseByteSize('1Gi', 0)).toBe(1073741824); + }); + + it('should parse 50Mi correctly', () => { + expect(parseByteSize('50Mi', 0)).toBe(52428800); + }); + }); });