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

Make docker entrypoint 'last_restore_file' runtime-variable #772

Open
ericsmalling opened this issue Jun 3, 2024 · 4 comments · May be fixed by #787
Open

Make docker entrypoint 'last_restore_file' runtime-variable #772

ericsmalling opened this issue Jun 3, 2024 · 4 comments · May be fixed by #787
Labels
complexity: low good first issue Good for newcomers help-wanted Issues in the state 'help-wanted' ready Issues in the state 'ready'

Comments

@ericsmalling
Copy link

ericsmalling commented Jun 3, 2024

Project board link

The hard coded path of for the last_restore_file in docker-entrypoint.sh does not work for users that need to use their own path for such files. Recommend using the same MEDUSA_TMP_DIR as used in restore.py for consistency.

Current:

last_restore_file=/var/lib/cassandra/.last-restore

Proposed replacement:

last_restore_file=${MEDUSA_TMP_DIR:-'/var/lib/cassandra'}/.last-restore

┆Issue is synchronized with this Jira Story by Unito
┆Issue Number: MED-5

@rzvoncek rzvoncek moved this to Ready in K8ssandra Jun 11, 2024
@adejanovski adejanovski added the ready Issues in the state 'ready' label Jun 11, 2024
@rzvoncek rzvoncek added good first issue Good for newcomers help wanted Extra attention is needed complexity: low labels Jun 11, 2024
@rzvoncek
Copy link
Contributor

Hi @ericsmalling ! This looks like a great idea and the proposed replacement is neat. Any chance you could find a bit of time to convert this into a PR as well?

@rzvoncek rzvoncek added help-wanted Issues in the state 'help-wanted' and removed help wanted Extra attention is needed labels Jun 11, 2024
@adejanovski adejanovski moved this from Ready to Help Wanted in K8ssandra Jun 11, 2024
@ericsmalling
Copy link
Author

Thanks, I'll tackle this ASAP

ericsmalling added a commit to ericsmalling/cassandra-medusa that referenced this issue Jun 21, 2024
@ericsmalling
Copy link
Author

Draft PR #787 open - I could use some pointers on how to add some tests around this though

@rzvoncek
Copy link
Contributor

rzvoncek commented Oct 3, 2024

Hi. Thanks for the PR. For testing, the first step is to expose the variable via Dockerfile like I did here.

The next step would be to pass in a value when the Medusa container actually runs. In Medusa tests, we have the k8ssandra-e2e-tests where we do build Medusa and then have it run in k8ssandra-operator (just 2 tests). However, we can only specify an image tag through the medusa.spec.containerImage. I cannot think of a clean way of passing in the env var that's currently available.

We could also hardcode it at build time, but that feels like a worse solution.

Perhaps there's another way to make Medusa's entrypoint see the env var that I can't think of right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: low good first issue Good for newcomers help-wanted Issues in the state 'help-wanted' ready Issues in the state 'ready'
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants