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

Add vcomlink test #1016

Merged
merged 3 commits into from
Oct 11, 2024
Merged

Add vcomlink test #1016

merged 3 commits into from
Oct 11, 2024

Conversation

shjala
Copy link
Member

@shjala shjala commented Aug 21, 2024

This is a test for lf-edge/eve#4142, but I don't know which one should go first ?!

tests/vcom/base.go Outdated Show resolved Hide resolved
pkg/remote/utils.go Outdated Show resolved Hide resolved
pkg/remote/utils.go Outdated Show resolved Hide resolved
pkg/remote/utils.go Outdated Show resolved Hide resolved
@milan-zededa
Copy link
Contributor

This is a test for lf-edge/eve#4142, but I don't know which one should go first ?!

If this test depends on the EVE PR and would fail until it is merged and released, then this PR should wait.
I typically mark such eden PR as draft until there is EVE version with the tested functionality available. Then I update DefaultEVETag in pkg/defaults as part of the PR and mark it as ready for review.

@shjala shjala marked this pull request as draft August 22, 2024 10:20
@shjala
Copy link
Member Author

shjala commented Aug 22, 2024

If this test depends on the EVE PR and would fail until it is merged and released, then this PR should wait.

Yeah, I get it. But it is a conundrum, how I can show that my PR in eve works fine without merging this test, and on the other hand merging this breaks the eden tests.

@shjala shjala force-pushed the vcom_eden_test branch 2 times, most recently from 8d0c3d5 to 06b9eac Compare August 22, 2024 16:23
@shjala shjala mentioned this pull request Aug 23, 2024
4 tasks
@shjala shjala changed the title add vcomlink test [WIP] add vcomlink test Aug 26, 2024
@shjala shjala force-pushed the vcom_eden_test branch 2 times, most recently from e27c64d to 9962754 Compare August 29, 2024 16:26
@shjala shjala marked this pull request as ready for review September 2, 2024 08:46
@shjala shjala changed the title [WIP] add vcomlink test add vcomlink test Sep 2, 2024
@shjala
Copy link
Member Author

shjala commented Sep 2, 2024

This is ready to be merge as soon as we get the new eve release.

@shjala shjala force-pushed the vcom_eden_test branch 5 times, most recently from f71d237 to 1fd58e3 Compare September 4, 2024 07:35
@shjala shjala marked this pull request as draft September 4, 2024 09:03
@shjala shjala self-assigned this Sep 4, 2024
@uncleDecart
Copy link
Collaborator

@shjala so EVE 13.3.0 supports the test?

@shjala
Copy link
Member Author

shjala commented Sep 23, 2024

@shjala so EVE 13.3.0 supports the test?

yes, it runs green locally, lets see what happens here.

@shjala shjala marked this pull request as ready for review September 23, 2024 11:53
@shjala shjala force-pushed the vcom_eden_test branch 3 times, most recently from 655570b to b5dc108 Compare September 24, 2024 14:26
@shjala
Copy link
Member Author

shjala commented Sep 24, 2024

This is really frustrating, it runs fine locally (running smoke test) but here fails to wait for the app to be ready, I increased the timer to 30m but still...

@uncleDecart
Copy link
Collaborator

We need to think if there's something in Eden or problem in runners...

@giggsoff
Copy link
Collaborator

This is really frustrating, it runs fine locally (running smoke test) but here fails to wait for the app to be ready, I increased the timer to 30m but still...

Inside attached artifacts (in console logs) I can see ERROR: App vcomlink-sleepy_leakey uuid 53ff1865-61bf-43dd-bab4-ab7c1ffe2fb6 state INSTALLED error: failed to run domain 53ff1865-61bf-43dd-bab4-ab7c1ffe2fb6.1.1: not based on an OCI image. AFAIK it indicates that you try to run VM (based on qcow2 image) on host without hardware assistance acceleration (or with app in no-hyper mode in config).

BTW it would be nice to show timestanps and errors if any in state transition in test logs (from AppWaitForRunningState function). It will helps to detect such problems quickly without diving into artifacts.

@shjala
Copy link
Member Author

shjala commented Oct 9, 2024

Inside attached artifacts (in console logs) I can see ERROR: App vcomlink-sleepy_leakey uuid 53ff1865-61bf-43dd-bab4-ab7c1ffe2fb6 state INSTALLED error: failed to run domain 53ff1865-61bf-43dd-bab4-ab7c1ffe2fb6.1.1: not based on an OCI image. AFAIK it indicates that you try to run VM (based on qcow2 image) on host without hardware assistance acceleration (or with app in no-hyper mode in config).

I see the eden default DefaultEVEH is KVM and in the app config NoHyper is set to false, and expectation.virtualizationMode = config.VmMode_HVM for amd64, are you suggesting I should move this to virtualization test suite?

BTW it would be nice to show timestanps and errors if any in state transition in test logs (from AppWaitForRunningState function). It will helps to detect such problems quickly without diving into artifacts.

Good point, I will add it.

@shjala
Copy link
Member Author

shjala commented Oct 9, 2024

I see require_virtualization: true only for virtualization test in github actions, I move this test to virt.

@shjala shjala force-pushed the vcom_eden_test branch 2 times, most recently from e0efa23 to 06d3107 Compare October 9, 2024 09:55
Signed-off-by: Shahriyar Jalayeri <shahriyar@zededa.com>
AppWaitForRunningState : now reports each state changes
AppWaitForSSH : repoprts wating
EveRebootAndWait : reports waiting
AppSCPCopy : check if local file exists since it is not checked by scp

plus adjusted some sleep times.

Signed-off-by: Shahriyar Jalayeri <shahriyar@zededa.com>
This is a test for EVE PR #4142.

Signed-off-by: Shahriyar Jalayeri <shahriyar@zededa.com>
@shjala
Copy link
Member Author

shjala commented Oct 9, 2024

@giggsoff Thanks for your help. It is now passing as part of virtualization tests:

--- PASS: TestEdenScripts (0.00s)
    --- PASS: TestEdenScripts/eden_vcom (119.12s)

@milan-zededa @uncleDecart some other tests are failing but I'm not touching any part relevant to those, general flakiness? anyways this PR is ready.

@milan-zededa
Copy link
Contributor

@giggsoff Thanks for your help. It is now passing as part of virtualization tests:

--- PASS: TestEdenScripts (0.00s)
    --- PASS: TestEdenScripts/eden_vcom (119.12s)

@milan-zededa @uncleDecart some other tests are failing but I'm not touching any part relevant to those, general flakiness? anyways this PR is ready.

We will retry them just in case. At least one of them is a known failure.

@shjala
Copy link
Member Author

shjala commented Oct 10, 2024

@milan-zededa @uncleDecart should we merge this?

Copy link
Collaborator

@uncleDecart uncleDecart left a comment

Choose a reason for hiding this comment

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

LGTM from my side, if @milan-zededa is okay with patch I'd merge it

@uncleDecart uncleDecart merged commit 8545f10 into lf-edge:master Oct 11, 2024
13 of 19 checks passed
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