Skip to content

LDR: selectable local mounts #12092

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.

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
merged 5 commits into from
Jan 3, 2025
Merged

LDR: selectable local mounts #12092

merged 5 commits into from
Jan 3, 2025

Conversation

simonrw
Copy link
Contributor

@simonrw simonrw commented Jan 2, 2025

Motivation

When using python -m localstack.dev.run, local code checkouts if present are mounted into the container. This may cause confusion if the local project checkout is old and incompatible with the version of LocalStack in the container.

In particular, we rely heavily on our fork of moto which changes semi-frequently. This can cause semi-regular breakages which are difficult to debug.

I think mounting non-core packages should be opt-in to reduce surprises, and should be straightforward and self-documenting to help discoverability.

Changes

  • Add new argument -l/--local-packages which can be used to specify which packages are mounted (if found)
  • Define which packages are available in a declarative way so additional packages can be easier to add in the future
  • Ensure the help string includes the list of packages which can be mounted, as well as validating the user's input to ensure only valid packages can be specified
  • Update the example docstring

Testing

  • Check out rolo and moto in the same parent directory as localstack is checked out in, making the following structure:
somedir
├── localstack
│   ├── ...
│   ├── localstack-core
│   │   ├── localstack               <- will be mounted into the container
│   │   └── localstack_core.egg-info
│   ├── pyproject.toml
│   ├── tests
│   └── ...
├── moto
│   ├── AUTHORS.md
│   ├── ...
│   ├── moto                         <- will be mounted into the container
│   ├── moto_ext.egg-info
│   ├── pyproject.toml
│   ├── tests
│   └── ...
├── rolo
│   ├── AUTHORS.md
│   ├── ...
│   ├── moto                         <- will be mounted into the container
│   ├── moto_ext.egg-info
│   ├── pyproject.toml
│   ├── tests
│   └── ...
  • On master, run python -m localstack.dev.run and note the shown configuration posted at the start of the command contains rolo and moto, e.g. in the following example see the volumes section contains both localstack/moto/moto and localstack/rolo/rolo
{
    'image_name': 'localstack/localstack',
    'name': 'localstack-main',
    'volumes': [
        '/Users/simon/work/localstack/localstack/localstack-core/.filesystem/var/lib/localstack/cache/machine.json:/var/lib/localstack/cache/machine.json:ro',
        '/Users/simon/Library/Caches/localstack/volume:/var/lib/localstack',
        '/var/run/docker.sock:/var/run/docker.sock',
        '/Users/simon/work/localstack/localstack/localstack-core/localstack:/opt/code/localstack/localstack-core/localstack',
        '/Users/simon/work/localstack/moto/moto:/opt/code/localstack/.venv/lib/python3.11/site-packages/moto:ro',
        '/Users/simon/work/localstack/rolo/rolo:/opt/code/localstack/.venv/lib/python3.11/site-packages/rolo:ro',
        '/Users/simon/work/localstack/localstack/bin/docker-entrypoint.sh:/usr/local/bin/docker-entrypoint.sh:ro'
    ],
    'ports': ['0.0.0.0:4566:4566', '0.0.0.0:443:443', '0.0.0.0:4510-4560:4510-4560'],
    'exposed_ports': [],
    'env_vars': {
        'GATEWAY_LISTEN': '0.0.0.0:4566,0.0.0.0:443',
        'EXTERNAL_SERVICE_PORTS_START': 4510,
        'EXTERNAL_SERVICE_PORTS_END': 4560,
        'DNS_ADDRESS': '127.0.0.1',
        'DEBUG': '1',
        'LOCALSTACK_ROOT_DIR': '/Users/simon/work/localstack',
        'DISABLE_EVENTS': '1',
        'GATEWAY_SERVER': 'twisted',
        'DOCKER_HOST': 'unix:///var/run/docker.sock',
        'IMAGE_NAME': 'localstack/localstack'
    },
    'privileged': False,
    'remove': True,
    'interactive': True,
    'tty': True,
    'detach': False
}
  • Check out this PR and run the same command and note that the two directories are not present
  • Run python -m localstack.dev.run -l rolo and note that rolo is mounted, but moto is not
  • Try further combinations

@simonrw simonrw added the semver: patch Non-breaking changes which can be included in patch releases label Jan 2, 2025
@simonrw simonrw self-assigned this Jan 2, 2025
@simonrw simonrw marked this pull request as ready for review January 2, 2025 16:01
@simonrw simonrw requested review from thrau and bentsku January 2, 2025 16:02
Copy link

github-actions bot commented Jan 2, 2025

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 53m 28s ⏱️ +21s
3 920 tests +1  3 613 ✅ +1  307 💤 ±0  0 ❌ ±0 
3 922 runs  +1  3 613 ✅ +1  309 💤 ±0  0 ❌ ±0 

Results for commit 152320a. ± Comparison against base commit 3e1471a.

♻️ This comment has been updated with latest results.

Copy link
Member

@thrau thrau left a comment

Choose a reason for hiding this comment

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

That's a nice change! I agree with your assessment.

My only recommendation would be to encapsulate the mapping in some way into the .paths package, to be consistent (all path definitions are there). You could simply move the global, or perhaps create a class encapsulation as we have for the other paths.

@simonrw
Copy link
Contributor Author

simonrw commented Jan 3, 2025

My only recommendation would be to encapsulate the mapping in some way into the .paths package, to be consistent (all path definitions are there). You could simply move the global, or perhaps create a class encapsulation as we have for the other paths.

I think that's a good idea, thank you. For now I have moved the constant into the paths module, but in the future if we get the need to expand on the paths in the future we can revisit making a nicer API to it.

@simonrw simonrw merged commit ed8046a into master Jan 3, 2025
31 checks passed
@simonrw simonrw deleted the feat/ldr/selectable-mounts branch January 3, 2025 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants