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

refactor: simplify explanation for exposed ports for httpGet in probes #312

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

BoboTheBarbar
Copy link

I think there is no reason to illustrate an exposed containerport 80, since it would obfuscate that there is no reason for this port to be exposed.

BoboTheBarbar and others added 2 commits March 10, 2023 18:22
I think there is no reason to illustrate an exposed containerport 80, since it would obfuscate that there is no reason for this port to be exposed.
@dgkanatsios
Copy link
Owner

sorry, what's wrong with exposing the default nginx port?

@BoboTheBarbar
Copy link
Author

sorry, what's wrong with exposing the default nginx port?

There is nothing wrong with exposing it. But the task requires to run a readyness probe and verify the port 80 is in use.
It is an educational detail to show a student, that port 80 doesn't have to be exposed, since the readiness probe runs within the container.
If we would expose port 80 on the pod, this detail would be obfuscated. I will clarify that in the comment a bit more in another commit.

@BoboTheBarbar
Copy link
Author

New configuration has been tested manually
grafik

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.

3 participants