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

Replace Nginx proxy with Traefik #3409

Merged
merged 19 commits into from
Jul 26, 2021
Merged

Replace Nginx proxy with Traefik #3409

merged 19 commits into from
Jul 26, 2021

Conversation

tadejsv
Copy link
Contributor

@tadejsv tadejsv commented Jul 12, 2021

Motivation and context

This replaces the Nginx with a more "modern" proxy, Traefik.
There should be no change in the basic usage, i.e. the new docker-compose.yml file is a drop-in replacement for the old one. For using HTTPS the process becomes vastly simplified, and boils down to setting 2 env variables and using an override docker compose file.

Apart from HTTPS, the only other noticeable user-facing change is that the hostname is no longer set by the CVAT_HOST env variable in the proxy service, but by the CVAT_HOST env variable in the shell from which the user will execute the docker-compose command.

Closes #3407.

How has this been tested?

I have tested the new docker-compose.yml file locally, and I have also tested the docker-compose.https.yml file on an EC2 instance with my own domain - HTTPS was properly set up, as expected. But it would be great if someone else would test this on their own, just to double-check.

Also, I am not sure how this would work with proxy (in this sense). Should anyone be able to help me with checking this, that would be great.

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2021 Intel Corporation
#
# SPDX-License-Identifier: MIT

@tadejsv tadejsv marked this pull request as ready for review July 14, 2021 07:35
@tadejsv
Copy link
Contributor Author

tadejsv commented Jul 14, 2021

@nmanovic This PR is now ready for review

@tadejsv
Copy link
Contributor Author

tadejsv commented Jul 18, 2021

@nmanovic Would you mind running the workflows here again?

Copy link
Contributor

@azhavoro azhavoro left a comment

Choose a reason for hiding this comment

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

@tadejsv Hi,

Thanks for the contribution, great job!
I have 2 comments:

@tadejsv
Copy link
Contributor Author

tadejsv commented Jul 24, 2021

Thanks for the feedback, @azhavoro . I have implemented the changes you suggested. If you re-run the workflows, I think all tests should pass

@nmanovic
Copy link
Contributor

@tadejsv , great job! Thanks for the contribution.

nmanovic
nmanovic previously approved these changes Jul 26, 2021
@bsekachev bsekachev mentioned this pull request Jul 26, 2021
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants