Allow to run with other than local cluster #91

hhorak · 2018-06-05T21:04:16Z

Do not run local cluster, if already logged into any (remote/local) cluster -- oc whoami should work here, to determine whether we're logged in to the cluster already, but maybe there is a better solution.
Also do not login to a docker registry, if the address (now newly configurable) is defined.

omron93 · 2018-06-06T09:38:26Z

test-lib-openshift.sh

+# Returns 0 if already logged in docker
+# Uses global REGISRTY_ADDRESS environment variable for arbitrary registry address.
+function ct_os_docker_logged() {
+  grep -q -e ${REGISRTY_ADDRESS:-172.30.1.1:5000} ~/.docker/config.json


I think that this will work on temporary machine which is deleted after test run. On longer running machines no docker logout is run.

For example on my machine I still have "172.30.1.1:5000" in .docker/config.json - even when no oc cluster is running.

What about checking REGISTRY_ADDRESS if it contains non-default value. And in that case skip docker login.

Sounds fine to me, thanks for the suggestion.

hhorak · 2018-06-07T09:09:01Z

I have some more changes in a queue, still tuning, so, please, do not merge yet.

hhorak · 2018-06-08T07:51:15Z

I believe I finished the changes, so it should be ready for a review.

Except additions, there is one incompatible change worth mentioning: since -p NAME is not set in all templates, it is not used in ct_os_test_template_app_func any more, so templates which need it set, must set it explicitly as a parameter. Typically it means to add -p NAME=perl-testing when working with e.g. perl templates, because otherwise the template defines a different name and the functions won't know which pod to work with.

I checked the existing images and adding the -p NAME=<img>-testing fixed this issue in all of them.

omron93 · 2018-06-08T11:35:57Z

test-lib-openshift.sh

+# Deletes all objects within the project.
+# Handy when we have one project and want to run more tests.
+function ct_delete_all_objects() {
+  for x in bc builds is dc svc po routes secrets ; do


Only these types? or everything? - see oc types or https://docs.openshift.com/enterprise/3.0/cli_reference/basic_cli_operations.html#object-types

Thanks, will extend this list.

Current list is:
https://docs.openshift.com/container-platform/3.9/cli_reference/basic_cli_operations.html#object-types

Thanks for the oc types tip :).

Actually I did not include events, since it produced too many errors that I'm not able to remove them (external cluster). I think it's ok to keep events as a garbage after the test run.

omron93 · 2018-06-08T11:43:47Z

test-lib-openshift.sh

@@ -460,7 +494,19 @@ function ct_os_test_s2i_app_func() {

  ct_os_new_project
  # Create a specific imagestream tag for the image so that oc cannot use anything else
-  ct_os_upload_image "${image_name}" "${image_tagged}"
+  if [ "${CT_SKIP_UPLOAD_IMAGE:-false}" == 'true' ] ; then


Do we have usage for both cases? Wouldn't oc import-image fits our current usage in tests too? @pkubatrh What do you think?

I'm not sure whether import-image works with the local docker -- if it would, then it would make it easier. @pvalena might know maybe, have you tried that?

AFAIR It should work, if you already have the image locally. But then you have to setup the connection to remote registry in your OpenShift instance and use it for the import.

Looking at it more --from=docker.io/openshift/ruby-20-centos7 should probably work even with the IP of the host. But I tried the access to host (via some IP), so it may be blocked internally by some OpenShift routing system.

But if you only have the image in the local registry, that is not visible to the remote cluster, this won't work.

omron93 · 2018-06-08T11:48:25Z

test-lib-openshift.sh

+# --------------------
+# Runs a command CMD inside a special command pod
+# Arguments: cmd - shell command with args to run in a pod
+function ct_os_cmd_image_run() {


What about automatic calling ct_os_deploy_cmd_image on first ct_os_cmd_image_run invocation?

ct_os_deploy_cmd_image requires an image name to be used, so we would either need to specify image for each ct_os_cmd_image_run call, or run a default (python) image for those cases. I don't have a strong opinion about which variant is better, or whether the current solution is enough.. Feel free to express your preference.

You are right. Maybe better to require explicit call to deploy image.

praiskup · 2018-06-11T10:14:35Z

I'd skip the deep review, and giving +1.

pvalena · 2018-08-03T22:08:30Z

test-lib-openshift.sh

+spec:
+  containers:
+  - name: command-container
+    image: ${image_name}


This should be in quotes if I'm not mistaken.

Thx, will use quotes.

pvalena · 2018-08-03T22:12:37Z

test-lib-openshift.sh

+  SECONDS=0
+  echo -n "Waiting for command POD ."
+  while [ $SECONDS -lt 60 ] ; do
+    ct_os_cmd_image_run "echo 24" &>/dev/null && echo "DONE" && return 0


How about using capturing the stdout and using grep -q to check for an actual command execution.

sout="$(bash -c 'echo $((11*11))')"; grep -q '^121$' <<< "$sout" && echo OK

Good idea :)

pvalena · 2018-08-03T22:33:19Z

test-lib-openshift.sh

+ct_os_get_image_from_pod() {
+  local pod_prefix=$1 ; shift
+  local pod_name=$(ct_os_get_pod_name $pod_prefix)
+  oc get "po/${pod_name}" -o yaml | grep '^    image: ' | head -n 1 | sed -e 's|^    image: ||'


You can use (IMO) simplified and resilient:

oc get "po/${pod_name}" -o yaml | sed -ne 's/^\s*image:\s*$.*$\s*$/\1/ p' | head -1

Ok, will try.

pvalena · 2018-08-03T22:36:58Z

test-lib-openshift.sh

+  while true ; do
+    output=$(ct_os_cmd_image_run "$check_command_exp")
+    ret=$?
+    echo "${output}" | grep -qe "${expected_content_match}" || ret=1


Checking for correct output of ct_os_cmd_image_run for 60 in loop seconds seem quite duplicated (I've counted 3) in this PR- maybe you could use a helper function?

I'd add for some next improvements, it is used differently each time, so I don't consider this a big deal for now.

I could probably create a PR for that.

Great. If you'd be blocked by this one, feel free to merge it, I didn't want to do it before pto and leave it unfixed in other repos (the -p NAME=perl-testing thing mentioned above.. but otherwise this pr is kinda merge-able.

pvalena · 2018-08-03T22:38:46Z

Otherwise LGTM- but I've not tested it.

omron93 · 2018-08-28T10:57:00Z

@hhorak Please rebase. Otherwise, +1.

…luster Allow to not run local cluster, if already logged into any (remote/local) cluster. Allow to use external docker registry and allow skipping image upload and new project creation. Add posibility to import images and use explicit image for default cmd (that is used for testing). New environment variables: * CT_SKIP_NEW_PROJECT: if set to true, no new project is created * CT_SKIP_UPLOAD_IMAGE: if set to true, image is not uploaded to the cluster * CT_NAMESPACE: possibility to re-define the namespace for images New functions: * ct_os_test_cmd_internal: runs a check cmd for verification of the service in the container that is internal in the cluster * ct_os_test_response_internal: checks a response using curl from the container that is internal in the cluster * ct_os_cmd_image_run: run a cmd in the container that is internal in the cluster * ct_os_deploy_cmd_image: deploys a helper container, that is used for commands that needs to run internally in the cluster * ct_os_get_image_from_pod: helper function for getting image name with a register from an existing pod Incompatible change: -p NAME is not set for all templates in ct_os_test_template_app_func, so templates which need it, must set it explicitly as a parameter Other changes: * some calls (curl etc.) are done from within the cluster by default, from a different (helper) container. That's necessary for clusters that do not expose services outside.

hhorak · 2018-09-18T20:11:44Z

Let's do it, rebasing and merging.

hhorak changed the title ~~Do not run local cluster, if already logged into any (remote/local) cluster~~ Allow to run with other than local cluster Jun 5, 2018

omron93 reviewed Jun 6, 2018

View reviewed changes

hhorak changed the title ~~Allow to run with other than local cluster~~ [WIP] Allow to run with other than local cluster Jun 7, 2018

hhorak force-pushed the no-cluster-up-logged branch from d1170b9 to a579837 Compare June 8, 2018 07:47

hhorak changed the title ~~[WIP] Allow to run with other than local cluster~~ Allow to run with other than local cluster Jun 8, 2018

omron93 reviewed Jun 8, 2018

View reviewed changes

hhorak force-pushed the no-cluster-up-logged branch from c59778f to dde6cc0 Compare June 8, 2018 15:58

pvalena reviewed Aug 3, 2018

View reviewed changes

hhorak added 3 commits September 18, 2018 22:11

Add more object types

44f8141

Some small enhancments done based on review

49fccdd

hhorak force-pushed the no-cluster-up-logged branch from feae0b2 to 49fccdd Compare September 18, 2018 20:11

hhorak merged commit 10879d7 into sclorg:master Sep 18, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to run with other than local cluster #91

Allow to run with other than local cluster #91

hhorak commented Jun 5, 2018 •

edited

Loading

omron93 Jun 6, 2018

hhorak Jun 7, 2018

hhorak commented Jun 7, 2018

hhorak commented Jun 8, 2018 •

edited

Loading

omron93 Jun 8, 2018 •

edited

Loading

hhorak Jun 8, 2018

pvalena Jun 8, 2018

hhorak Jun 8, 2018

omron93 Jun 8, 2018 •

edited

Loading

hhorak Jun 8, 2018

pvalena Jun 11, 2018

pvalena Jun 11, 2018

hhorak Aug 24, 2018

omron93 Jun 8, 2018

hhorak Jun 8, 2018

omron93 Jun 11, 2018

praiskup commented Jun 11, 2018

pvalena Aug 3, 2018

hhorak Aug 24, 2018

pvalena Aug 3, 2018

hhorak Aug 24, 2018

pvalena Aug 3, 2018

hhorak Aug 24, 2018

pvalena Aug 3, 2018

hhorak Aug 24, 2018

pvalena Aug 24, 2018

hhorak Aug 24, 2018 •

edited

Loading

pvalena commented Aug 3, 2018

omron93 commented Aug 28, 2018

hhorak commented Sep 18, 2018

Allow to run with other than local cluster #91

Allow to run with other than local cluster #91

Conversation

hhorak commented Jun 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hhorak commented Jun 7, 2018

hhorak commented Jun 8, 2018 • edited Loading

omron93 Jun 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omron93 Jun 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

praiskup commented Jun 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hhorak Aug 24, 2018 • edited Loading

Choose a reason for hiding this comment

pvalena commented Aug 3, 2018

omron93 commented Aug 28, 2018

hhorak commented Sep 18, 2018

hhorak commented Jun 5, 2018 •

edited

Loading

hhorak commented Jun 8, 2018 •

edited

Loading

omron93 Jun 8, 2018 •

edited

Loading

omron93 Jun 8, 2018 •

edited

Loading

hhorak Aug 24, 2018 •

edited

Loading