cleanup: move provisionStart into placement

[Sarthak-Shreshtha01]

What this PR does / why we need it

Moves provisionStart into the placement struct, so the provisioning start time sits with the other placement data.

Which issue(s) this PR fixes

Fixes #79

Special notes for your reviewer

Does this PR introduce a user-facing change?

NONE

Summary by CodeRabbit

• Bug Fixes
  • Readiness timing is now recorded only after a provisioned instance reaches the Running state, and only once.
  • Pod teardown continues to use the recorded region after readiness is observed.
  • Re-adopted pods and pods without placement metadata are handled without errors.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 5802a411-9929-4e7d-a8d9-e9e36c9a42c0

📥 Commits

Reviewing files that changed from the base of the PR and between ef280f1 and 767092f.

📒 Files selected for processing (2)

• pkg/vnode/handler.go
• pkg/vnode/handler_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Provisioning time now resides with region and capacity tier in an optional placement. Provisioning, readiness observation, teardown, and tests use this placement. Paths without placement handle it as nil.

Changes

Placement-Scoped Provision Timing

Layer / File(s)	Summary
Placement data and provisioning storage pkg/vnode/handler.go, pkg/vnode/handler_test.go	placement now contains the provisioning start time, and trackedPod stores an optional placement pointer. Provisioning passes placement to store; failed provisioning and re-adoption use nil. The readiness-deadline test fixture sets the timestamp through placement.
Readiness observation and teardown pkg/vnode/handler.go, pkg/vnode/handler_test.go	observeReady records readiness duration only for Running instances with placement and a nonzero start time, then clears the timestamp. DeletePod reads the region when placement exists. Tests verify timestamp clearing, placement retention, and handling of nil placement.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: kerthcet

Merge Risk: ⚪ Minimal · up to 76709

No actionable issue remains before merge; normal checks still apply.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 76709

The lifecycle change appears to preserve existing placement checks and cleanup behavior. No new security issue was established, though external retry guarantees remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed state is held inside the vnode handler for tracked pods; the diff does not add a provider operation or change the placement inputs sent to provisioning.

Trust Boundaries and Controls

• observed — Pod-derived placement is checked against the NodeClaim's Pod UID before provisioning. Environment and registry credentials are resolved before the provider call; these checks are outside the changed control flow.

Resilience and Maintainability Implications

• inferred — The handler still excludes in-flight provisioning from tracked state. The diff does not change that pre-existing behavior, while caller serialization and provider idempotency remain unverified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: moving provisionStart into placement.
Linked Issues check	✅ Passed	The changes satisfy issue #79. provisionStart now resides in optional *placement with the region and capacity tier. Provisioning, re-adoption, readiness observation, and deletion handle nil placem…
Out of Scope Changes check	✅ Passed	The reviewed changes stay within issue #79. The production changes reorganize placement and provisioning-start data and update related lifecycle handling. The test changes verify this behavior. No unr…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Sarthak-Shreshtha01]

/kind cleanup

[kerthcet]

/assign

Thanks @Sarthak-Shreshtha01 will take a look later.

[kerthcet]

/lgtm
/approve

[InftyAI-Agent]

Approved: PR has both lgtm and approved labels