-
Notifications
You must be signed in to change notification settings - Fork 207
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
tests: add run-openshift template tests #209
Conversation
@omron93 Are the tests run when they are present or do we have to trigger them manually in some way? |
test/run-openshift
Outdated
set -exo nounset | ||
|
||
function check_postgresql_os_service_connection() { | ||
local util_image_name="${1}" ; shift |
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.
Nit:please use just "foo=$baz" syntax. Looks much nicer, the quotes and brackets are not needed in this case.
test/run-openshift
Outdated
|
||
THISDIR=$(dirname ${BASH_SOURCE[0]}) | ||
|
||
source ${THISDIR}/test-lib-openshift.sh |
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 would rather quote $THISDIR.
test/run-openshift
Outdated
: " Service ${service_name} check ..." | ||
|
||
local cmd="echo 'SELECT 42 as testval' | PGPASSWORD=${pass} PGCONNECT_TIMEOUT=15 \ | ||
psql -t -h ${pod_ip} -U ${user} -d ${database}" |
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.
Could we use pg_isready
, since that will be the live check in future, too?
test/run-openshift
Outdated
} | ||
|
||
function test_postgresql_pure_image() { | ||
local image_name=${1:-centos/postgresql-96-centos7} |
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.
The default value seems to be unused, so image_name=$1
would look a bit safer.
ct_os_wait_pod_ready "${service_name}" 60 | ||
check_postgresql_os_service_connection "${image_name}" "${service_name}" testu testp testdb | ||
|
||
ct_os_delete_project |
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.
Nice, this seems to be promising! I'm looking forward to see the library code.
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.
Ah, it's already there :-) my bad.
Seems like the 'test-openshift' target used to be in https://github.com/sclorg/rhscl-container-ci/blob/master/yaml/jobs/collections/mongodb-rh.yaml so the openshift tests used to be triggered automatically for mongo. Better approach could be to invoke the |
@praiskup Made some changes according to your suggestions, thanks! |
Seems like the code works on both rhel and centos-based Openshift installations. Only the library does not yet provide any functions to install openshift on a rhel7 system. |
@omron93 Since |
OK. I've enabled it for all images. Test has to be explicitly triggered by [test-openshift]
I would let installing oc tool to the user (and to the CI), same we do with |
RHEL CI is failing because you try to install centos package (CI image already contain oc). |
Looking at the logs the centos image does not seem to have openshift installed by default. Are we going to update it so it does or should I just make the install function call specific to centos? |
The same issue is in rhel, it happens only the first time the |
iptables has to be flushed. Please take a look at sclorg/container-common-scripts#40 With this it should work. No |
[test-openshift] |
Seems like the openshift test fails now on |
d22fd2d
to
49f9b37
Compare
[test-openshift] |
It seems that it takes some time. In last centos openshift tests it succeeded in fifth attempt. I'll try to find out what is causing this. |
[test-openshift] |
1 similar comment
[test-openshift] |
There was Jenkins upgrade by Central CI admins. But problems with plugins occurred so we have to wait. There will be probably next try in near future. |
All checks passing now |
Looks fine. |
Thanks for review! Merging |
} | ||
|
||
function test_postgresql_template() { | ||
local image_name=${1:-centos/postgresql-96-centos7} |
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 there 'backup' image_name?
IMHO this should be tested specifically with the CI-built image.
Resolves: #204