Skip to content

Telemetry store agent tested and dusted #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
amshithnair merged 7 commits into from
Jul 27, 2026
Merged

Telemetry store agent tested and dusted #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fd61f11
feat(telemetry): add NodeTelemetry and GPUTelemetry data models
amshithnair Jul 27, 2026
1cd5542
test(telemetry): add comprehensive tests for data model validation an…
amshithnair Jul 27, 2026
88b5feb
feat(api): add GPUNodeStatus CRD Go types with hand-written DeepCopy
amshithnair Jul 27, 2026
38fba99
feat(store): add thread-safe TelemetryStore with RWMutex and tests
amshithnair Jul 27, 2026
7e310cd
feat(agent): add polling telemetry agent with retry logic and tests
amshithnair Jul 27, 2026
ca5d71f
feat(manifests): add GPUNodeStatus CRD with status subresource
amshithnair Jul 27, 2026
02f99ef
docs: add Phase 3 documentation, verification scripts, and manual che…
amshithnair Jul 27, 2026
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
390 changes: 390 additions & 0 deletions checklists/phase3-manual-checklist.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,390 @@
# Phase 3 Manual Verification Guide

> Complete every item in order. Check each box only after you have personally observed the
> described outcome. Do not proceed to Phase 4 until all boxes are checked.

---

## Prerequisites

- [ ] Go 1.22 or later is installed.
- Verify: `go version` prints `go1.22` or higher.
- [ ] You are in the repository root directory.
- Verify: `Test-Path go.mod` returns `True` (PowerShell) or `ls go.mod` succeeds (bash).
- [ ] `go.sum` exists (should have been created in Phase 1).
- [ ] Phase 1 and Phase 2 tests still pass.
- Verify: `go test ./pkg/telemetry/... ./pkg/scoring/... -count=1` shows `ok` for both packages.

---

## Section 1 — File Existence

Confirm every Phase 3 file is present on disk.

**Data Model & Telemetry**
- [ ] `pkg/telemetry/model.go` exists.
- [ ] `pkg/telemetry/model_test.go` exists.

**CRD Go Types**
- [ ] `pkg/api/v1alpha1/types.go` exists.
- [ ] `pkg/api/v1alpha1/deepcopy.go` exists.

**Telemetry Store**
- [ ] `pkg/store/store.go` exists.
- [ ] `pkg/store/store_test.go` exists.

**Polling Agent**
- [ ] `pkg/agent/agent.go` exists.
- [ ] `pkg/agent/agent_test.go` exists.

**CRD Manifest**
- [ ] `manifests/gpunodestatus-crd.yaml` exists.

**Documentation**
- [ ] `docs/phase3.md` exists.

**Verification Scripts**
- [ ] `scripts/phase3-test.ps1` exists.
- [ ] `scripts/phase3-test.sh` exists.
- [ ] `scripts/phase3-manual.ps1` exists.
- [ ] `scripts/phase3-manual.sh` exists.

---

## Section 2 — Module Correctness

- [ ] Open `go.mod`. Confirm the module line reads:
```
module github.com/amshithnair/gpu-aware-scheduler
```
- [ ] Confirm the `go` directive is `1.22` or higher.
- [ ] Run `go build ./...` — no errors. All Phase 3 packages compile cleanly.
- [ ] Run `go vet ./...` — no warnings or errors.

---

## Section 3 — Automated Script Execution (Windows)

Run the PowerShell verification script and observe its output.

```powershell
cd "C:\Users\Amshith Nair\gpu-aware-scheduler"
.\scripts\phase3-test.ps1
```

