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
task: adding local deploy targets #6
Conversation
make/Makefile-local-install
Outdated
@echo "🟢 Start mdai-system-reqs..." | ||
@go version || brew list go || brew install go \ | ||
kubectl version || brew list kubectl || brew install kubectl \ | ||
npm -v || brew list npm || brew install npm \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is npm
needed? I don't see it used, but maybe some other command is using it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good call out, npm
is needed for AWS (cdk
) but not local install
make/Makefile-local-install
Outdated
.PHONY: mdai-system-reqs | ||
mdai-system-reqs: | ||
@echo "🟢 Start mdai-system-reqs..." | ||
@go version || brew list go || brew install go \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is go needed for like brew install kubectl
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename make/Makefile-local-recipies
to make/Makefile-local-recipes
schedule: "*/1 * * * *" | ||
concurrencyPolicy: Forbid | ||
successfulJobsHistoryLimit: 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious why this was removed
Co-authored-by: Justin Hunter <arcanez@users.noreply.github.com>
mdai-install-helm-charts: | ||
@echo "🟢 Start mdai-install-helm-charts..." | ||
helm repo update | ||
helm upgrade -f ./templates/prometheus-values.yaml prometheus prometheus-community/prometheus --install --wait |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed a file path here, changing prometheus_values.yaml
to prometheus-values.yaml
when I did my tabs fixes. However I think maybe the values in that file actually might break the metrics scraping 😬 Mine no longer seem to work, but that could be specific to my setup
Any background context you want to provide for reviewers?
These are the relevant steps for a local engine install.
What is the focus or goal of the code changes?
Provide a local approach to test the tech before deploying to a CSP like AWS.
Where should the reviewer start?
README.md
ln. 29Does this add new dependencies? Is there an installation or build procedure?
Yes, there are 4 new build procedures.
1.
make -f ./make/Makefile-local-recipies create-mdai
deploy a local cluster from scratch2.
make -f ./make/Makefile-local-recipies delete-mdai
deletes mdai cluster deployed locally and all artifacts associated3.
make -f ./make/Makefile-local-recipies delete-mdai-all
deletes mdai cluster deployed locally and all artifacts associated, plus helm charts4.
make -f ./make/Makefile-local-recipies update-mdai-collector
updates mdai the collector to the latest configurationHow should this be manually tested?
Run each of the commands in their correct use as called out above.
Screenshots (if applicable)
n/a
Optional: What gif best describes this PR or how it makes you feel?