Skip to content

Dockerfile improvements #74

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 2 commits into from
Nov 11, 2017
Merged

Conversation

Beanow
Copy link

@Beanow Beanow commented Nov 6, 2017

  • Validate the downloaded artifact's checksum
  • Expose default port
  • Use a volume for the default depot
  • Use the healtcheck feature

The operation GetCACaps for the health check is perhaps not ideal.

- Validate the downloaded artifact's checksum
- Expose default port
- Use a volume for the default depot
- Use the healtcheck feature
Dockerfile Outdated

EXPOSE 8080
VOLUME ["/depot"]
HEALTHCHECK --interval=30s --timeout=5s --retries=3 \
Copy link
Member

Choose a reason for hiding this comment

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

This is too opinionated. Please remove the healthcheck line from the dockerfile.

Copy link
Author

@Beanow Beanow Nov 6, 2017

Choose a reason for hiding this comment

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

I can agree, though the idea was to override it for different needs doing the right thing by default. Removed in following commit.

@groob
Copy link
Member

groob commented Nov 11, 2017

Thanks for checking in. I agree that we need a healthcheck, and will add a /healthz endpoint in the future.

@groob groob merged commit c5c2718 into micromdm:master Nov 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants