improve the option to upgrade from a previous rhoai version to a newer one on selfmanaged cluster #2011
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Robocop found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Robot Results
|
kobihk
changed the title
| @@ -155,7 +155,7 @@ Get RHODS Version | |||
| ${RHODS_VERSION}= Run oc get csv -n ${OPERATOR_NAMESPACE} | grep "opendatahub" | awk -F ' {2,}' '{print $3}' | |||
| END | |||
| END | |||
| Log Product:${PRODUCT} Version:${RHODS_VERSION} | |||
| Log To Console Product:${PRODUCT} Version:${RHODS_VERSION} | |||
There was a problem hiding this comment.
you can do both things by adding "console=yes" in the Log parameters
There was a problem hiding this comment.
I like the descriptive keyword
Unless there is an advantage by using the param "console=yes"
There was a problem hiding this comment.
Log to console doesn't log in the HTML report no? while Log with console=yes does both
There was a problem hiding this comment.
Good to know :)
I'll update it.
Thank you!!
| ${csv_created}= Run Process oc get csv --no-headers -n ${operators_namespace} | awk '/${display_name}/ {print \$1}' shell=yes | ||
| IF "${csv_created.stdout}" == "${EMPTY}" CONTINUE | ||
| Log To Console The Result of csv_created Output is: ${csv_created.stdout} RC: ${csv_created.rc} | ||
| IF '$csv_created.stdout' == '${EMPTY}' CONTINUE |
There was a problem hiding this comment.
why changing from "${csv_created.stdout}" to '$csv_created.stdout'? it is a regular string check, the original version should work just fine
There was a problem hiding this comment.
I did it because of this error:
Variables in the original expression ''${csv_created.stdout}' == '${EMPTY}'' were resolved before the expression was evaluated. Try using ''$csv_created.stdout' == '$EMPTY'' syntax to avoid that. See Evaluating Expressions appendix in Robot Framework User Guide for more details.
There was a problem hiding this comment.
but this is not a new code, was it broken before?
ods_ci/tests/Resources/Common.robot
Outdated
| IF ${csv_ready.rc} == ${0} BREAK | ||
| END | ||
|
|
||
| Check For Resource Exist In Command Output |
There was a problem hiding this comment.
it's actually running the command, not just checking the output
Run Command And Fail If Output Is Empty maybe?
There was a problem hiding this comment.
I updated the Keyword name and I also added doc string to make it clear
ods_ci/tasks/Tasks/rhods_olm.robot
Outdated
| IF "${RHOAI_VERSION}" != "${EMPTY}" | ||
| ${rhoai_version} = Set Variable ${RHOAI_VERSION} | ||
| END |
There was a problem hiding this comment.
cannot you just pass ${RHOAI_VERSION} to the next line? Also, is RHOAI version is actually empty, next line may fail because the ${rhoai_version} variable wouldn't exist
There was a problem hiding this comment.
sounds reasonable :)
| IF ${patch_status} == 0 | ||
| Log To Console Approving the installplan ${installplan_name} successfully!! | ||
| ELSE | ||
| Log To Console Failed to approving the installplan ${installplan_name} |
ods_ci/tests/Tests/100__deploy/120__upgrades/121__during_upgrade.robot
Outdated
Show resolved
Hide resolved
| data["CLUSTER_TYPE"] = config_data["CLUSTER_TYPE"] | ||
| data["TEST_ENV"] = config_data["TEST_ENV"] | ||
| data["INSTALL_TYPE"] = config_data["INSTALL_TYPE"] if config_data.get("INSTALL_TYPE") else "OperatorHub" |
There was a problem hiding this comment.
we haven't had need for it so far, why this is necessary now?
There was a problem hiding this comment.
I think it would be better to add them to the test config file instead of sending them each time like we are doing in:
https://gitlab.cee.redhat.com/ods/jenkins/-/blob/master/vars/deployRhoaiOperator.groovy?ref_type=heads#L110-180
There was a problem hiding this comment.
in your pr for CI, it is still passing the parameter, so maybe this change is not necessary now. Up to you, important thing is that we don't introduce new regression risk for things we won't use
…hen to upgrade Signed-off-by: Kobi Hakimi <[email protected]>
kobihk
force-pushed
the
add_option_for_rhoai_upgrade_2.10.0
branch
from
d6bf3c8 to
3547f04
Compare
|
| # In case of upgrade there are 2 operators, we need to wait until only one will be available | ||
| ${lines}= Split String ${csv_created.stdout} \n | ||
| ${line_count}= Get Length ${lines} | ||
| IF ${line_count} > 1 CONTINUE | ||
| ${csv_ready}= Run Process | ||
| ... oc wait --timeout\=${timeout} --for jsonpath\='{.status.phase}'\=Succeeded csv -n ${operators_namespace} ${csv_created.stdout} shell=yes |
There was a problem hiding this comment.
hm shouldn't you pass the CSV name as well?
| Log To Console ${resource_type} ${resource_name} in the namespace ${namespace} attribute ${attribute_path} value is EMPTY | ||
| END | ||
| RETURN ${value} | ||
|
|
||
| Wait For Installplan And Approve It | ||
| [Documentation] Wait for the operator's installplan to appear then approve it in case of Manual installplan approval |
There was a problem hiding this comment.
could you check if we could re-use code instead of creating a new keyword?
In this MR I updated the upgrade logic to be able to run upgrade of our rhoai operator in case of:
Related to Jira ticket: RHOAIENG-2720
Signed-off-by: Kobi Hakimi [email protected]