-
Notifications
You must be signed in to change notification settings - Fork 20
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
concore FRI Docker port mapping with Kong is not tested #44
Comments
This is not a bug. It uses the Flask's default port, 5000. Since it does not state the port explicitly, it is slightly confusing for one who does not know Flask. So I have added the port 5000 explicitly in #47. The line: Your pull request attempts to run both Gunicorn and Flask at 8080 and will break FRI (port conflict), as far as I know. Did you test your pull request? |
yeah @pradeeban , I have ran this before creating the PR When you will run this command given in DOCKER_README.md Docker container will listen on 8090 and will send the request to port 8081 , but our app is running on port 5000 so it will not be processed that's why I suggested to change either in DOCKER_README.md or in our app |
The problem in your PR is, But Gunicorn is running in 8080 and it automatically does the mapping 8080 (Gunicorn) -> 5000 (Flask) through the line, CMD ["gunicorn", "--timeout=180", "--workers=4", "--bind=0.0.0.0:8080", "--access-logfile=-", "main:app"] in Dockerfile. There could be other issues in the README as you then pointed out above in your above comment. But your PR breaks the Gunicorn -> Flask mapping by running both in 8080. In my opinion, do not bother much with the FRI Docker. We always run it locally (not with Docker). |
OHH! I understood thanks for the clarification But here this command should be changes i think As unicorn is running on port 8080, I am right @pradeeban As Now what is happening is docker mapping port 8090 to 8081 but gunicorn port is 8080 as you told |
Yes, that is correct, @parteekcoder. There seems to be some more missing pieces there in the README. The port mapping actually goes: Host port (8090) -> Kong Gateway (8081?) -> Gunicorn (8080) -> Flask (5000). But Kong was not tested in GSoC for FRI. I fear there are more to fix in the DOCKER README and Dockerfile. I will look at this at leisure. Keeping the bug open. |
I have tested the container with changed port so will I create a PR for this @pradeeban |
You can create a PR (just one line change in the DOCKER README). I will merge it. But this bug you reported shows a larger issue (in the Dockerfile and/or Docker README): We implemented with the concept of having a container that goes like: Local port (8090) -> Kong (8081 or whatever that port) -> Gunicorn (8080) -> Flask (5000). But we never really used the Kong here (we did that for CONTROL-CORE Mediator). The Kong implementation here must be incomplete. |
hey @pradeeban Will I implement if else approch for the routes as I told in the discussions |
Merged your PR. As you said, it should work as-is for now until we figure out what is with the Kong deployment. The Kong instructions in the Docker README here can be confusing. https://github.com/ControlCore-Project/Mediator/ is a working implementation (it runs in production) of a similar deployment if that is helpful. Let me know if you have any questions. |
Thanks @pradeeban for your efforts, I will look into it and ask you if face any issue |
"The Kong instructions in the Docker README here can be confusing." To avoid this, we can develope the docker-compose file. Also, it helps users to build and run multiple container with one command. |
@shivangvijay Good suggestion. I have made that into a separate issue. On an unrelated note: I went through your GSoC proposal. Pls make sure to highlight your current contributions in your proposal. I know you have made significant contributions (pull requests and bug reports) to the concore repository. But they are not included in your proposal. |
@pradeeban I added commit ids of my pull requests in my proposal. Now I'll update it and provide link of all the bug reports and recent commits ids too. Thanks. |
Ports are not correctly mapped so docker container is running but not able to correctly send request to app
The text was updated successfully, but these errors were encountered: