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

k8s deploy/delete scripts #2

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

gustavomassa
Copy link

Created a script to deploy sock-shop to kubernetes minikube/cluster
Created a script to delete all sock-shop/monitoring/default resources of kubernetes cluster
Renamed nginx-ingress-controller to not deploy it automatically
Removed deprecated warnings

TODO: Figure out how to port-forward/expose Grafana/Prometheus services on the real cluster

@nimrod-up9
Copy link
Contributor

nimrod-up9 commented Feb 3, 2022

You can remove manifests/extras/nginx. It is not working and nobody is using it. Assigned you the relevant Jira ticket.

@nimrod-up9
Copy link
Contributor

Why did you rename deploy/kubernetes/manifests/sock-shop-ns.yaml?

@nimrod-up9
Copy link
Contributor

"Removed deprecated warnings" - Is that achieved through the removal of the "beta." prefix?
I think this is something that you can try and contribute upstream to https://github.com/microservices-demo/microservices-demo for some internet points if you are interested.

delete () {
echo "Deleting sock-shop namespace resources"
kubectl delete all --all -n sock-shop
kubectl delete namespace sock-shop || true
Copy link
Contributor

Choose a reason for hiding this comment

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

What does the || true do?
If the delete operation fail, why would we want to suppress the error?

Comment on lines +14 to +21
echo "Deploying sock-shop namespace"
kubectl create -f manifests/sock-shop-ns.yaml

echo "Deploying sock-shop core"
kubectl apply $(ls manifests/*[[:digit:]]*.yaml | awk ' { print " -f " $1 } ')

echo "Deploying sock-shop-mizu extras"
kubectl apply $(ls manifests/extras/*[[:digit:]]*.yaml | awk ' { print " -f " $1 } ')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
echo "Deploying sock-shop namespace"
kubectl create -f manifests/sock-shop-ns.yaml
echo "Deploying sock-shop core"
kubectl apply $(ls manifests/*[[:digit:]]*.yaml | awk ' { print " -f " $1 } ')
echo "Deploying sock-shop-mizu extras"
kubectl apply $(ls manifests/extras/*[[:digit:]]*.yaml | awk ' { print " -f " $1 } ')
echo "Deploying sock-shop"
kubectl create -f complete-demo.yaml

@nimrod-up9
Copy link
Contributor

In principal this PR should have been several PRs. (Fix warnings, remove NGINX, add scripts).
You don't have to split this PR, but please keep it in mind for the next ones.

@nimrod-up9
Copy link
Contributor

Document script usage in one of the markdown files.

@mertyildiran
Copy link
Member

@gustavomassa @nimrod-up9 any update on this PR?

@RefaelBotbol
Copy link
Contributor

@gustavomassa @nimrod-up9 - any update?

@nimrod-up9
Copy link
Contributor

nimrod-up9 commented Mar 14, 2022

I don't have any more comments besides what I already wrote. Waiting for the fixes/answers.

@nimrod-up9
Copy link
Contributor

btw, who requested this? We have gone a long time without merging this branch. Is it still necessary or should we close without merging?

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.

None yet

4 participants