Skip to content

Commit

Permalink
Avoid instantiating config_layout in upper injector hierarchy
Browse files Browse the repository at this point in the history
* Vm was grabbing a ConfigLayout as a dependency.

* Unfortunately, MacStore and KvStore were arranging for a ConfigLayout to be associated with base_injector.

* But carthage-runner was setting --keep on the layout injector not the base_injector

* So VMs were auto-shutting down

Fixes:

* VM explicitly gets an ConfigLayout from its injector rather than trusting dependency resolution

* MacStore and KvStore do the same to avoid polluting the base injector.

Thanks @timrcm for noticing that some VMs were disappearing.
  • Loading branch information
hartmans committed Nov 1, 2024
1 parent c4118c8 commit 72d2dbc
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 3 deletions.
3 changes: 2 additions & 1 deletion carthage/kvstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

__all__ += ['persistent_seed_path']

@inject_autokwargs(config_layout=ConfigLayout,
@inject_autokwargs(injector=Injector,
persistent_seed_path=InjectionKey(persistent_seed_path, _optional=True),
)
class KvStore(Injectable):
Expand All @@ -45,6 +45,7 @@ def __init__(
self, store_dir="persistent_assignments", max_size=4*2**30,
**kwargs):
super().__init__(**kwargs)
self.config_layout = self.injector(ConfigLayout)
store_path = Path(store_dir)
if not store_path.is_absolute():
store_path = Path(self.config_layout.state_dir)/store_path
Expand Down
3 changes: 2 additions & 1 deletion carthage/network/mac.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def random_mac_addr():
__all__ = ['random_mac_addr']


@inject_autokwargs(config_layout=ConfigLayout,
@inject_autokwargs(
injector=Injector,
kvstore=carthage.kvstore.KvStore)
class MacStore(Injectable):
Expand All @@ -39,6 +39,7 @@ class MacStore(Injectable):
def __init__(self, **kwargs):
from .base import NetworkConfig
super().__init__(**kwargs)
self.config_layout = self.injector(ConfigLayout)
state_dir = Path(self.config_layout.state_dir)
self.path = state_dir / "macs.yml"
self.domain = self.kvstore.domain('mac', True)
Expand Down
2 changes: 1 addition & 1 deletion carthage/vm.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@


@inject_autokwargs(
config_layout=ConfigLayout,
injector=Injector,
image=InjectionKey(vm_image_key, _ready=False),
network_config=carthage.network.NetworkConfig
Expand All @@ -52,6 +51,7 @@ class VM(Machine, SetupTaskMixin):
def __init__(self, name, *, console_needed=None,
**kwargs):
super().__init__(name=name, **kwargs)
self.config_layout = self.injector(ConfigLayout)
injector = self.injector
config_layout = self.config_layout
self.console_needed = console_needed
Expand Down

0 comments on commit 72d2dbc

Please sign in to comment.