-
Notifications
You must be signed in to change notification settings - Fork 84
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
RHOAIENG-2508 ODS-2600 Validate DSC creates all Serverless CRs #1259
Changes from 5 commits
e15eb4d
132594f
bd5184c
af7b4d8
de5d3e9
8c813bb
46b2b76
8b1f734
e0fc0be
edafa97
9c34b0d
b46accf
ae0d917
43f0a2d
53e9465
e68dd21
a699a90
35c9761
1c68c23
4261ec6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,116 @@ | ||||||||
*** Settings *** | ||||||||
Documentation Test Cases to verify Serverless installation | ||||||||
Library Collections | ||||||||
Library SeleniumLibrary | ||||||||
Library OpenShiftLibrary | ||||||||
Resource ../../../../Resources/Page/OCPDashboard/OCPDashboard.resource | ||||||||
Suite Setup Suite Setup | ||||||||
Suite Teardown Suite Teardown | ||||||||
|
||||||||
|
||||||||
*** Variables *** | ||||||||
${SERVERLESS_APPNAME} Red Hat OpenShift Serverless | ||||||||
${SERVERLESS_OPERATOR_NAME} Red Hat OpenShift Serverless | ||||||||
${KNATIVESERVING_NS} knative-serving | ||||||||
${ISTIO_NS} istio-system | ||||||||
|
||||||||
|
||||||||
*** Test Cases *** | ||||||||
Validate DSC creates all Serverless CRs | ||||||||
Check warning Code scanning / Robocop Test case '{{ test_name }}' has too many keywords inside ({{ keyword_count }}/{{ max_allowed_count }}) Warning test
Test case 'Validate DSC creates all Serverless CRs' has too many keywords inside (12/10)
|
||||||||
[Documentation] The purpose of this Test Case is to validate the creation | ||||||||
... of Serverless Custom Resources | ||||||||
[Tags] Operator ODS-2600 | ||||||||
Assign Vars According To Product ${PRODUCT} | ||||||||
Check And Install Operator in Openshift ${OPERATOR_APPNAME} ${OPERATOR_NAME} | ||||||||
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Show resolved
Hide resolved
|
||||||||
Check And Install Operator in Openshift ${SERVERLESS_APPNAME} ${SERVERLESS_OPERATOR_NAME} | ||||||||
|
||||||||
Is Resource Present KnativeServing knative-serving ${KNATIVESERVING_NS} | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DSC must be created first in Self-managed installations |
||||||||
Check Status oc get KnativeServing knative-serving -n ${KNATIVESERVING_NS} -o json | jq '.status.conditions[] | select(.type=="Ready") | .status' KnativeServing "True" # robocop: disable | ||||||||
Is Resource Present Gateway knative-ingress-gateway ${KNATIVESERVING_NS} | ||||||||
Is Resource Present Gateway knative-local-gateway ${KNATIVESERVING_NS} | ||||||||
Is Resource Present Service knative-local-gateway ${ISTIO_NS} | ||||||||
Is Resource Present deployment controller ${KNATIVESERVING_NS} | ||||||||
Wait For Pods Numbers 2 namespace=${KNATIVESERVING_NS} | ||||||||
... label_selector=app.kubernetes.io/component=controller timeout=120 | ||||||||
${pod_names}= Get Pod Names ${KNATIVESERVING_NS} app.kubernetes.io/component=controller | ||||||||
Verify Containers Have Zero Restarts ${pod_names} ${KNATIVESERVING_NS} | ||||||||
${podname}= Get Pod Name ${OPERATOR_NAMESPACE} label_selector=name=rhods-operator | ||||||||
Check For Errors On Operator Logs ${podname} ${OPERATOR_NAMESPACE} | ||||||||
Check DSC Conditions ${KNATIVESERVING_NS} default-dsc | ||||||||
|
||||||||
|
||||||||
*** Keywords *** | ||||||||
Suite Setup | ||||||||
[Documentation] Suite Setup | ||||||||
Set Library Search Order SeleniumLibrary | ||||||||
RHOSi Setup | ||||||||
|
||||||||
Suite Teardown | ||||||||
[Documentation] Suite Teardown | ||||||||
Close All Browsers | ||||||||
RHOSi Teardown | ||||||||
|
||||||||
Assign Vars According To Product | ||||||||
jgarciao marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
[Documentation] Assign vars related to product | ||||||||
[Arguments] ${product} | ||||||||
IF "${product}" == "RHODS" | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should be ${PRODUCT}, as it is a global variable There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks Jorge, I change it |
||||||||
Set Suite Variable ${OPERATOR_APPNAME} Red Hat OpenShift AI | ||||||||
Check notice Code scanning / Robocop {{ set_variable_keyword }} can be replaced with VAR Note test
Set Suite Variable can be replaced with VAR
|
||||||||
Set Suite Variable ${OPERATOR_NAME} Red Hat OpenShift AI | ||||||||
|
||||||||
Set Suite Variable ${OPERATOR_NAMESPACE} redhat-ods-operator | ||||||||
|
||||||||
ELSE IF "${product}" == "ODH" | ||||||||
Set Suite Variable ${OPERATOR_APPNAME} Open Data Hub Operator | ||||||||
Check notice Code scanning / Robocop {{ set_variable_keyword }} can be replaced with VAR Note test
Set Suite Variable can be replaced with VAR
|
||||||||
Set Suite Variable ${OPERATOR_NAME} Open Data Hub Operator | ||||||||
|
||||||||
Set Suite Variable ${OPERATOR_NAMESPACE} openshift-operators | ||||||||
|
||||||||
END | ||||||||
|
||||||||
Is Resource Present | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. changed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @asanzgom pay attention that @mattmahoneyrh has already implemented this keyword in a common resources place https://github.com/red-hat-data-services/ods-ci/pull/1336/files#diff-38a9e44ef5c23ae2eb5d0d3f12eb1431c7a808bd85570813c0fc6a593154af44 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will talk with him to agree on using one of them
jgarciao marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
[Documentation] Check CR | ||||||||
[Arguments] ${resource} ${resource_name} ${namespace} | ||||||||
${rc}= Run and Return Rc | ||||||||
|
||||||||
... oc get ${resource} ${resource_name} -n ${namespace} | ||||||||
Should Be Equal "${rc}" "0" msg=${resource} does not exist | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. did you change it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. committed |
||||||||
|
||||||||
Check Status | ||||||||
jgarciao marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
[Documentation] Check Resource Status | ||||||||
[Arguments] ${oc_get} ${resource} ${expected_status} | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why taking the full oc get command as input? I think the command should go inside the keyword There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because this way the keyword is more generic for future uses There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but if you put the entire command in the test, at this point you don't need this keyword, you can just do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well, I am also logging the status to the console There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
${status} contains the result of your command no? So it's not different from the approach I wrote in my comment. Anyways, don't want to insist, you can leave the kw as it is if you think it's helpful. |
||||||||
${status}= Run | ||||||||
... ${oc_get} | ||||||||
Log ${status} console=True | ||||||||
Should Be Equal ${status} ${expected_status} msg=${resource} is not in Ready status | ||||||||
|
||||||||
Check DSC Conditions | ||||||||
[Documentation] Checks that all conditions Reconciled Succesfully | ||||||||
[Arguments] ${namespace} ${dsc_name} | ||||||||
${rc} ${out}= Run And Return Rc And Output | ||||||||
... oc get DataScienceCluster ${dsc_name} -n ${namespace} -o jsonpath='{.status.conditions[].reason}' | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How exactly we check the condition here? 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is meant to be displayed as it is very prone to errors depending on the installation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, the status of the components isn't stable... this is something I think that operator could also improve... the statuses are regularly changing even in case that all is fine (Ready vs Unknown). Again, I am new to the OpenShift world, so my assumption can be wrong. But wanted to say that this doesn't check anything... it just prints some info... so if that is desired and won't be changed, please rename the keyword appropriately so it's not confusing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. changed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just realized that we are checking the condition of just one component - then... shouldn't we want it to be in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. changed the keyword to wait for Conditions on DSC to be Reconciled successfully or timeout |
||||||||
Should Be Equal As Integers ${rc} ${0} | ||||||||
Log ${out} console=${out} | ||||||||
|
||||||||
Check For Errors On Operator Logs | ||||||||
[Documentation] Checks there are no errors on Operator Logs | ||||||||
[Arguments] ${operator_name} ${operator_namespace} | ||||||||
${pod_logs}= Oc Get Pod Logs name=${operator_name} namespace=${operator_namespace} container=rhods-operator | ||||||||
${error_present}= Run Keyword And Return Status Should Contain ${pod_logs} ERROR | ||||||||
IF ${error_present} | ||||||||
Log message=Check Pod Logs, ERROR level logs found. level=WARN | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just warning here? Errors in operator logs are just fine? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are ERROR level logs at the start because of Reconciliation sync, then it is fine There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, so shouldn't we limit to check logs after some time when the operator is stable? Otherwise, do you plan to open these test results even when they are green and search for the content of this? Also, it would be nice if operator would be smart enough to try to understand the context and reduce emitting errors at the beginning when components are starting. But I'm new to the OpenShift world, so my assumption may be just wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @zdtsw This is something that maybe could be improved on the operator side There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. which part need to be done in operator? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not printing ERROR logs due to Reconcile if afterward everything goes fine
I see this error everytime Operator gets installed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @asanzgom regarding this test - so no plan to wait until operator is stable and check only the portion of the logs after that? Also, looks like it just prints this warning message but doesn't print any actual error in the log... it's kind of useless, isn't it? 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to check the Operator's Pod Logs for errors. There is one Test Case that could be refactored into a reusable Keyword for this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with Jan's points. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactored to fail when ERROR reg ex is found on logs and added ProductBug Tag |
||||||||
ELSE | ||||||||
Log message=No ERROR logs found on Pod. level=INFO | ||||||||
END | ||||||||
|
||||||||
Extract Errors From Logs | ||||||||
asanzgom marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
[Documentation] Given Pod Logs it retrieves only the ERROR level ones. | ||||||||
[Arguments] ${logs} | ||||||||
${log_splits}= Split String string=${logs} separator=. | ||||||||
${value}= Set Variable ${logs} | ||||||||
FOR ${idx} ${split} IN ENUMERATE @{log_splits} start=1 | ||||||||
Log ${idx} - ${split} | ||||||||
${present}= Run Keyword And Return Status | ||||||||
... Should Contain ${split} ERROR | ||||||||
IF ${present} | ||||||||
${value}= Set Variable ${value["${split}"]} | ||||||||
Log message=No ERROR logs found on Pod. level=WARN | ||||||||
ELSE | ||||||||
${value}= Set Variable ${EMPTY} | ||||||||
Log message=No ERROR logs found on Pod. level=INFO | ||||||||
BREAK | ||||||||
END | ||||||||
END |
Check warning
Code scanning / Robocop
Invalid number of empty lines between sections ({{ empty_lines }}/{{ allowed_empty_lines }}) Warning test