Skip to content
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

Option to use KUBECONFIG when running ODS-CI Image #1247

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

manosnoam
Copy link
Contributor

  • Add option to use KUBECONFIG when calling ods_ci/build/run.sh

  • Fix yq commands for yq version 4.25 (-r is not supported)

Running example:

podman run --rm \
-v $PWD/.setup/test-variables.yml:/tmp/ods-ci/ods_ci/test-variables.yml:Z \
-v $PWD/ods_ci/test-output:/tmp/ods-ci/ods_ci/test-output:Z \
-v ${KUBECONFIG}:/tmp/kubeconfig:Z -e KUBECONFIG=/tmp/kubeconfig \
-e RUN_SCRIPT_ARGS='--skip-oclogin true' \
-e ROBOT_EXTRA_ARGS='--include smoke' \
-e SET_ENVIRONMENT=1 -e USE_OCM_IDP=0 \
-e OC_HOST='https://api.rhoai-cluster:6443' \
ods-ci:latest

Output:

RUN SCRIPT ARGS: --skip-oclogin true
ROBOT EXTRA ARGS: --include smoke
SET TEST ENVIRONMENT: 1
-- USE OCM to install IDPs: 0
Set Kubeconfig file path to: /tmp/kubeconfig
-----| SET_ENVIRONMENT option is enabled. ODS-CI is going to create IDP users |-----
Stage) validating user_config.json
user_config.json found! Starting json validation..
--> Going through requested users configuration
validating prefix: htp-user-
--> suffix type: incremental_with_rand_base
validating prefix: htp-basic-user-
--> suffix type: incremental_with_rand_base
--> Going through requested users configuration

validating prefix: ldap-op-
--> suffix type: incremental_with_rand_base
validating prefix: ldap-usr-
--> suffix type: incremental_with_rand_base
validating prefix: ldap-noaccess-
--> suffix type: incremental_with_rand_base
validating prefix: ldap-special
--> suffix type: custom
-----| ODS-CI is starting the tests run...|-----
ods_ci/test-variables.yml
INFO: we found a yq executable
skipping OC login as per parameter --skip-oclogin

In run_robot_test.sh we called yq -er, but after upgrading to latest
yq version 4.25, it is not supported (and can break the script):
```
$ yq --version
yq (https://github.com/mikefarah/yq/) version 4.25.1
$ yq -r
Error: unknown shorthand flag: 'r' in -r
```

Signed-off-by: manosnoam <[email protected]>
Copy link
Contributor

github-actions bot commented Mar 3, 2024

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass %
429 0 0 429 100

@@ -1,28 +1,32 @@
#!/bin/bash
set -e
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome, +100

Suggested change
set -e
set -Eeo pipefail

This is the recommendation from https://vaneyckt.io/posts/safer_bash_scripts_with_set_euxo_pipefail/, leaving out -u because that needs more work

echo "Couldn't find '.OCP_ADMIN_USER.USERNAME' variable in provided '${TEST_VARIABLES_FILE}'."
echo "Please either provide it or use '--skip-oclogin true' (don't forget to login to the testing cluster manually then)."
exit 1
}
oc_pass=$(yq -er '.OCP_ADMIN_USER.PASSWORD' "${TEST_VARIABLES_FILE}") || {
oc_pass=$(yq -e '.OCP_ADMIN_USER.PASSWORD' "${TEST_VARIABLES_FILE}") || {
Copy link
Member

@jstourac jstourac Mar 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will change behavior for those who use the Python based yq implementaiton, see #953:

$ yq -e '.OCP_ADMIN_USER.PASSWORD' "ods_ci/test-variables.yml"
"heslo"
$ yq -er '.OCP_ADMIN_USER.PASSWORD' "ods_ci/test-variables.yml"
heslo
$ yq --version                                                 
yq 3.2.3

For more context, see d498907.

Note: latest GoLang based yq version is v4.42.1 (https://github.com/mikefarah/yq/releases).

Truth is that we have both implementation of yq in our CI image now. But looks like the Jenkins job still work. So we should be careful with changes here and rather than this, I would actually invite an agreement on the aforementioned issue and do a proper fix 🤔

Copy link
Member

@jstourac jstourac Mar 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the -r works for me even with the GoLang based implementation 🤔 :

$ /tmp/yq_linux_arm64 -er '.BROWSER.NAME' "ods_ci/test-variables.yml"
chrome
$ /tmp/yq_linux_arm64 --version                                      
yq (https://github.com/mikefarah/yq/) version v4.42.1

Copy link

sonarcloud bot commented Mar 4, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@chetna14manku
Copy link

@manosnoam we tested the container image created from this PR and these are the logs

---> Adding additional HTP users, if needed per requested configuration
Adding password for user htp-basic-user-dvQBzwOVXjmbHAEIUPCM1
Adding password for user htp-basic-user-dvQBzwOVXjmbHAEIUPCM2
secret/htpasswd-secret replaced
Stage) Setting LDAP Identity provider
--> Generating users based on requested configuration
elaborating prefix: ldap-op-
--> suffix type: incremental_with_rand_base
elaborating prefix: ldap-usr-
--> suffix type: incremental_with_rand_base
elaborating prefix: ldap-noaccess-
--> suffix type: incremental_with_rand_base
elaborating prefix: ldap-special
--> suffix type: custom
--> configuring LDAP server and users
namespace/openldap created
secret/openldap created
service/openldap created
deployment.apps/openldap created
secret/ldap-bind-password created
oauth.config.openshift.io/cluster patched
Stage) Configure RHODS test user groups
Error from server (AlreadyExists): groups.user.openshift.io "rhods-admins" already exists
group.user.openshift.io/rhods-users created
group.user.openshift.io/rhods-noaccess created
group.user.openshift.io/dedicated-admins created
rhods-admins dedicated-admins
group.user.openshift.io/rhods-admins added: "ldap-op-NhARCyYLXfZbnrPlDmvS1"
group.user.openshift.io/rhods-admins added: "ldap-op-NhARCyYLXfZbnrPlDmvS2"
group.user.openshift.io/rhods-admins added: "ldap-op-NhARCyYLXfZbnrPlDmvS3"
group.user.openshift.io/rhods-admins added: "ldap-op-NhARCyYLXfZbnrPlDmvS4"
group.user.openshift.io/rhods-admins added: "ldap-op-NhARCyYLXfZbnrPlDmvS5"
group.user.openshift.io/dedicated-admins added: "ldap-op-NhARCyYLXfZbnrPlDmvS1"
group.user.openshift.io/dedicated-admins added: "ldap-op-NhARCyYLXfZbnrPlDmvS2"
group.user.openshift.io/dedicated-admins added: "ldap-op-NhARCyYLXfZbnrPlDmvS3"
group.user.openshift.io/dedicated-admins added: "ldap-op-NhARCyYLXfZbnrPlDmvS4"
group.user.openshift.io/dedicated-admins added: "ldap-op-NhARCyYLXfZbnrPlDmvS5"
rhods-users
group.user.openshift.io/rhods-users added: "ldap-usr-bJPDeKLszXCEoAVxNHgc1"
group.user.openshift.io/rhods-users added: "ldap-usr-bJPDeKLszXCEoAVxNHgc2"
group.user.openshift.io/rhods-users added: "ldap-usr-bJPDeKLszXCEoAVxNHgc3"
group.user.openshift.io/rhods-users added: "ldap-usr-bJPDeKLszXCEoAVxNHgc4"
group.user.openshift.io/rhods-users added: "ldap-usr-bJPDeKLszXCEoAVxNHgc5"
rhods-noaccess
group.user.openshift.io/rhods-noaccess added: "ldap-noaccess-WUFqQpAMlGLmoZybjNRw1"
group.user.openshift.io/rhods-noaccess added: "ldap-noaccess-WUFqQpAMlGLmoZybjNRw2"
group.user.openshift.io/rhods-noaccess added: "ldap-noaccess-WUFqQpAMlGLmoZybjNRw3"
group.user.openshift.io/rhods-noaccess added: "ldap-noaccess-WUFqQpAMlGLmoZybjNRw4"
group.user.openshift.io/rhods-noaccess added: "ldap-noaccess-WUFqQpAMlGLmoZybjNRw5"
rhods-users
group.user.openshift.io/rhods-users added: "ldap-special."
group.user.openshift.io/rhods-users added: "ldap-special^"
group.user.openshift.io/rhods-users added: "ldap-special$"
group.user.openshift.io/rhods-users added: "ldap-special*"
group.user.openshift.io/rhods-users added: "ldap-special?"
group.user.openshift.io/rhods-users added: "ldap-special("
group.user.openshift.io/rhods-users added: "ldap-special)"
group.user.openshift.io/rhods-users added: "ldap-special["
group.user.openshift.io/rhods-users added: "ldap-special]"
group.user.openshift.io/rhods-users added: "ldap-special{"
group.user.openshift.io/rhods-users added: "ldap-special}"
group.user.openshift.io/rhods-users added: "ldap-special|"
group.user.openshift.io/rhods-users added: "ldap-special@"
group.user.openshift.io/rhods-users added: "ldap-special;"
Stage) Sleeping 180sec to wait for IDPs to become available
-----| ODS-CI is starting the tests run...|-----
ods_ci/test-variables.yml
INFO: we found a yq executable

@manosnoam
Copy link
Contributor Author

manosnoam commented Mar 22, 2024 via email

@jstourac
Copy link
Member

Jan, regarding ods_ci/run_robot_test <#1247 (comment)> .sh I think the CSPI team uses it only, so I don't see any risk in this pr.

Hm, but the part I commented is in the ods_ci/run_robot_test.sh file which we use normally. You're changing the behavior for everyone, I believe. Also, per my latest comment there - I don't even understand why, since -r is supported in the yq implementation you mention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants