Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions docs/claude-progress.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,23 @@
# Claude Progress Log
# Newest entries first. Agents: append your entry at the top after the header.

---
## 2026-06-25 | Session: SRVOCF-841 fix resource coupling
Worked on: Decouple ksvc/deployment pairing from FunctionsListPage into useClusterService
Completed:
- Introduced ClusterFunction interface (name, optional knativeService, optional deployment)
- Moved ksvc-to-deployment pairing logic from FunctionsListPage into useClusterService with useMemo
- Pairing matches by revision label with fallback to function name label
- Changed ClusterService return type from separate arrays to functions: ClusterFunction[]
- Simplified FunctionsListPage enrichment to a name-based lookup
- Renamed FunctionTableItem.deployment to mainResource (it holds the ksvc, not a Deployment)
- Fixed FunctionTable.test.tsx fixture to use Knative Service shape instead of Deployment
- Added 4 new hook-level pairing tests (revision match, fallback, multiple revisions, empty)
- Removed duplicate "picks latest revision" test from FunctionsListPage (now tested in hook)
- All 147 tests passing, lint clean, build succeeds
Left off: Changes staged, ready for commit.
Blockers: None

---
## 2026-06-24 | Session: Fix pre-commit hook error handling
Worked on: Fix pre-commit hook to fail on lint/type-check/go vet errors
Expand Down
13 changes: 13 additions & 0 deletions docs/features.json
Original file line number Diff line number Diff line change
Expand Up @@ -202,5 +202,18 @@
"Document usage in README or script header"
],
"passes": true
},
{
"category": "technical",
"description": "useClusterService: fix resource coupling and improve tests. Knative Services and Deployments are co-present via OwnerReferences, clarify this invariant and simplify to a merged return type.",
"steps": [
"Merge separate knativeServices/deployments arrays into a single paired return type (e.g. ClusterFunction[]) that pairs each ksvc with its deployment",
"Move ksvc-to-deployment matching logic from FunctionsListPage into useClusterService",
"Rename FunctionTableItem.deployment to knativeService (it stores the ksvc, not the deployment)",
"Update useClusterService tests to reflect the co-presence invariant (no test should have a ksvc without a deployment or vice versa)",
"Update FunctionsListPage to consume the simplified return type",
"All tests pass, no broken imports"
],
"passes": false
}
]
127 changes: 103 additions & 24 deletions src/common/services/cluster/useClusterService.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { render, screen } from '@testing-library/react';
import { useClusterService } from './useClusterService';
import { useClusterService, ClusterFunction } from './useClusterService';

const mockUseK8sWatchResource = vi.fn();

Expand Down Expand Up @@ -38,22 +38,18 @@ const mockDeployment = {
};

function TestConsumer({ functionNames = [] }: { functionNames?: string[] }) {
const { knativeServices, deployments, loaded, error } = useClusterService(functionNames);
const { functions, loaded, error } = useClusterService(functionNames);
return (
<>
<span data-testid="loaded">{String(loaded)}</span>
<span data-testid="error">{String(error)}</span>
<span data-testid="ksvc-count">{knativeServices.length}</span>
<span data-testid="dep-count">{deployments.length}</span>
{knativeServices.map((s) => (
<span key={s.metadata?.name} data-testid="ksvc">
{s.metadata?.name}
</span>
))}
{deployments.map((d) => (
<span key={d.metadata?.name} data-testid="deployment">
{d.metadata?.name}
</span>
<span data-testid="fn-count">{functions.length}</span>
{functions.map((fn: ClusterFunction) => (
<div key={fn.name} data-testid="cluster-fn">
<span data-testid="fn-name">{fn.name}</span>
<span data-testid="has-ksvc">{String(!!fn.knativeService)}</span>
<span data-testid="has-dep">{String(!!fn.deployment)}</span>
</div>
))}
</>
);
Expand All @@ -71,14 +67,13 @@ describe('useClusterService', () => {

expect(mockUseK8sWatchResource).toHaveBeenCalledWith(null);
expect(screen.getByTestId('loaded')).toHaveTextContent('true');
expect(screen.getByTestId('ksvc-count')).toHaveTextContent('0');
expect(screen.getByTestId('dep-count')).toHaveTextContent('0');
expect(screen.getByTestId('fn-count')).toHaveTextContent('0');
});

it('watches Knative Services with In selector for given function names', () => {
mockUseK8sWatchResource
.mockReturnValueOnce([[mockKsvc], true, null])
.mockReturnValueOnce([[], true, null]);
.mockReturnValueOnce([[mockDeployment], true, null]);

render(<TestConsumer functionNames={['my-func']} />);

Expand All @@ -91,13 +86,11 @@ describe('useClusterService', () => {
],
},
});
expect(screen.getByTestId('ksvc-count')).toHaveTextContent('1');
expect(screen.getByTestId('ksvc')).toHaveTextContent('my-func');
});

it('watches Deployments with In selector for given function names', () => {
mockUseK8sWatchResource
.mockReturnValueOnce([[], true, null])
.mockReturnValueOnce([[mockKsvc], true, null])
.mockReturnValueOnce([[mockDeployment], true, null]);

render(<TestConsumer functionNames={['my-func']} />);
Expand All @@ -111,19 +104,105 @@ describe('useClusterService', () => {
],
},
});
expect(screen.getByTestId('dep-count')).toHaveTextContent('1');
expect(screen.getByTestId('deployment')).toHaveTextContent('my-func-00001-deployment');
});

it('returns empty arrays when not loaded', () => {
it('returns empty functions array when not loaded', () => {
mockUseK8sWatchResource
.mockReturnValueOnce([[], false, null])
.mockReturnValueOnce([[], false, null]);

render(<TestConsumer functionNames={['my-func']} />);

expect(screen.getByTestId('loaded')).toHaveTextContent('false');
expect(screen.getByTestId('ksvc-count')).toHaveTextContent('0');
expect(screen.getByTestId('dep-count')).toHaveTextContent('0');
expect(screen.getByTestId('fn-count')).toHaveTextContent('0');
});

it('pairs ksvc with deployment by revision label', () => {
mockUseK8sWatchResource
.mockReturnValueOnce([[mockKsvc], true, null])
.mockReturnValueOnce([[mockDeployment], true, null]);

render(<TestConsumer functionNames={['my-func']} />);

expect(screen.getByTestId('fn-count')).toHaveTextContent('1');
expect(screen.getByTestId('fn-name')).toHaveTextContent('my-func');
expect(screen.getByTestId('has-ksvc')).toHaveTextContent('true');
expect(screen.getByTestId('has-dep')).toHaveTextContent('true');
});

it('falls back to function name label when no latestReadyRevisionName', () => {
const ksvcNoRevision = {
...mockKsvc,
status: { ...mockKsvc.status, latestReadyRevisionName: undefined },
};
const depByName = {
...mockDeployment,
metadata: {
...mockDeployment.metadata,
labels: { 'function.knative.dev/name': 'my-func' },
},
};

mockUseK8sWatchResource
.mockReturnValueOnce([[ksvcNoRevision], true, null])
.mockReturnValueOnce([[depByName], true, null]);

render(<TestConsumer functionNames={['my-func']} />);

expect(screen.getByTestId('fn-count')).toHaveTextContent('1');
expect(screen.getByTestId('has-dep')).toHaveTextContent('true');
});

it('picks latest revision deployment when multiple revisions exist', () => {
const ksvcV2 = {
...mockKsvc,
status: { ...mockKsvc.status, latestReadyRevisionName: 'my-func-00002' },
};
const depV1 = {
...mockDeployment,
metadata: {
...mockDeployment.metadata,
name: 'my-func-00001-deployment',
labels: {
'function.knative.dev/name': 'my-func',
'serving.knative.dev/revision': 'my-func-00001',
},
},
spec: { replicas: 0 },
status: { readyReplicas: 0 },
};
const depV2 = {
...mockDeployment,
metadata: {
...mockDeployment.metadata,
name: 'my-func-00002-deployment',
labels: {
'function.knative.dev/name': 'my-func',
'serving.knative.dev/revision': 'my-func-00002',
},
},
spec: { replicas: 1 },
status: { readyReplicas: 1 },
};

mockUseK8sWatchResource
.mockReturnValueOnce([[ksvcV2], true, null])
.mockReturnValueOnce([[depV1, depV2], true, null]);

render(<TestConsumer functionNames={['my-func']} />);

expect(screen.getByTestId('fn-count')).toHaveTextContent('1');
expect(screen.getByTestId('has-dep')).toHaveTextContent('true');
});

it('returns empty functions array when no resources match', () => {
mockUseK8sWatchResource
.mockReturnValueOnce([[], true, null])
.mockReturnValueOnce([[], true, null]);

render(<TestConsumer functionNames={['my-func']} />);

expect(screen.getByTestId('loaded')).toHaveTextContent('true');
expect(screen.getByTestId('fn-count')).toHaveTextContent('0');
});
});
35 changes: 31 additions & 4 deletions src/common/services/cluster/useClusterService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,16 @@ import { OcpClusterService } from './OcpClusterService';
const instance = new OcpClusterService();

