-
Notifications
You must be signed in to change notification settings - Fork 43
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
modify start-postgres-localstack by adding wait time and start-harmon… #540
base: main
Are you sure you want to change the base?
Conversation
…y by adding check if harmony pod is ready
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'm glad you were able to find a way to get the scripts to work consistently for you. I think we can keep the 30 second sleep you added after starting up minikube, and then remove the other sleeps and just update line 51 in bin/port-forward
to wait for up to 120 seconds for a pod to reach the ready state. That way it should work for everyone without adding any extra time to the startup.
@@ -13,10 +13,16 @@ eval "$env_save" | |||
|
|||
envsubst < ./config/harmony-k8s.yaml | kubectl apply -f - -n harmony > /dev/null | |||
# harmony takes a while to start up, so we will do other things before we try to set up | |||
|
|||
while [ "$(kubectl -n harmony get pods -l app='harmony' -o jsonpath='{.items[*].status.containerStatuses[0].ready}')" != "true" ]; do |
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.
Line 51 in the bin/port-forward
script is waiting for harmony to be ready, but we have the timeout set to 60 seconds. I think we still want it to time out (for example to fail our Github workflows rather than have them hang forever). Maybe we can add an env var which can be set to something larger if needed? We could also change that default timeout from 60s to 120s? How long does it take for you right now?
@@ -82,6 +82,8 @@ if [ -z "$harmony_namespace" ]; then | |||
kubectl create namespace harmony | |||
fi | |||
|
|||
sleep 10 |
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'm not seeing anything where this sleep here would solve a timing issue - I'd like to remove this one.
@@ -118,6 +120,9 @@ kubectl apply -n harmony -f ./config/local-postgres-localstack-deployment.yml | |||
[[ $USE_LOCALSTACK = 'true' ]] && s3_endpoint="localstack:4572" || s3_endpoint="s3.amazonaws.com" | |||
POSTGRES_PORT="${POSTGRES_PORT:-5432}" | |||
|
|||
|
|||
sleep 10 |
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'm guessing this was added here because sometimes localstack was not ready in time? For this one again bin/port-forward
should be handling this by waiting for localstack to be ready, but now you are giving it up to 70 seconds instead of 60 seconds to become ready. So if we change line 51 in bin/port-forward
to wait for up to 120 seconds (per the other comment), I think you can get rid of this one as well.
…y by adding check if harmony pod is ready
Jira Issue ID
Description
When I install harmony on my ubuntu20.04, sometimes it is not successful. The reason is that port forward fails because the harmony pod is not ready yet. I add statement in start-harmony to check if the harmony pod is ready before doing port forward.
Local Test Steps
After I delete minikube, do fresh run ./bin/bootstrap-harmony. it works very time now.
PR Acceptance Checklist