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
Run the CI test locally #317
Conversation
056c873
to
f2d2c30
Compare
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.
Some questions and suggestions.
| - sleep 30 | ||
| - oc create -f ./scripts/broker-ci/bind-mediawiki-postgresql.yaml || export ERROR=true | ||
| - sleep 5 | ||
| - oc delete pods $(oc get pods -n default | grep mediawiki | awk $'{ print $1 }') -n default || export ERROR=true | ||
| - oc delete pods $(oc get pods -o name -l app=mediawiki123 -n default | head -1 | cut -f 2 -d '/') -n default | ||
| - sleep 20 |
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 it possible to move most of this to its own script instead of cluttering .travis.yml?
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.
Yes. After make ci merges, travis is going to use make ci for testing. I'm going to follow up with that and some changes to make sure the scripts log really well and return good error messages.
| - sleep 30 | ||
| - oc create -f ./scripts/broker-ci/bind-mediawiki-postgresql.yaml || export ERROR=true | ||
| - sleep 5 | ||
| - oc delete pods $(oc get pods -n default | grep mediawiki | awk $'{ print $1 }') -n default || export ERROR=true | ||
| - oc delete pods $(oc get pods -o name -l app=mediawiki123 -n default | head -1 | cut -f 2 -d '/') -n default |
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 use of head and cut. I always forget about them.
| ci: | ||
| ./scripts/broker-ci/local-ci.sh | ||
|
|
||
| .PHONY: vendor build run prepare-build-image build-image release-image release push clean deploy test ci cleanup-ci |
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.
like the new targets. Do we want to add cleanup-ci to really-clean? Just curious maybe not needed.
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.
Sure.
| - Curl the Mediawiki endpoint to check for success | ||
| * Requires: | ||
| - Cluster | ||
| - Service Catalog |
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 addition.
| oc delete instance postgresql mediawiki -n default | ||
| oc delete dc postgresql mediawiki123 -n default | ||
| ./scripts/broker-ci/wait-for-resource.sh delete pod postgresql | ||
| ./scripts/broker-ci/wait-for-resource.sh delete pod mediawiki |
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.
+1 looks good to me
| echo "Running: curl ${ROUTE}| grep \"div class\" | cut -f 2 -d \"'\"" | ||
| if [ "${BIND_CHECK}" = "error" ]; then | ||
| echo "MAKE CI FAILED" | ||
| elif [ "${BIND_CHECK}" = "" ]; then |
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 it really = and not ==?
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.
Either works.
string1 == string2
string1 = string2
True if the strings are equal.
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.
Thanks didn't know that.
scripts/broker-ci/local-ci.sh
Outdated
| fi | ||
| } | ||
|
|
||
| provision |
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'd prefer a comment above provision:
#
# MAIN
#
something like that just to make it easier to remember that this is where we start.
| echo "Waiting for ${RESOURCE_NAME} ${RESOURCE} to be created" | ||
| sleep 1 | ||
| done | ||
| elif [ "${ACTION}" = "delete" ]; then |
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.
+1 to rename, same question about = vs ==.
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.
Was able to test ACK
Give the developer the ability to run the CI test locally. Run the test by: ```make ci```. It will provision mediawiki and postgresql apbs then bind them.
Give the developer the ability to run the CI test locally. Run the
test by:
make ci. It will provision mediawiki and postgresqlapbs then bind them.
Changes proposed in this pull request
make runto test provision & bind