Skip to content

add a MINIKUBE deployment file that pulls the webserv image , deploys it and expose it in a k8s cluster.#19

Merged
olaaroub merged 2 commits intomainfrom
olaaroub
Nov 3, 2025
Merged

add a MINIKUBE deployment file that pulls the webserv image , deploys it and expose it in a k8s cluster.#19
olaaroub merged 2 commits intomainfrom
olaaroub

Conversation

@olaaroub
Copy link
Copy Markdown
Owner

@olaaroub olaaroub commented Nov 3, 2025

No description provided.

@olaaroub olaaroub requested a review from Copilot November 3, 2025 18:47
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a consolidated Kubernetes configuration file and improves documentation in the Dockerfile. The new webserv-KUBE.yaml combines the deployment and service configurations into a single file, while the Dockerfile gains a clarifying comment.

  • Added a unified Kubernetes manifest file (webserv-KUBE.yaml) combining deployment and service resources
  • Updated the container image reference to use a specific Docker Hub image (olaaroub/webserv-test:1.0)
  • Added an explanatory comment in the Dockerfile about package cleanup

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
webserv-KUBE.yaml New consolidated Kubernetes manifest combining deployment and service definitions with updated image reference
Dockerfile Added comment explaining the purpose of the apt cleanup command

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -0,0 +1,36 @@
# webserv-app.yaml
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

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

The filename in the comment is 'webserv-app.yaml', but the actual file is named 'webserv-KUBE.yaml'. Update the comment to match the actual filename.

Suggested change
# webserv-app.yaml
# webserv-KUBE.yaml

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

bro, the least thing i am interested in is naming.

php-cgi \
python3 \
&& rm -rf /var/lib/apt/lists/*
#the last cmd deletes the packages that i dont need so the container stays small
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

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

Grammatical errors in comment: 'i' should be capitalized to 'I', and 'dont' should be 'don't' with an apostrophe.

Suggested change
#the last cmd deletes the packages that i dont need so the container stays small
# The last cmd deletes the packages that I don't need so the container stays small

Copilot uses AI. Check for mistakes.
php-cgi \
python3 \
&& rm -rf /var/lib/apt/lists/*
#the last cmd deletes the packages that i dont need so the container stays small
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

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

The comment is misleading. The command rm -rf /var/lib/apt/lists/* deletes the apt package cache/lists, not installed packages. Consider revising to: 'the last cmd deletes the apt package lists to reduce the container size'.

Suggested change
#the last cmd deletes the packages that i dont need so the container stays small
# the last cmd deletes the apt package lists to reduce the container size

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Man I asked you to review the actual deployment script not to judge my writing... SHAMELESS

@olaaroub olaaroub merged commit ac68ba1 into main Nov 3, 2025
7 checks passed
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.

2 participants