- [ ] **Step 1 — Validate tools**: Script detects `go` and prints its version. Missing optional
tools (kubectl, kind, docker, golangci-lint) are reported as `[SKIP]`, not `[FAIL]`.
- [ ] **Step 2 — go fmt**: Script prints `[PASS] STEP 2: go fmt ./...` with no reformatted files.
- [ ] **Step 3 — go vet**: Script prints `[PASS] STEP 3: go vet ./...` with no issues.
- [ ] **Step 4 — golangci-lint**: Either prints `[PASS]` or `[SKIP]` (if not installed).
Both are acceptable for verification.
- [ ] **Step 5 — go build**: Script prints `[PASS] STEP 5: go build ./...` with no errors.
- [ ] **Step 6 — go test**: Script prints `[PASS] STEP 6: go test ./... -v -count=1 -cover`.
- The `-v` output shows every test function ending with `--- PASS`.
- No test ends with `--- FAIL` or `--- SKIP`.
- Coverage numbers are printed for each package.
- Final lines include:
```
ok github.com/amshithnair/gpu-aware-scheduler/pkg/agent ...
ok github.com/amshithnair/gpu-aware-scheduler/pkg/scoring ...
ok github.com/amshithnair/gpu-aware-scheduler/pkg/store ...
ok github.com/amshithnair/gpu-aware-scheduler/pkg/telemetry ...
```
- [ ] **Step 7 — go test -race**: Either `[PASS]` (Linux/macOS with CGo) or
`[SKIP] CGO_ENABLED=0` (Windows without C compiler). Both are acceptable.
- [ ] **Summary banner**: Script prints `ALL PHASE 3 CHECKS PASSED`.
- [ ] **Exit code**: Script exits with code 0.
- Verify: `echo $LASTEXITCODE` prints `0`.

---

## Section 4 — Individual Test Inventory

Open the `-v` test output from Step 6 and confirm each of the following test functions appeared
and passed. Cross off each one as you find it.

### 4A — `pkg/telemetry` — `model_test.go` (23 tests)

**GPUTelemetry.Validate**
- [ ] `TestGPUTelemetry_Validate_Happy`
- [ ] `TestGPUTelemetry_Validate_EmptyGPUID`
- [ ] `TestGPUTelemetry_Validate_UtilizationOutOfRange` (3 sub-tests: negative, above 1, way above)
- [ ] `TestGPUTelemetry_Validate_UnknownUtilization_NoError`
- [ ] `TestGPUTelemetry_Validate_NegativeFreeMemory`
- [ ] `TestGPUTelemetry_Validate_ZeroTotalMemory`
- [ ] `TestGPUTelemetry_Validate_FreeExceedsTotal`

**GPUTelemetry helpers**
- [ ] `TestGPUTelemetry_IsHealthy`
- [ ] `TestGPUTelemetry_DeepCopy_Independence`

**NodeTelemetry.Validate**
- [ ] `TestNodeTelemetry_Validate_Happy`
- [ ] `TestNodeTelemetry_Validate_EmptyNodeID`
- [ ] `TestNodeTelemetry_Validate_ZeroCollectedAt`
- [ ] `TestNodeTelemetry_Validate_InvalidSchemaVersion`
- [ ] `TestNodeTelemetry_Validate_DuplicateGPUID`
- [ ] `TestNodeTelemetry_Validate_InvalidGPU`

**NodeTelemetry.IsStale**
- [ ] `TestNodeTelemetry_IsStale_ZeroTimestamp`
- [ ] `TestNodeTelemetry_IsStale_Fresh`
- [ ] `TestNodeTelemetry_IsStaleAt_Deterministic`

**NodeTelemetry.DeepCopy**
- [ ] `TestNodeTelemetry_DeepCopy_GPUSliceIndependence`
- [ ] `TestNodeTelemetry_DeepCopy_TopologyIndependence`
- [ ] `TestNodeTelemetry_DeepCopy_NilTopology`
- [ ] `TestNodeTelemetry_DeepCopy_NilGPUs`

**TopologyMatrix deep copy**
- [ ] `TestDeepCopyTopologyMatrix_Empty`

---

### 4B — `pkg/store` — `store_test.go` (20 tests)

