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

Backport required changes from master to releases/2.8.0 #1318

Merged
merged 8 commits into from
Mar 28, 2024

Conversation

jstourac
Copy link
Member

@jstourac jstourac commented Mar 25, 2024

This PR contains following fixes from the master branch:


I kept the order in which these commits were merged in the master branch and provided both link to the particular commit and also link to the particular PR so it's clearer to understand later on.

${result} = Run Process git clone -b ${KUEUE_REPO_BRANCH} ${KUEUE_REPO_URL} ${KUEUE_DIR}
... shell=true stderr=STDOUT
Log To Console "Downloading compiled test binary e2e-singlecluster"
${result} = Run Process curl --location --silent --output e2e-singlecluster ${KUEUE_RELEASE_ASSETS}/e2e-singlecluster && chmod +x e2e-singlecluster

Check warning

Code scanning / Robocop

Line is too long ({{ line_length }}/{{ allowed_length }}) Warning test

Line is too long (157/120)
@@ -145,6 +146,7 @@

Wait For Cluster To Be Ready
${pool_namespace} = Get Cluster Pool Namespace ${pool_name}
Set Task Variable ${pool_namespace}

Check notice

Code scanning / Robocop

{{ set_variable_keyword }} can be replaced with VAR Note

Set Task Variable can be replaced with VAR
@@ -145,6 +146,7 @@

Wait For Cluster To Be Ready
${pool_namespace} = Get Cluster Pool Namespace ${pool_name}
Set Task Variable ${pool_namespace}

Check warning

Code scanning / Robocop

Test, suite and global variables should be uppercase Warning

Test, suite and global variables should be uppercase
${result} = Run Process
... oc -n ${pool_namespace} get cd ${pool_namespace} -o json | jq -r '.status.webConsoleURL' --exit-status
... shell=yes
Should Be True ${result.rc} == 0

Check notice

Code scanning / Robocop

'{{ block_name }}' condition can be simplified Note

'Should Be True' condition can be simplified
@@ -62,7 +62,7 @@
[Teardown] Disable Component kueue
Wait Component Ready kueue
Log To Console Waiting for kueue-controller-manager to be available
${result} = Run Process oc wait --for\=condition\=Available --timeout\=60s -n ${ODH_NAMESPACE} deployment/kueue-controller-manager
${result} = Run Process oc wait --for\=condition\=Available --timeout\=300s -n ${ODH_NAMESPACE} deployment/kueue-controller-manager

Check warning

Code scanning / Robocop

Line is too long ({{ line_length }}/{{ allowed_length }}) Warning test

Line is too long (141/120)
${test_crs}= Create List ${CR_1} ${CR_2}
FOR ${nb_cr} IN @{test_crs}
${present}= Run Keyword And Return Status OpenshiftLibrary.Oc Get kind=Notebook namespace=${NOTEBOOK_NS} name=${nb_cr}
${test_usernames}= Create List ${TEST_USER.USERNAME} ${TEST_USER_3.USERNAME} ${TEST_USER_4.USERNAME}

Check notice

Code scanning / Robocop

{{ create_keyword }} can be replaced with VAR Note test

Create List can be replaced with VAR
${PVC_BASIC_USER}= Get User Notebook PVC Name ${TEST_USER_3.USERNAME}
${PVC_BASIC_USER_2}= Get User Notebook PVC Name ${TEST_USER_4.USERNAME}
${test_pvcs}= Create List ${PVC_BASIC_USER} ${PVC_BASIC_USER_2}
${test_pvcs}= Create List ${PVC_ADMIN_USER} ${PVC_BASIC_USER} ${PVC_BASIC_USER_2}

Check notice

Code scanning / Robocop

{{ create_keyword }} can be replaced with VAR Note test

Create List can be replaced with VAR
Copy link
Contributor

Robot Results

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

@jstourac jstourac self-assigned this Mar 25, 2024
@jstourac jstourac added the 2.8.z label Mar 25, 2024
@jiridanek
Copy link
Member

jiridanek commented Mar 25, 2024

Would it make sense to create this PR through git cherry-pick, so that the hashes of the original commits are preserved in commit messages? Looking in the github webui, I don't see the "cherry picked from commit" lines in commit messages. It is added with -x, https://mirrors.edge.kernel.org/pub/software/scm/git/docs/git-cherry-pick.html

Or do a merge with merge commit which (maybe) also preserves commit hashes?

There is this "Daggy Fixes and Cherry-picking" from https://queue.acm.org/detail.cfm?id=1595636 which I found from https://stackoverflow.com/questions/2922652/git-is-there-a-way-to-figure-out-where-a-commit-was-cherry-picked-from sounds interesting, btw. But it's too late to use it for the commits above, and I don't exactly understand how this is supposed to work in Git.

@jstourac
Copy link
Member Author

Would it make sense to create this PR through git cherry-pick, so that the hashes of the original commits are preserved in commit messages? Looking in the github webui, I don't see the "cherry picked from commit" lines in commit messages. It is added with -x, https://mirrors.edge.kernel.org/pub/software/scm/git/docs/git-cherry-pick.html

I don't know how to do these things in UI so yeah - I used git cherry-pick. Can do again with the -x will think about it tomorrow.

Or do a merge with merge commit which (maybe) also preserves commit hashes?

I was also thinking about this, to keep the merge commit there, yes.

bdattoma and others added 8 commits March 27, 2024 12:02
* adding link to prod bugs

* add ref to bugs

* add flaky test tags

(cherry picked from commit edafbcc)
related to jira ticket: RHOAIENG-3338

Signed-off-by: Kobi Hakimi <[email protected]>
(cherry picked from commit b6fb7f7)
(cherry picked from commit 9485246)
There were two headers section logged instead of one request body
section logged in the report.

(cherry picked from commit abfb8a5)
The cleanup only checked for resources of two users but in the tests
there are used three different users, so let's clean after all three.

(cherry picked from commit fc76761)
This test was probably failing since the addition of the accelerator
profiles. Since I have no idea where is some documentation to the
dashboard API which we use here, I grepped my local disk for some
keywords and was able to find this file [1] from which I was able to
understand what is wrong with this test. As such, please review
thoroughly since I could miss something.

[1]
https://github.com/opendatahub-io/odh-dashboard/blob/main/frontend/src/types.ts#L670>

(cherry picked from commit 439e2b3)
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@jstourac
Copy link
Member Author

@jiridanek I have updated this PR so that all commits now have the info they are being cherry-picked from somewhere.

For others: I removed the 3e35a71 and added 5db9e26.

@jstourac jstourac requested a review from jiridanek March 28, 2024 14:12
@jstourac jstourac marked this pull request as ready for review March 28, 2024 17:43
@jiridanek jiridanek added this to the 2.8.1 milestone Mar 28, 2024
@jiridanek jiridanek added enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) ⚠️ high priority review I need this to be reviewed ASAP labels Mar 28, 2024
@jstourac jstourac merged commit 4088985 into red-hat-data-services:releases/2.8.0 Mar 28, 2024
12 checks passed
@jstourac jstourac added the verified This PR has been tested with Jenkins label Mar 28, 2024
@jstourac jstourac deleted the masterTo28 branch March 28, 2024 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.8.z enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) verified This PR has been tested with Jenkins ⚠️ high priority review I need this to be reviewed ASAP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants