From a80aa2b45c5113da5cb9ef0b82bb506215655d92 Mon Sep 17 00:00:00 2001 From: Ali <83188384+testA113@users.noreply.github.com> Date: Tue, 14 May 2024 13:39:53 +1200 Subject: [PATCH] fix(app): ensure placement errors surface per node [EE-7065] (#11820) Co-authored-by: testa113 --- .storybook/preview.tsx | 1 - .../CreateView/CreateForm.validation.ts | 17 +- .../CreateView/KubeManifestForm.tsx | 1 + .../PlacementsDatatableSubRow.tsx | 1 + .../PlacementsDatatable/columns/status.tsx | 12 +- .../usePlacementTableData.tsx | 150 ++++++++++-------- .../custom-templates/EditView/InnerForm.tsx | 1 - 7 files changed, 105 insertions(+), 78 deletions(-) diff --git a/.storybook/preview.tsx b/.storybook/preview.tsx index 2f0528a22..c2155c84f 100644 --- a/.storybook/preview.tsx +++ b/.storybook/preview.tsx @@ -8,7 +8,6 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; initMSW( { onUnhandledRequest: ({ method, url }) => { - console.log(method, url); if (url.startsWith('/api')) { console.error(`Unhandled ${method} request to ${url}. diff --git a/app/react/edge/edge-stacks/CreateView/CreateForm.validation.ts b/app/react/edge/edge-stacks/CreateView/CreateForm.validation.ts index b60ab36e8..a6dbbe961 100644 --- a/app/react/edge/edge-stacks/CreateView/CreateForm.validation.ts +++ b/app/react/edge/edge-stacks/CreateView/CreateForm.validation.ts @@ -17,7 +17,7 @@ import { useCurrentUser } from '@/react/hooks/useUser'; import { relativePathValidation } from '@/react/portainer/gitops/RelativePathFieldset/validation'; import { CustomTemplate } from '@/react/portainer/templates/custom-templates/types'; import { TemplateViewModel } from '@/react/portainer/templates/app-templates/view-model'; -import { GitFormModel } from '@/react/portainer/gitops/types'; +import { DeployMethod, GitFormModel } from '@/react/portainer/gitops/types'; import { envVarValidation } from '@@/form-components/EnvironmentVariablesFieldset'; import { file } from '@@/form-components/yup-file-validation'; @@ -76,10 +76,17 @@ export function useValidation({ }), git: mixed().when('method', { is: 'repository', - then: buildGitValidationSchema( - gitCredentialsQuery.data || [], - !!customTemplate - ), + then: () => { + const deploymentMethod: DeployMethod = + values.deploymentType === DeploymentType.Compose + ? 'compose' + : 'manifest'; + return buildGitValidationSchema( + gitCredentialsQuery.data || [], + !!customTemplate, + deploymentMethod + ); + }, }) as SchemaOf, relativePath: relativePathValidation(), useManifestNamespaces: boolean().default(false), diff --git a/app/react/edge/edge-stacks/CreateView/KubeManifestForm.tsx b/app/react/edge/edge-stacks/CreateView/KubeManifestForm.tsx index 9626c967b..8a99da7e2 100644 --- a/app/react/edge/edge-stacks/CreateView/KubeManifestForm.tsx +++ b/app/react/edge/edge-stacks/CreateView/KubeManifestForm.tsx @@ -95,6 +95,7 @@ export function KubeManifestForm({ {method === git.value && ( diff --git a/app/react/kubernetes/applications/DetailsView/PlacementsDatatable/PlacementsDatatableSubRow.tsx b/app/react/kubernetes/applications/DetailsView/PlacementsDatatable/PlacementsDatatableSubRow.tsx index e6b2e5bf5..22a502919 100644 --- a/app/react/kubernetes/applications/DetailsView/PlacementsDatatable/PlacementsDatatableSubRow.tsx +++ b/app/react/kubernetes/applications/DetailsView/PlacementsDatatable/PlacementsDatatableSubRow.tsx @@ -170,6 +170,7 @@ function UnmatchedAffinitiesInfo({ 'datatable-highlighted': isHighlighted, 'datatable-unhighlighted': !isHighlighted, })} + key={aff.map((term) => term.key).join('')} > diff --git a/app/react/kubernetes/applications/DetailsView/PlacementsDatatable/columns/status.tsx b/app/react/kubernetes/applications/DetailsView/PlacementsDatatable/columns/status.tsx index caa83d37c..991c1cc9b 100644 --- a/app/react/kubernetes/applications/DetailsView/PlacementsDatatable/columns/status.tsx +++ b/app/react/kubernetes/applications/DetailsView/PlacementsDatatable/columns/status.tsx @@ -11,11 +11,13 @@ export const status = columnHelper.accessor('acceptsApplication', { cell: ({ getValue }) => { const acceptsApplication = getValue(); return ( - +
+ +
); }, meta: { diff --git a/app/react/kubernetes/applications/DetailsView/PlacementsDatatable/usePlacementTableData.tsx b/app/react/kubernetes/applications/DetailsView/PlacementsDatatable/usePlacementTableData.tsx index 5efd0d552..16f4e9f3f 100644 --- a/app/react/kubernetes/applications/DetailsView/PlacementsDatatable/usePlacementTableData.tsx +++ b/app/react/kubernetes/applications/DetailsView/PlacementsDatatable/usePlacementTableData.tsx @@ -2,9 +2,9 @@ import { useCurrentStateAndParams } from '@uirouter/react'; import { useMemo } from 'react'; import { Pod, Taint, Node } from 'kubernetes-types/core/v1'; import _ from 'lodash'; -import * as JsonPatch from 'fast-json-patch'; import { useNodesQuery } from '@/react/kubernetes/cluster/HomeView/nodes.service'; +import { KubernetesPodNodeAffinityNodeSelectorRequirementOperators } from '@/kubernetes/pod/models'; import { BasicTableSettings, @@ -15,7 +15,7 @@ import { import { useTableState } from '@@/datatables/useTableState'; import { useApplication, useApplicationPods } from '../../application.queries'; -import { NodePlacementRowData } from '../types'; +import { Affinity, Label, NodePlacementRowData } from '../types'; interface TableSettings extends BasicTableSettings, RefreshableTableSettings {} @@ -162,6 +162,68 @@ function computeTolerations(nodes: Node[], pod: Pod): NodePlacementRowData[] { return nodePlacements; } +function getUnmatchedNodeSelectorLabels(node: Node, pod: Pod): Label[] { + const nodeLabels = node.metadata?.labels || {}; + const podNodeSelectorLabels = pod.spec?.nodeSelector || {}; + + return Object.entries(podNodeSelectorLabels) + .filter(([key, value]) => nodeLabels[key] !== value) + .map(([key, value]) => ({ + key, + value, + })); +} + +// Function to get unmatched required node affinities +function getUnmatchedRequiredNodeAffinities(node: Node, pod: Pod): Affinity[] { + const basicNodeAffinity = + pod.spec?.affinity?.nodeAffinity + ?.requiredDuringSchedulingIgnoredDuringExecution; + + const unmatchedRequiredNodeAffinities: Affinity[] = + basicNodeAffinity?.nodeSelectorTerms.map( + (selectorTerm) => + selectorTerm.matchExpressions?.flatMap((matchExpression) => { + const exists = !!node.metadata?.labels?.[matchExpression.key]; + const isIn = + exists && + _.includes( + matchExpression.values, + node.metadata?.labels?.[matchExpression.key] + ); + + // Check if the match expression is satisfied + if ( + (matchExpression.operator === 'Exists' && exists) || + (matchExpression.operator === 'DoesNotExist' && !exists) || + (matchExpression.operator === 'In' && isIn) || + (matchExpression.operator === 'NotIn' && !isIn) || + (matchExpression.operator === 'Gt' && + exists && + parseInt(node.metadata?.labels?.[matchExpression.key] || '', 10) > + parseInt(matchExpression.values?.[0] || '', 10)) || + (matchExpression.operator === 'Lt' && + exists && + parseInt(node.metadata?.labels?.[matchExpression.key] || '', 10) < + parseInt(matchExpression.values?.[0] || '', 10)) + ) { + return []; + } + + // Return the unmatched affinity + return [ + { + key: matchExpression.key, + operator: + matchExpression.operator as KubernetesPodNodeAffinityNodeSelectorRequirementOperators, + values: matchExpression.values?.join(', ') || '', + }, + ]; + }) || [] + ) || []; + return unmatchedRequiredNodeAffinities; +} + // Node requirement depending on the operator value // https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#node-affinity function computeAffinities( @@ -173,76 +235,32 @@ function computeAffinities( (node, nodeIndex) => { let { acceptsApplication } = nodePlacements[nodeIndex]; - if (pod.spec?.nodeSelector) { - const patch = JsonPatch.compare( - node.metadata?.labels || {}, - pod.spec.nodeSelector - ); - _.remove(patch, { op: 'remove' }); - const unmatchedNodeSelectorLabels = patch.map((operation) => ({ - key: _.trimStart(operation.path, '/'), - value: operation.op, - })); - if (unmatchedNodeSelectorLabels.length) { - acceptsApplication = false; - } - } + // check node selectors for unmatched labels + const unmatchedNodeSelectorLabels = getUnmatchedNodeSelectorLabels( + node, + pod + ); - const basicNodeAffinity = - pod.spec?.affinity?.nodeAffinity - ?.requiredDuringSchedulingIgnoredDuringExecution; - if (basicNodeAffinity) { - const unmatchedTerms = basicNodeAffinity.nodeSelectorTerms.map( - (selectorTerm) => { - const unmatchedExpressions = selectorTerm.matchExpressions?.flatMap( - (matchExpression) => { - const exists = {}.hasOwnProperty.call( - node.metadata?.labels, - matchExpression.key - ); - const isIn = - exists && - _.includes( - matchExpression.values, - node.metadata?.labels?.[matchExpression.key] - ); - if ( - (matchExpression.operator === 'Exists' && exists) || - (matchExpression.operator === 'DoesNotExist' && !exists) || - (matchExpression.operator === 'In' && isIn) || - (matchExpression.operator === 'NotIn' && !isIn) || - (matchExpression.operator === 'Gt' && - exists && - parseInt( - node.metadata?.labels?.[matchExpression.key] || '', - 10 - ) > parseInt(matchExpression.values?.[0] || '', 10)) || - (matchExpression.operator === 'Lt' && - exists && - parseInt( - node.metadata?.labels?.[matchExpression.key] || '', - 10 - ) < parseInt(matchExpression.values?.[0] || '', 10)) - ) { - return []; - } - return [true]; - } - ); + // check node affinities that are required during scheduling + const unmatchedRequiredNodeAffinities = + getUnmatchedRequiredNodeAffinities(node, pod); - return unmatchedExpressions; - } - ); - - _.remove(unmatchedTerms, (i) => !i); - if (unmatchedTerms.length) { - acceptsApplication = false; - } + // If there are any unmatched affinities or node labels, the node does not accept the application + if ( + unmatchedRequiredNodeAffinities.length || + unmatchedNodeSelectorLabels.length + ) { + acceptsApplication = false; } - return { + + const nodePlacementRowData: NodePlacementRowData = { ...nodePlacements[nodeIndex], acceptsApplication, + unmatchedNodeSelectorLabels, + unmatchedNodeAffinities: unmatchedRequiredNodeAffinities, }; + + return nodePlacementRowData; } ); return nodePlacementsFromAffinities; diff --git a/app/react/portainer/templates/custom-templates/EditView/InnerForm.tsx b/app/react/portainer/templates/custom-templates/EditView/InnerForm.tsx index 329cc7419..3075d6ca0 100644 --- a/app/react/portainer/templates/custom-templates/EditView/InnerForm.tsx +++ b/app/react/portainer/templates/custom-templates/EditView/InnerForm.tsx @@ -51,7 +51,6 @@ export function InnerForm({ isSubmitting, dirty, } = useFormikContext(); - console.log({ isEditorReadonly, isSubmitting, isLoading }); usePreventExit( initialValues.FileContent, values.FileContent,