Installables: submodules fix

[podkidyshev]

Summary

Issue:

1. Install a git repo with init_submodules = false
2. Change init_submodules to true
3. Run the workload - submodules aren't installed

Test Plan

• Automated CI
• Manual cloudai install/run runs on EOS

Additional Notes

[coderabbitai]

📝 Walkthrough

Walkthrough

Centralizes Git submodule handling by adding GitRepo.check_submodules_state() and GitRepo.ensure_submodules_state(), updating Kubernetes and SLURM installers to call these methods, and adding unit tests for submodule-state inspection and reconciliation.

Changes

Cohort / File(s)	Summary
Core GitRepo Submodule Methods src/cloudai/_core/installables.py	Added check_submodules_state(repo_path) which runs git submodule status --recursive and interprets status prefixes vs. self.init_submodules, and ensure_submodules_state(repo_path) which calls the checker and runs either git submodule update --init --recursive or git submodule deinit --all --force to reconcile state, returning detailed (bool, message) results.
Installer Callsites src/cloudai/systems/kubernetes/kubernetes_installer.py, src/cloudai/systems/slurm/slurm_installer.py	Replaced inline _init_submodules subprocess logic with calls to item.ensure_submodules_state(repo_path) after clone/checkout and item.check_submodules_state(repo_path) when verifying existing repos; failures now propagate the returned messages and cloned repos are cleaned up on failure.
Installer Tests Updated tests/test_git_repo_installer.py	Reworked tests to mock GitRepo.check_submodules_state and GitRepo.ensure_submodules_state (new fixtures), replacing prior _init_submodules mocks; updated assertions for cleanup and submodule-related paths.
New Unit Tests for GitRepo tests/test_installables.py	New pytest module exercising check_submodules_state() and ensure_submodules_state() by patching subprocess.run to simulate git submodule status outputs, reconciliation commands, success/failure, and stderr propagation.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Poem

🐇 I hop through trees of branches and tags,

I sniff the submodules, mend holes and rags.
If states disagree I nudge them right,
or tidy the nest in the pale moonlight. 🥕

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically refers to fixing submodules handling in installables, which is the main focus of all changeset modifications.
Description check	✅ Passed	The description clearly articulates the problem being fixed: submodules not being installed when init_submodules changes from false to true after initial installation.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ipod/mbridge/submodules-fix

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cloudai/_core/installables.py`:
- Around line 139-143: The current submodule-status check using
submodules_initialized = (line.startswith(" ") for line in output) misses '+'
and 'U' prefixes; update the logic so when self.init_submodules is False you
verify all submodules are uninitialized (i.e., lines start with '-'), and when
self.init_submodules is True you detect initialized states (space, '+', or 'U')
as initialized; change the generator/conditions around submodules_initialized
and the two checks that call all(...) / any(...) (referencing
submodules_initialized and the branches using self.init_submodules) to
explicitly test for the correct set of prefixes (e.g., startswith one of (' ',
'+', 'U') for initialized and startswith('-') for uninitialized).

In `@tests/test_installables.py`:
- Around line 43-56: Add test cases to the pytest.mark.parametrize matrices in
tests/test_installables.py to cover git submodule status prefixes '+' (and
optionally 'U') which indicate populated checkouts; treat '+' like the space
prefix (initialized) in the expected outcomes. Concretely, add tuples where
stdout starts with "+0123456789abcdef path/to/submodule\n" mirroring the
existing space-prefixed cases in both parametrize blocks so that when
init_submodules=False the '+' case yields the same
expected_result/expected_message as the space case, and when
init_submodules=True the '+' case yields the same result as the space case;
update both parameter lists that currently only include " " and "-" to include
"+" (and "U" if desired).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e7515511-fe63-4bff-9eda-bbf8a67ec4d0

📥 Commits

Reviewing files that changed from the base of the PR and between 7638f1b and 088293b.

📒 Files selected for processing (5)

• src/cloudai/_core/installables.py
• src/cloudai/systems/kubernetes/kubernetes_installer.py
• src/cloudai/systems/slurm/slurm_installer.py
• tests/test_git_repo_installer.py
• tests/test_installables.py

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cloudai/_core/installables.py`:
- Around line 149-157: The check_submodules_state call can fail with a non-empty
error message but the code currently ignores that and proceeds to mutate
submodules; update the logic around check_submodules_state in the installables
code so that after calling check_submodules_state(repo_path) you immediately
return False with the provided error string when the second value is non-empty
(or when submodules_are_ok is False and an error message exists) instead of
falling through to run git submodule update/deinit; reference the
check_submodules_state return tuple, the init_submodules flag, repo_path, and
the existing subprocess.run(["git", "submodule", *cmd], ...) invocation to
locate where to add this early-return.

In `@tests/test_installables.py`:
- Around line 121-160: Add a regression case to tests/test_installables.py for
ensure_submodules_state() where the initial subprocess.run (the status query)
returns a non-zero code: extend the parametrized inputs in
test_ensure_submodules_state_reconcile_failure (or add a new test) to include a
scenario like (init_submodules value, stdout value, expected_message "Failed to
get submodule status: err"), set mock_run.side_effect so the first
CompletedProcess has returncode=1 and stderr="err" (no further calls), assert
result is False, message equals the expected_message, and assert
mock_run.call_count == 1; reference ensure_submodules_state and the use of
subprocess.run/mock_run to locate where to change.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2a840992-54dc-46b7-ade2-7d2d4d5c2bf7

📥 Commits

Reviewing files that changed from the base of the PR and between 088293b and 6474ae7.

📒 Files selected for processing (2)

• src/cloudai/_core/installables.py
• tests/test_installables.py

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/test_installables.py`:
- Around line 96-119: Add two more parametrized cases to
test_ensure_submodules_state_reconciles for init_submodules=True that use stdout
lines starting with '+' and 'U' (e.g. "+0123456789abcdef path/to/submodule\n"
and "U0123456789abcdef path/to/submodule\n") and assert the same
expected_command ["git", "submodule", "update", "--init", "--recursive"]; this
keeps coverage symmetric with the existing '-' case and uses the same
git.init_submodules path and mock_run behavior to verify the reconciliation
command.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d82530f6-d3bb-4f1d-85c5-e1388688dc35

📥 Commits

Reviewing files that changed from the base of the PR and between 6474ae7 and a380fcc.

📒 Files selected for processing (2)

• src/cloudai/_core/installables.py
• tests/test_installables.py