Skip to content
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

Lilypad runs cached modules instead of pulling new Docker Image if there are changes #520

Open
DevlinRocha opened this issue Feb 20, 2025 · 2 comments
Labels
bug Something isn't working

Comments

@DevlinRocha
Copy link
Contributor

DevlinRocha commented Feb 20, 2025

Describe the bug

Current Behavior

When a user runs a job module using the Lilypad CLI, modules get downloaded to:

  • /tmp/lilypad/data/repos/<MODULE_AUTHOR>/<MODULE_NAME>

All job module runs are then ran from the cached module.

If a module is ran once, it will never update unless /tmp is cleared.

The most recent changes from the GitHub repo are correctly pulled, but the newest image from Docker Hub is not downloaded (unless the job.Spec.Docker.Image field changes).

Proposed Behavior

Check the hash of the Docker hash locally and compare it with the latest published hash, then pull the new Docker image if necessary

Instead download and run modules to/from:

- /tmp/lilypad/data/repos/<MODULE_NAME>/<MODULE_VERSION>

or:

- /tmp/lilypad/data/repos/<MODULE_AUTHOR>/<MODULE_NAME>/<MODULE_VERSION>

Either way, the key difference is the <MODULE_VERSION> which ensures the latest version of the module is being ran.

- We can add a version field to the lilypad_module.json.tmpl file
- We can alternatively use the Docker Hub hash or index digest (probably a better solution)
- Replace <MODULE_VERSION> with <DOCKER_HASH>

Reproduction

  • Download module repo (or create your own)
  • Build and push Docker image to Docker Hub
  • Run module with lilypad run ...
  • Make changes to module, and push changes to new Docker image
  • Run module again with lilypad run ...
  • None of the changes from the new Docker image will be present in the locally cached module

Logs

Screenshots

System Info

Apple M1 Pro
MacOs 15.2

Severity

Annoyance

@DevlinRocha DevlinRocha added the bug Something isn't working label Feb 20, 2025
@walkerlj0 walkerlj0 added bug Something isn't working and removed bug Something isn't working labels Feb 20, 2025
@bgins
Copy link
Contributor

bgins commented Feb 20, 2025

Awesome, thanks! We should definitely handle this situation better. 💯

Our planned module benchmarking process may help out because we will have a enforced relationship between a git ref and docker SHAs. But that's a ways off, so worth considering short term solutions.

We can add a version field to the lilypad_module.json.tmpl file

This option would add a maintenance burden on module authors who would need to keep the version in sync with git tags. Also, the module author may not be ready to assign a version if they are testing their module.

We can alternatively use the Docker Hub hash or index digest (probably a better solution)
Replace <MODULE_VERSION> with <DOCKER_HASH>

This version does seem like it would work, but it could end up consuming a lot of extra disk space.

We might be able to check the hash of the Docker version locally and compare it with the latest published hash, then update if needed?

@DevlinRocha DevlinRocha changed the title Lilypad /tmp behavior Lilypad runs cached modules instead of pulling new Docker Image if there are changes Feb 20, 2025
@DevlinRocha
Copy link
Contributor Author

DevlinRocha commented Feb 20, 2025

@bgins I agree your solution is more elegant and asking module builders to maintain a version number in their lilypad_module.json.tmpl file is an extra burden. I was trying to come up with a solution that wouldn't require much work on the engineering side, but if you think it will be feasible to check Docker hashed locally and compare, this wouldn't require anything extra from module builders and should make sure modules are running the correct (latest) version of their Docker image

I've updated my original issue to reflect your suggestion, thanks for the input!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants