Skip to content

Commit cb80c95

Browse files
jeswankejeswanke
authored
ACM-14403-Appset-application-remains-on-Ul-after-deletion (stolostron#3946)
* ACM-14403-Appset-application-remains-on-Ul-after-deletion Signed-off-by: John Swanke <jswanke@redhat.com> * fix coverage Signed-off-by: John Swanke <jswanke@redhat.com> * coverage Signed-off-by: John Swanke <jswanke@redhat.com> * coverage Signed-off-by: John Swanke <jswanke@redhat.com> * coverage Signed-off-by: John Swanke <jswanke@redhat.com> * fixes Signed-off-by: John Swanke <jswanke@redhat.com> * more fixes Signed-off-by: John Swanke <jswanke@redhat.com> * fix check Signed-off-by: John Swanke <jswanke@redhat.com> --------- Signed-off-by: John Swanke <jswanke@redhat.com>
1 parent 4ff0b43 commit cb80c95

File tree

8 files changed

+68
-27
lines changed

8 files changed

+68
-27
lines changed

‎backend/src/routes/aggregators/applications.ts

+13-9
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ appKeys.forEach((key) => {
8888

8989
export function getApplications() {
9090
const items: ITransformedResource[] = []
91+
aggregateKubeApplications(false)
9192
Object.keys(applicationCache).forEach((key) => {
9293
if (applicationCache[key].resources) {
9394
items.push(...applicationCache[key].resources)
@@ -108,7 +109,7 @@ export function startAggregatingApplications() {
108109
// aggregate local applications found in kube every 5 seconds
109110
async function localKubeLoop(): Promise<void> {
110111
while (!stopping) {
111-
aggregateKubeApplications()
112+
aggregateKubeApplications(true)
112113
await new Promise((r) => setTimeout(r, 5000))
113114
}
114115
}
@@ -121,16 +122,18 @@ async function searchAPILoop(): Promise<void> {
121122
}
122123
}
123124

124-
export function aggregateKubeApplications() {
125+
export function aggregateKubeApplications(force: boolean) {
125126
// ACM Apps
126-
applicationCache['subscription'] = generateTransforms(
127-
structuredClone(getKubeResources('Application', 'app.k8s.io/v1beta1'))
128-
)
127+
let resources = getKubeResources('Application', 'app.k8s.io/v1beta1')
128+
if (force || resources.length !== applicationCache['subscription'].resources.length) {
129+
applicationCache['subscription'] = generateTransforms(structuredClone(resources))
130+
}
129131

130132
// AppSets
131-
applicationCache['appset'] = generateTransforms(
132-
structuredClone(getKubeResources('ApplicationSet', 'argoproj.io/v1alpha1'))
133-
)
133+
resources = getKubeResources('ApplicationSet', 'argoproj.io/v1alpha1')
134+
if (force || resources.length !== applicationCache['appset'].resources.length) {
135+
applicationCache['appset'] = generateTransforms(structuredClone(resources))
136+
}
134137
}
135138

136139
export async function aggregateSearchAPIApplications(pass: number) {
@@ -283,7 +286,8 @@ function getAppSetCluster(resource: IArgoApplication, placementDecisions: IDecis
283286
pd.metadata.labels?.['cluster.open-cluster-management.io/placement'] === placementName &&
284287
pd.metadata.namespace === placementNamespace
285288
)
286-
const clusterDecisions = placementDecision?.status.decisions || []
289+
/* istanbul ignore next */
290+
const clusterDecisions = placementDecision?.status?.decisions || []
287291

288292
clusterDecisions.forEach((cd: { clusterName: string }) => {
289293
if (cd.clusterName !== 'local-cluster') {

‎backend/test/routes/aggregator.test.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ describe(`aggregator Route`, function () {
1919
setupNocks()
2020

2121
// fill in application cache from resourceCache and search api mocks
22-
aggregateKubeApplications()
22+
aggregateKubeApplications(true)
2323
await aggregateSearchAPIApplications(1)
2424

2525
// NO FILTER
@@ -44,7 +44,7 @@ describe(`aggregator Route`, function () {
4444
setupNocks()
4545

4646
// fill in application cache from resourceCache and search api mocks
47-
aggregateKubeApplications()
47+
aggregateKubeApplications(true)
4848
await aggregateSearchAPIApplications(1)
4949

5050
// FILTERED
@@ -75,7 +75,7 @@ describe(`aggregator Route`, function () {
7575
// fill in application cache from resourceCache and search api mocks
7676
const prefixes = await discoverSystemAppNamespacePrefixes()
7777
expect(JSON.stringify(prefixes)).toEqual(JSON.stringify(systemPrefixes))
78-
aggregateKubeApplications()
78+
aggregateKubeApplications(true)
7979
await aggregateSearchAPIApplications(1)
8080

8181
// FILTERED

‎frontend/src/lib/delete-application.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import { IResource, ResourceError, ResourceErrorCode } from '../resources'
44
import { deleteResources } from './delete-resources'
55

6-
export function deleteApplication(app: IResource, childResources?: any[]) {
6+
export function deleteApplication(app: IResource, childResources?: any[], deleted?: (resource: IResource) => void) {
77
const allResources = [app]
88

99
childResources?.forEach((resource) => {
@@ -38,6 +38,7 @@ export function deleteApplication(app: IResource, childResources?: any[]) {
3838
return
3939
}
4040
resolve(promisesSettledResult)
41+
deleted?.(app)
4142
})
4243
}),
4344
abort: deleteResourcesResult.abort,

‎frontend/src/lib/useAggregates.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export interface IRequestListView {
2828
export interface IResultListView {
2929
page: number
3030
loading: boolean
31+
refresh: () => void
3132
items: IResource[]
3233
emptyResult: boolean
3334
processedItemCount: number
@@ -43,6 +44,7 @@ export interface IResultStatuses {
4344
filterCounts: FilterCounts | undefined
4445
systemAppNSPrefixes: string[]
4546
loading: boolean
47+
refresh: () => void
4648
}
4749

4850
export enum SupportedAggregate {
@@ -56,6 +58,7 @@ export enum SupportedAggregate {
5658
const defaultListResponse: IResultListView = {
5759
page: 1,
5860
loading: true,
61+
refresh: () => {},
5962
items: [],
6063
emptyResult: false,
6164
processedItemCount: 0,
@@ -67,6 +70,7 @@ const defaultStatusResponse: IResultStatuses = {
6770
filterCounts: { type: {} },
6871
systemAppNSPrefixes: [],
6972
loading: true,
73+
refresh: () => {},
7074
}
7175

7276
type RequestStatusesType = IRequestStatuses | undefined
@@ -111,7 +115,7 @@ export function useAggregate(
111115
: undefined
112116
}, [aggregate, backendUrl, requestedViewStr])
113117

114-
const { data, loading, startPolling, stopPolling } = useQuery(queryFunc, [defaultResponse], {
118+
const { data, loading, startPolling, stopPolling, refresh } = useQuery(queryFunc, [defaultResponse], {
115119
pollInterval: 15,
116120
})
117121

@@ -131,6 +135,7 @@ export function useAggregate(
131135
response = {
132136
page: response.page,
133137
loading: loading && !usingStoredResponse,
138+
refresh,
134139
items: response.items,
135140
processedItemCount: response.processedItemCount,
136141
emptyResult: response.emptyResult,
@@ -144,6 +149,7 @@ export function useAggregate(
144149
filterCounts: response.filterCounts,
145150
systemAppNSPrefixes: response.systemAppNSPrefixes,
146151
loading: loading && !usingStoredResponse,
152+
refresh,
147153
}
148154
// save response for next time
149155
if (!loading) setWithExpiry(STATUSESKEY, response)

‎frontend/src/routes/Applications/Application.sharedmocks.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,7 @@ export const mockRequestedCounts: IResultStatuses = {
469469
argo: 4,
470470
},
471471
},
472+
refresh: () => {},
472473
systemAppNSPrefixes: [],
473474
loading: false,
474475
}

‎frontend/src/routes/Applications/Overview.tsx

+22-10
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,8 @@ export default function ApplicationsOverview() {
430430
resultCounts.itemCount = resultView.processedItemCount
431431
const { systemAppNSPrefixes } = resultCounts
432432
const allApplications = resultView.items
433+
const { refresh: listRefresh } = resultView
434+
const { refresh: countRefresh } = resultCounts
433435

434436
const fetchAggregateForExport = async (requestedExport: IRequestListView) => {
435437
return fetchAggregate(SupportedAggregate.applications, backendUrl, requestedExport)
@@ -925,6 +927,12 @@ export default function ApplicationsOverview() {
925927
appSetsSharingPlacement: appSetRelatedResources[1],
926928
appKind: resource.kind,
927929
appSetApps: getAppSetApps(argoApplications, resource.metadata?.name!),
930+
deleted: /* istanbul ignore next */ () => {
931+
resultView.refresh()
932+
resultCounts.refresh()
933+
listRefresh()
934+
countRefresh()
935+
},
928936
close: () => {
929937
setModalProps({ open: false })
930938
},
@@ -959,19 +967,23 @@ export default function ApplicationsOverview() {
959967
return actions
960968
},
961969
[
962-
applicationSets,
970+
t,
971+
acmExtensions,
972+
navigate,
973+
canDeleteApplicationSet,
974+
canDeleteApplication,
963975
applications,
976+
subscriptions,
977+
placementRules,
978+
placements,
979+
channels,
980+
applicationSets,
964981
argoApplications,
965-
canDeleteApplication,
966-
canDeleteApplicationSet,
982+
resultView,
983+
resultCounts,
984+
listRefresh,
985+
countRefresh,
967986
canCreateApplication,
968-
channels,
969-
navigate,
970-
placements,
971-
placementRules,
972-
subscriptions,
973-
acmExtensions,
974-
t,
975987
]
976988
)
977989

‎frontend/src/routes/Applications/components/DeleteResourceModal.test.tsx

+12
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ import { nockIgnoreApiPaths } from '../../../lib/nock-util'
1616

1717
const t = i18n.t.bind(i18n)
1818

19+
jest.mock('../../../resources/utils/resource-request', () => ({
20+
deleteResource: jest.fn(() => ({ promise: Promise.resolve() })),
21+
}))
22+
1923
describe('DeleteResourceModal', () => {
2024
it('should render delete ACM app no related resources', () => {
2125
const resource: IResource = {
@@ -42,6 +46,7 @@ describe('DeleteResourceModal', () => {
4246
appSetsSharingPlacement={[]}
4347
appKind={resource.kind}
4448
appSetApps={[]}
49+
deleted={() => void {}}
4550
close={() => void {}}
4651
t={t}
4752
/>
@@ -99,6 +104,7 @@ describe('DeleteResourceModal', () => {
99104
appSetsSharingPlacement={[]}
100105
appKind={resource.kind}
101106
appSetApps={[]}
107+
deleted={() => void {}}
102108
close={() => void {}}
103109
t={t}
104110
/>
@@ -168,6 +174,7 @@ describe('DeleteResourceModal', () => {
168174
appSetsSharingPlacement={[]}
169175
appKind={resource.kind}
170176
appSetApps={[]}
177+
deleted={() => void {}}
171178
close={() => void {}}
172179
t={t}
173180
/>
@@ -230,6 +237,7 @@ describe('DeleteResourceModal', () => {
230237
appSetsSharingPlacement={[]}
231238
appKind={resource.kind}
232239
appSetApps={[]}
240+
deleted={() => void {}}
233241
close={() => void {}}
234242
t={t}
235243
/>
@@ -269,13 +277,15 @@ describe('DeleteResourceModal', () => {
269277
appSetsSharingPlacement={[]}
270278
appKind={resource.kind}
271279
appSetApps={[]}
280+
deleted={() => void {}}
272281
close={() => void {}}
273282
t={t}
274283
/>
275284
</MemoryRouter>
276285
)
277286

278287
expect(getByText('Permanently delete ApplicationSet appset1?')).toBeTruthy()
288+
userEvent.click(screen.getByRole('button', { name: /delete/i }))
279289
})
280290

281291
it('should render delete appset with placement', () => {
@@ -305,6 +315,7 @@ describe('DeleteResourceModal', () => {
305315
appSetsSharingPlacement={[]}
306316
appKind={resource.kind}
307317
appSetApps={appSetApps}
318+
deleted={() => void {}}
308319
close={() => void {}}
309320
t={t}
310321
/>
@@ -349,6 +360,7 @@ describe('DeleteResourceModal', () => {
349360
appSetsSharingPlacement={appSetsSharingPlacement}
350361
appKind={resource.kind}
351362
appSetApps={appSetApps}
363+
deleted={() => void {}}
352364
close={() => void {}}
353365
t={t}
354366
/>

‎frontend/src/routes/Applications/components/DeleteResourceModal.tsx

+8-3
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export interface IDeleteResourceModalProps {
2424
appSetsSharingPlacement?: string[]
2525
appKind: string
2626
appSetApps?: string[]
27+
deleted?: (resource: IResource) => void
2728
close: () => void
2829
t: TFunction
2930
redirect?: string
@@ -55,12 +56,14 @@ export function DeleteResourceModal(props: IDeleteResourceModalProps | { open: f
5556
}
5657

5758
if (props.resource.kind === ApplicationKind) {
58-
return deleteApplication(props.resource, removeAppResources ? props.selected : [])
59+
/* istanbul ignore next */
60+
return deleteApplication(props.resource, removeAppResources ? props.selected : [], props.deleted)
5961
}
6062

6163
if (props.resource.kind === ApplicationSetKind) {
6264
return deleteApplication(
6365
props.resource,
66+
/* istanbul ignore next */
6467
props.appSetsSharingPlacement?.length === 0 && removeAppSetResource
6568
? [
6669
{
@@ -70,10 +73,12 @@ export function DeleteResourceModal(props: IDeleteResourceModalProps | { open: f
7073
namespace: props.resource.metadata?.namespace,
7174
},
7275
]
73-
: []
76+
: [],
77+
props.deleted
7478
)
7579
}
76-
return deleteApplication(props.resource)
80+
/* istanbul ignore next */
81+
return deleteApplication(props.resource, [], props.deleted)
7782
}
7883

7984
const renderConfirmCheckbox = () => {

0 commit comments

Comments
 (0)