microsoft / garnet

Garnet is a remote cache-store from Microsoft Research that offers strong performance (throughput and latency), scalability, storage, recovery, cluster sharding, key migration, and replication features. Garnet can work with existing Redis clients.
https://microsoft.github.io/garnet/
MIT License
9.71k stars 459 forks source link

Fix Dockerfile entrypoints #467

Closed jthvai closed 1 week ago

jthvai commented 1 week ago

Fixes a regression introduced in #224;

entrypoints containing relative paths are incompatible with the usage of custom working directories.

This causes unexpected failures particularly in CI (GitHub, Gitea, Forgejo actions), where the repository is often mounted and set as the working directory - notably, when garnet is used as a service.


Workaround for end users

Manually set the entrypoint to ["/app/GarnetServer", "-i", "128m", "--port", "6379"] when invoking the container.

The docs for doing this with GH Actions services are here. The workaround must be altered, and is not complete; the --entrypoint option does not accept array syntax, so only --entrypoint /app/GarnetServer may be specified. Note then that Garnet's default port is 3278, and one must change their application behaviour accordingly.

This workaround does not work for the main container of a job (under container), as it does not support --entrypoint.


This cropped up during the prelim stages of adding garnet support to Forgejo, and cost a fair bit of time to troubleshoot, as it was such an unexpected error. I would expect this to cause problems for integration testing in general, which may limit the adoption of garnet.

See also these docs as a side note. I've also seen a couple of StackOverflow posts claiming relative entrypoints don't behave well on Ansible.

jthvai commented 1 week ago

@microsoft-github-policy-service agree

badrishc commented 1 week ago

I think we can just change Garnet's default port to 6379, then there is no need to pass arguments to the entry point.

jthvai commented 1 week ago

then there is no need to pass arguments to the entry point.

I did find it startling that the Dockerfile added arguments to the entrypoint at all.

Garnet's default port is well documented (in its default configuration file). I'll leave it up to the team to decide whether using redis's port would be best in all cases; I suspect there are users depending on it being 3278.

The mismatch between its default port and the behaviour of the Dockerfile is an unfortunate side effect, but nonetheless not the main issue addressed by this PR.


Changing Garnet's default port does nothing against the primary error caused by this regression - that the entrypoint cannot be found if the working directory is changed. It only makes the workaround complete; a user must still manually redeclare the entrypoint if they want to change the working directory.

badrishc commented 1 week ago

Makes sense, we can merge this and revisit the default port in a separate PR (see https://github.com/microsoft/garnet/pull/468) if it seems like the best way to avoid confusion. Thanks.

jthvai commented 1 week ago

Thank you for your time!