Skip to content

refactor: replace wiremind_kubernetes with local helpers (ch-32z)#50

Draft
zapaan wants to merge 3 commits intomainfrom
polecat/chrome/ch-32z@mmbuu6uj
Draft

refactor: replace wiremind_kubernetes with local helpers (ch-32z)#50
zapaan wants to merge 3 commits intomainfrom
polecat/chrome/ch-32z@mmbuu6uj

Conversation

@zapaan
Copy link
Contributor

@zapaan zapaan commented Mar 4, 2026

Implements ch-32z by removing the wiremind_kubernetes dependency from chartreuse and migrating to in-repo helpers.

Changes:

  • add local KubernetesDeploymentManager + kube config loader (chartreuse.utils.kubernetes_helper)
  • add local run_command wrapper (chartreuse.utils.command)
  • migrate runtime and tests to local helper imports
  • remove wiremind-kubernetes from project dependencies (keep direct kubernetes)

Validation:

  • uv run --extra test pytest src/chartreuse/tests/unit_tests -q (88 passed)

@zapaan zapaan force-pushed the polecat/chrome/ch-32z@mmbuu6uj branch from 626151f to f6d1d33 Compare March 4, 2026 15:33
@zapaan
Copy link
Contributor Author

zapaan commented Mar 4, 2026

Independent review summary (duplication/usefulness/behavior/tests):

Useful direction overall: moving helper logic in-repo removes an external dependency and keeps ownership local.

I found blocking behavioral parity concerns to address before merge:

  1. stop_pods() scope changed:
  • New local helper scales all release deployments selected by labels.
  • Previous wiremind_kubernetes behavior targeted deployments declared by ExpectedDeploymentScale.
  • This increases blast radius and can stop workloads that were intentionally out-of-scope.
  1. HPA + stop-wait semantics were dropped:
  • Previous flow disabled HPA before scale-down, re-enabled on start, and waited for actual stop.
  • New flow does not disable HPA and does not wait for stop completion before returning.
  • That can race migrations (pods not actually down yet, or HPA scaling back up).
  1. EDS priority ordering removed:
  • Previous manager handled priority-based ordering for stop/start.
  • New implementation flattens expected scales, changing ordering guarantees.
  1. E2E safety fixture regression:
  • Replacing imported setUpE2E/create_namespace with no-op local fixtures removes the former cluster safety guard.

Test gap:

  • Please add local helper unit tests covering EDS filtering scope, HPA handling, priority/order, and stop-wait behavior.

If these changes are intentional (not parity), please document the behavior change explicitly and why the broader stop/start semantics are safe for chartreuse.

@zapaan
Copy link
Contributor Author

zapaan commented Mar 4, 2026

Quality-gate update for helper pruning after rebase to main:\n\n- Reviewed local Kubernetes helper surface for duplication/dead carry-over.\n- Kept the runtime/test-relevant surface: load_kubernetes_config, KubernetesDeploymentManager.stop_pods, start_pods, is_deployment_stopped, including ExpectedDeploymentScale + previous-replicas restore logic used by upgrade flow.\n- Removed unused carry-over: dropped never-used CoreV1Api client initialization/state in kubernetes_helper.py.\n\nVerification on this branch:\n- ruff check .\n- ruff format --check .\n- pytest --verbose . (88 passed)\n\nWill keep the issue/PR open pending approval + required checks green, per policy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant