docker / docs

Source repo for Docker's Documentation
https://docs.docker.com
Apache License 2.0
4.17k stars 7.27k forks source link

Python guide 4: Invalid port value #21320

Closed yuto-kimura-g closed 15 hours ago

yuto-kimura-g commented 1 day ago

Is this a docs issue?

Type of issue

Information is incorrect

Description

Hello, and thank you for providing such valuable documentation. As this is my first OSS contribution, I apologize if I make any mistakes.

In the 4. Test your deployment section, which is part of the "Python language-specific guide" chapter, I encountered an issue while following the instructions. When I copied and pasted the docker-python-kubernetes.yaml, then execute $ kubectl apply -f docker-python-kubernetes.yaml, $ curl http://localhost:30001/ locally, there was no output.

Upon checking the CONTAINER ID with $ docker ps and using $ docker top <CONTAINER ID> I found the following:

$ docker top <CONTAINER ID>
UID                 PID                 PPID                C                   STIME               TTY                 TIME                CMD
10001               7211                7191                0                   08:32               ?                   00:00:00            /bin/sh -c python3 -m uvicorn app:app --host=0.0.0.0 --port=5001
10001               7225                7211                0                   08:32               ?                   00:00:04            python3 -m uvicorn app:app --host=0.0.0.0 --port=5001

After changing the port in docker-python-kubernetes.yaml from 8001 to 5001, everything worked as expected.

$ curl http://localhost:30001/
Hello, Docker!

Location

https://docs.docker.com/guides/python/deploy/

Suggestion

Please consider updating the port number in the docker-python-kubernetes.yaml from 8001 to 5001.

dvdksn commented 19 hours ago

I see that in your case you're running the server on port 5001, in which case the containerPort should be 5001.

/bin/sh -c python3 -m uvicorn app:app --host=0.0.0.0 --port=5001

But the examples in the guide all use port 8001 - in which case the current port mappings are correct. Why do you want to change it from 8001 to 5001?

yuto-kimura-g commented 19 hours ago

Thanks for your review.

Yes, I followed the guide and set port 8001. And it worked correctly until the 3. configure CI/CD section. However, the Container Image (technox64/python-docker-dev-example-test:latest) used in the docker-python-kubernetes.yaml file in the guide seems to specify port 5001.

Image

ref: https://hub.docker.com/layers/technox64/python-docker-dev-example-test/latest/images/sha256-b55f623388e769b8d693942f46d243dbe5c417d14bd3d78b6f9370df7a348a8f?context=explore

I think it is easy to change docker-python-kubernetes.yaml than to change the Container Image used, so I proposed to follow the Container Image setting (i.e., change to port 5001). Or is it better to change the link of the Container Image to be used ?

dvdksn commented 18 hours ago

Oh OK, I see: The Kubernetes config uses a hard-coded image name! This must have been an oversight from when it was originally authored. We should instruct users to edit the yaml and replace the placeholder with their own details (the same details as used in the GitHub Actions workflow in the previous step):

    spec:
      containers:
       - name: fastapi-service
         image: DOCKER_USERNAME/REPO_NAME

@yuto-kimura-g would you be up for making this change instead?

yuto-kimura-g commented 18 hours ago

I see. I thought the image of technox64 was official and intentionally specified, but that’s not the case, is it?

Of course. I have created a revised PR #21327. Please review it.