const FUNCTION_NAME_LABEL = 'function.knative.dev/name';
const REVISION_LABEL = 'serving.knative.dev/revision';

export interface ClusterFunction {
name: string;
knativeService?: K8sResourceKind;
deployment?: K8sResourceKind;
}

interface ClusterService {
knativeServices: K8sResourceKind[];
deployments: K8sResourceKind[];
functions: ClusterFunction[];
loaded: boolean;
error: unknown;
generateKubeconfig: (namespace: string) => Promise<string>;
Expand Down Expand Up @@ -50,11 +56,32 @@ export function useClusterService(functionNames: string[] = []): ClusterService
const [knSvcs, knLoaded, knError] = useK8sWatchResource<K8sResourceKind[]>(knSvcConfig);
const [deps, depLoaded, depError] = useK8sWatchResource<K8sResourceKind[]>(depConfig);

const functions = useMemo(() => {
const safeKnSvcs = knLoaded ? (knSvcs ?? []) : [];
const safeDeps = depLoaded ? (deps ?? []) : [];
return pairResources(safeKnSvcs, safeDeps);
}, [knSvcs, knLoaded, deps, depLoaded]);
Comment on lines +59 to +63

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const functions = useMemo(() => {
const safeKnSvcs = knLoaded ? (knSvcs ?? []) : [];
const safeDeps = depLoaded ? (deps ?? []) : [];
return pairResources(safeKnSvcs, safeDeps);
}, [knSvcs, knLoaded, deps, depLoaded]);
const functions = useMemo(
() => pairResources(
knLoaded ? (knSvcs ?? []) : [],
depLoaded ? (deps ?? []) : [],
),
[knSvcs, knLoaded, deps, depLoaded],
);


return {
knativeServices: knLoaded ? (knSvcs ?? []) : [],
deployments: depLoaded ? (deps ?? []) : [],
functions,
loaded: knLoaded && depLoaded,
error: knError || depError,
generateKubeconfig: instance.generateKubeconfig.bind(instance),
};
}

function pairResources(
knSvcs: K8sResourceKind[],
deployments: K8sResourceKind[],
): ClusterFunction[] {
return knSvcs.map((ksvc) => {
const name = ksvc.metadata?.labels?.[FUNCTION_NAME_LABEL] ?? ksvc.metadata?.name ?? '';
const latestRevision = ksvc.status?.latestReadyRevisionName;

const deployment = latestRevision
? deployments.find((d) => d.metadata?.labels?.[REVISION_LABEL] === latestRevision)
: deployments.find((d) => d.metadata?.labels?.[FUNCTION_NAME_LABEL] === name);

return { name, knativeService: ksvc, deployment: deployment ?? undefined };
});
}
Loading