**CRUD operations**
- [ ] `TestStore_SetAndGet`
- [ ] `TestStore_Get_UnknownNode`
- [ ] `TestStore_Set_Replaces`
- [ ] `TestStore_Delete`
- [ ] `TestStore_Delete_NoOp_UnknownNode`
- [ ] `TestStore_Len`

**List / Snapshot**
- [ ] `TestStore_List_Empty`
- [ ] `TestStore_List_AllNodes`
- [ ] `TestStore_List_DeepCopy`
- [ ] `TestStore_Snapshot_Empty`
- [ ] `TestStore_Snapshot_Independence`

**Staleness sweep**
- [ ] `TestStore_MarkStaleAt_OldRecord`
- [ ] `TestStore_MarkStaleAt_FreshRecord`
- [ ] `TestStore_MarkStaleAt_AlreadyStale_NotModified`
- [ ] `TestStore_MarkStaleAt_MixedRecords`

**Deep-copy contract**
- [ ] `TestStore_Get_DeepCopy`

**Concurrency**
- [ ] `TestStore_ConcurrentReadsAndWrites`
- [ ] `TestStore_ConcurrentMarkStale`
- [ ] `TestStore_ConcurrentSnapshot`
- [ ] `TestStore_ConcurrentSetAndDelete`

---

### 4C — `pkg/agent` — `agent_test.go` (17 tests)

**Constructor validation**
- [ ] `TestNewAgent_RequiresNodeID`
- [ ] `TestNewAgent_RequiresGPUIDs`
- [ ] `TestNewAgent_RequiresProvider`
- [ ] `TestNewAgent_RequiresStore`
- [ ] `TestNewAgent_AppliesDefaults`

**PollOnce**
- [ ] `TestAgent_PollOnce_HappyPath`
- [ ] `TestAgent_PollOnce_PerGPU_ProviderError`
- [ ] `TestAgent_PollOnce_AllGPUs_ProviderError`
- [ ] `TestAgent_PollOnce_TopologyError_NonFatal`
- [ ] `TestAgent_PollOnce_MultipleGPUs_Independent`

**Run loop & lifecycle**
- [ ] `TestAgent_RunOneTick_WritesToStore`
- [ ] `TestAgent_Run_ContextCancellation`
- [ ] `TestAgent_Run_ShutdownMarksStale`

**MetricsHook**
- [ ] `TestAgent_MetricsHook_OnPollComplete`
- [ ] `TestAgent_MetricsHook_OnStoreWrite`

**Staleness**
- [ ] `TestAgent_StalenessThreshold_Applied`

**Concurrency**
- [ ] `TestAgent_Race_ConcurrentReaders`

---

**Total expected: 60 test functions** across 3 packages (23 + 20 + 17).

> [!NOTE]
> The `pkg/telemetry/provider_test.go` tests (Phase 1) also run during `go test ./...` but are
> not counted here — they were verified in the Phase 1 checklist. Similarly for `pkg/scoring`
> (Phase 2).

---

## Section 5 — CRD Manifest Validation

### 5A — Structure review (manual code read)

Open `manifests/gpunodestatus-crd.yaml` and verify:

- [ ] `apiVersion` is `apiextensions.k8s.io/v1`.
- [ ] `metadata.name` is `gpunodestatuses.gpu.amshithnair.dev`.
- [ ] `spec.group` is `gpu.amshithnair.dev`.
- [ ] `spec.names.kind` is `GPUNodeStatus`.
- [ ] `spec.names.plural` is `gpunodestatuses`.
- [ ] `spec.names.shortNames` includes `gpuns`.
- [ ] `spec.scope` is `Namespaced`.
- [ ] `spec.versions[0].name` is `v1alpha1`.
- [ ] `spec.versions[0].served` is `true`.
- [ ] `spec.versions[0].storage` is `true`.
- [ ] `subresources.status: {}` is present (status subresource enabled).
- [ ] `additionalPrinterColumns` includes columns for: `Node`, `Stale`, `LastUpdated`, `Age`.

### 5B — Schema fields (manual code read)

Verify the OpenAPI v3 schema contains:

- [ ] `.spec.nodeName` — type: string, required, minLength: 1.
- [ ] `.spec.gpuIDs` — type: array of strings, required, minItems: 1, maxItems: 64.
- [ ] `.status.gpus` — type: array of objects with fields:
`gpuID`, `utilizationPct`, `utilizationKnown`, `freeMemoryMB`, `totalMemoryMB`,
`memoryKnown`, `eccErrors`, `eccKnown`.
- [ ] `.status.lastUpdated` — type: string, format: date-time.
- [ ] `.status.stale` — type: boolean.
- [ ] `.status.schemaVersion` — type: integer, minimum: 1.

### 5C — Dry-run validation

```powershell
kubectl apply --dry-run=client -f manifests/gpunodestatus-crd.yaml
```

- [ ] Command succeeds with output:
```
customresourcedefinition.apiextensions.k8s.io/gpunodestatuses.gpu.amshithnair.dev configured (dry run)
```
If kubectl is not available, mark as `[SKIP]` — this will be verified during cluster testing.

---

## Section 6 — CRD Go Types Validation (manual code read)

Open `pkg/api/v1alpha1/types.go` and verify:

- [ ] `GPUNodeStatus` struct has `TypeMeta`, `ObjectMeta`, `Spec`, and `Status` fields.
- [ ] `GPUNodeStatusSpec` has `NodeName string` and `GPUIDs []string`.
- [ ] `GPUNodeStatusStatus` has `GPUs []GPUStatusEntry`, `LastUpdated string`, `Stale bool`,
`SchemaVersion int`.
- [ ] `GPUStatusEntry` mirrors the CRD schema fields (gpuID, utilizationPct, etc.).
- [ ] JSON tags on all fields match the CRD YAML field names exactly.

Open `pkg/api/v1alpha1/deepcopy.go` and verify:

- [ ] `DeepCopyInto` methods exist for all CRD types.
- [ ] `DeepCopyObject` method exists on `GPUNodeStatus`, returning `runtime.Object`.
- [ ] GPU slice deep copy creates a new slice (not just a header copy).

Comment on lines +269 to +283

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Checklist assertions don't match the actual Go types.

Two items here can never pass as written:

  • Line 273 says LastUpdated string, but pkg/api/v1alpha1/types.go declares LastUpdated time.Time.
  • Line 281 requires a DeepCopyObject method returning runtime.Object, but pkg/api/v1alpha1/deepcopy.go has no such method — and can't, since the package doc in types.go explicitly states Phase 3 avoids importing k8s.io/apimachinery (so runtime.Object isn't available). This item is currently unsatisfiable, which blocks the checklist's own sign-off gate ("Do not proceed to Phase 4 until all boxes are checked").

