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

concore FRI Docker port mapping with Kong is not tested #44

Open
parteekcoder opened this issue Feb 19, 2023 · 13 comments
Open

concore FRI Docker port mapping with Kong is not tested #44

parteekcoder opened this issue Feb 19, 2023 · 13 comments
Labels
bug Something isn't working

Comments

@parteekcoder
Copy link
Contributor

Ports are not correctly mapped so docker container is running but not able to correctly send request to app

@parteekcoder parteekcoder changed the title Wrong ports Wrong ports (bug) Feb 19, 2023
@pradeeban
Copy link
Member

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:
CMD ["gunicorn", "--timeout=180", "--workers=4", "--bind=0.0.0.0:8080", "--access-logfile=-", "main:app"]
maps port 8080 of Gunicorn to the App's port 5000. This is the expected behavior.

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?

@parteekcoder
Copy link
Contributor Author

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: CMD ["gunicorn", "--timeout=180", "--workers=4", "--bind=0.0.0.0:8080", "--access-logfile=-", "main:app"] maps port 8080 of Gunicorn to the App's port 5000. This is the expected behavior.

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
nohup sudo docker run --name fri -p 8090:8081 fri > controlcore.out &

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

@pradeeban
Copy link
Member

The problem in your PR is,
You are attempting to change the Flask port to 8080 (currently, 5000), with the line:
app.run(host="0.0.0.0",port=8080)

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).

@parteekcoder
Copy link
Contributor Author

parteekcoder commented Feb 20, 2023

The problem in your PR is, You are attempting to change the Flask port to 8080 (currently, 5000), with the line: app.run(host="0.0.0.0",port=8080)

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
nohup sudo docker run --name fri -p 8090:8081 fri > controlcore.out &
to
nohup sudo docker run --name fri -p 8090:8080 fri > controlcore.out &

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

@pradeeban
Copy link
Member

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.

@pradeeban pradeeban reopened this Feb 20, 2023
@pradeeban pradeeban changed the title Wrong ports (bug) concore FRI Docker port mapping with Kong is not tested Feb 20, 2023
@parteekcoder
Copy link
Contributor Author

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

@pradeeban
Copy link
Member

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.

@pradeeban pradeeban added the bug Something isn't working label Feb 20, 2023
@parteekcoder
Copy link
Contributor Author

hey @pradeeban Will I implement if else approch for the routes as I told in the discussions

@pradeeban
Copy link
Member

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.

@parteekcoder
Copy link
Contributor Author

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

@shivangvijay
Copy link
Contributor

"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.

@pradeeban
Copy link
Member

@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.

@shivangvijay
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants