Skip to content

Makes docker healthcheck more robust #12344

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 6, 2025
Merged

Makes docker healthcheck more robust #12344

merged 2 commits into from
Mar 6, 2025

Conversation

crystalin
Copy link
Contributor

Motivation

Running the localstack/localstack docker with a different --workdir makes it "unhealthy". Ex: docker run -it --workdir /opt localstack/localstack:stable

Changes

Changes the docker healthcheck to use absolute path when executing the binary

@crystalin crystalin requested a review from alexrashed as a code owner March 5, 2025 13:49
localstack-bot
localstack-bot reviewed Mar 5, 2025
View reviewed changes
Copy link
Collaborator

@localstack-bot localstack-bot left a comment

Choose a reason for hiding this comment

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

Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.

@localstack-bot
Copy link
Collaborator

localstack-bot commented Mar 5, 2025 •
edited
Loading

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@crystalin
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

localstack-bot added a commit that referenced this pull request Mar 5, 2025
@localstack-bot
@crystalin has signed the CLA in #12344
90f2aee
@alexrashed alexrashed added the semver: patch Non-breaking changes which can be included in patch releases label Mar 5, 2025
alexrashed
alexrashed reviewed Mar 5, 2025
View reviewed changes
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, @crystalin! Would you mind elaborating on scenarios where it makes sense to set the working directory explicitly to a different one?

@crystalin
Copy link
Contributor Author

This originally comes from using https://github.com/nektos/act with localstack service on a github action. Ex:

jobs:
  orchestrator_coverage:
    runs-on: ubuntu-latest

    services:
      localstack:
        image: localstack/localstack
        env:
          SERVICES: s3, sqs, sns
          DEFAULT_REGION: us-east-1
          AWS_ACCESS_KEY_ID: "AWS_ACCESS_KEY_ID"
          AWS_SECRET_ACCESS_KEY: "AWS_SECRET_ACCESS_KEY"
        ports:
          - 4566:4566
      mongodb:
        image: mongo:latest
        ports:
          - 27017:27017

I'm no expert in act, but I think it changes the working directory to support some cases of docker in docker, but not sure at all

alexrashed
alexrashed approved these changes Mar 6, 2025
View reviewed changes
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification, and especially for the contribution in the first place! The changes are minimal and are looking good, all pipelines are 💚!

@alexrashed alexrashed merged commit 4e1cf52 into localstack:master Mar 6, 2025
21 of 22 checks passed
@alexrashed alexrashed mentioned this pull request Mar 6, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@localstack-bot localstack-bot localstack-bot left review comments

@alexrashed alexrashed alexrashed approved these changes

Assignees
No one assigned
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@crystalin @localstack-bot @alexrashed