Update the checklist to match the current type (time.Time) and either drop the DeepCopyObject requirement or explicitly defer it to Phase 4.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@checklists/phase3-manual-checklist.md` around lines 269 - 283, Update the
Phase 3 checklist assertions to match the implemented API: change
GPUNodeStatusStatus.LastUpdated from string to time.Time, and remove the
DeepCopyObject/runtime.Object requirement from the deepcopy verification or
explicitly defer it to Phase 4. Keep the remaining deep-copy and field/tag
checks unchanged.

---

## Section 7 — Manual Cluster Verification (optional)

> [!IMPORTANT]
> This section requires a live Kubernetes cluster (kind, minikube, or real).
> If no cluster is available, mark all items as `[SKIP — no cluster]`.
> The automated scripts in Sections 3–4 already cover code correctness.

### 7A — Run the manual script

**Windows:**
```powershell
.\scripts\phase3-manual.ps1
```

**Linux / macOS:**
```bash
./scripts/phase3-manual.sh
```

### 7B — Verify script sections

- [ ] **Section A — CRD Lifecycle**:
- [ ] A1: Dry-run passes (`[PASS]`).
- [ ] A2: CRD applied to cluster.
- [ ] A3: `kubectl get crd gpunodestatuses.gpu.amshithnair.dev` returns the CRD.
- [ ] A4: `kubectl describe crd` shows the schema and printer columns.

- [ ] **Section B — Sample Object**:
- [ ] B1: `gpu-scheduler` namespace created.
- [ ] B2: Sample `GPUNodeStatus` object `node-gpu-01` created.
- [ ] B3: `kubectl get gpuns -n gpu-scheduler` shows the object with printer columns
(Node, Stale, LastUpdated, Age).
- [ ] B4: `kubectl describe gpuns node-gpu-01` shows Spec with nodeName and gpuIDs.
- [ ] B5: Status subresource patched — stale=false, two GPU entries visible.
- [ ] B6: `kubectl get gpuns node-gpu-01 -o yaml` shows populated `.status` with GPU data.

- [ ] **Section C — Cleanup** (bash script only; PS1 skips directly to cleanup):
- [ ] Stale flag set to `true` via status patch.
- [ ] `kubectl get gpuns` shows `Stale = true` in printer column.

- [ ] **Section D — Cleanup**:
Comment on lines +322 to +326

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Incorrect claim about PS1 script behavior.

This states Section C ("Cleanup"... actually Stale Simulation) is "bash script only; PS1 skips directly to cleanup." But scripts/phase3-manual.ps1 implements SECTION C: Stale Simulation (patches stale=true and verifies it) identically to the bash script. The section label itself also appears mislabeled as "Cleanup" when both scripts title it "Stale Simulation."

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@checklists/phase3-manual-checklist.md` around lines 322 - 326, The
checklist’s Section C description is inaccurate: rename it from “Cleanup” to
“Stale Simulation” and remove the claim that the PS1 script skips it, while
retaining the stale flag patch and verification steps for both scripts.

- [ ] Sample object deleted.
- [ ] CRD deleted from cluster.
- [ ] CRD confirmed removed (`[PASS]`).

- [ ] **Summary**: Script prints `PHASE 3 MANUAL VERIFICATION COMPLETE`.

---

## Section 8 — Cross-Phase Regression Check

Confirm that Phase 3 did not break Phase 1 or Phase 2.

```powershell
go test ./pkg/telemetry/... ./pkg/scoring/... -v -count=1
```

- [ ] `pkg/telemetry` — all Phase 1 tests pass (18 tests).
- [ ] `pkg/scoring` — all Phase 2 tests pass.
- [ ] No import cycle errors or compilation failures.

---

## Section 9 — Documentation Review

- [ ] Open `docs/phase3.md`. Confirm it contains all of the following sections:
- Overview
- Architecture diagram
- Telemetry Flow
- Data Model (GPUTelemetry + NodeTelemetry tables)
- Store Design (API, deep-copy contract, lock granularity)
- Agent Design (AgentConfig, MetricsHook, error handling policy)
- CRD Explanation (identity, status subresource, namespace convention, example YAML)
- Concurrency Model
- Staleness Model
- Failure Handling
- Testing Guide
- Future Extension Points
- Known Limitations
- Rollback Notes

---

## Section 10 — Output Artifact

- [ ] Copy the full terminal output from Section 3 (phase3-test.ps1) and save it for the
Phase 3 verification record.
- [ ] If Section 7 was executed, copy the full terminal output from the manual cluster
verification script and append it to the verification record.
- [ ] Confirm the pasted output matches what you actually observed (no editing of results).

---

## Phase 3 Sign-off

- [ ] All items in Sections 1–6 and Section 8–9 are checked.
- [ ] Section 7 is either fully checked or all items marked `[SKIP — no cluster]`.
- [ ] Section 10 output artifact is saved.
- [ ] Zero test failures across all packages.

**Phase 3 is complete. You may proceed to Phase 4.**

---

*Checklist version: Phase 3 — created 2026-07-28*
Loading