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.

Sign up for GitHub

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

Jump to bottom

Install-driven enhancements #242

Merged
merged 24 commits into from
Aug 30, 2024
Merged

Install-driven enhancements #242

merged 24 commits into from
Aug 30, 2024

Conversation

ydirson
Copy link
Contributor

@ydirson ydirson commented Jul 24, 2024

This is a big collection of fixes and enhancements I found useful and/or necessary while working on #226

@ydirson ydirson force-pushed the install-driven-enhancements branch 3 times, most recently from ec27326 to fddb534 Compare July 25, 2024 08:30
benjamreis
benjamreis reviewed Jul 25, 2024
View reviewed changes
lib/commands.py Outdated Show resolved Hide resolved
@gduperrey
Copy link
Contributor

As seen with the failed run on the CI:

  • 8.2 xen ❌
    Currently the CI is running through Jenkins on AlmaLinux release 8.10 (Cerulean Leopard) with Python 3.6.8.
    It seems the version is too old for these changes.
    So there are discussions to be about modification of the tests or trying to upgrade the almalinux of the Jenkins servers (3) to version 9 (Python 3.9.18).

@ydirson ydirson force-pushed the install-driven-enhancements branch from fddb534 to 595a85f Compare July 25, 2024 10:16
@ydirson
Copy link
Contributor Author

ydirson commented Jul 25, 2024

As seen with the failed run on the CI:

* 8.2 xen ❌
  Currently the CI is running through Jenkins on AlmaLinux release 8.10 (Cerulean Leopard) with Python 3.6.8.
  It seems the version is too old for these changes.
  So there are discussions to be about modification of the tests or trying to upgrade the almalinux of the Jenkins servers (3) to version 9 (Python 3.9.18).

Removed for now the commit that requires python 3.8.

@ydirson ydirson requested a review from benjamreis July 25, 2024 12:01
@ydirson ydirson mentioned this pull request Jul 25, 2024
Draft
@ydirson
Copy link
Contributor Author

ydirson commented Jul 25, 2024

Added 2 things I was missing with the logs:

  • conftest: add pytest hook to make test results visible from fixtures
  • pytest: add milliseconds to log timestamp

@ydirson ydirson force-pushed the install-driven-enhancements branch from 5ef879f to ccc3a46 Compare July 25, 2024 14:09
@ydirson
Copy link
Contributor Author

ydirson commented Jul 25, 2024

(now with proper commit messages)

@gduperrey
Copy link
Contributor

run CI 8.2: ✅

stormi
stormi reviewed Aug 1, 2024
View reviewed changes
lib/host.py Show resolved Hide resolved
lib/vm.py Outdated Show resolved Hide resolved
lib/common.py Outdated Show resolved Hide resolved
lib/host.py Outdated Show resolved Hide resolved
tests/misc/test_basic_without_ssh.py Show resolved Hide resolved
tests/misc/test_basic_without_ssh.py Outdated Show resolved Hide resolved
tests/guest-tools/unix/test_guest_tools_unix.py Show resolved Hide resolved
lib/commands.py Outdated Show resolved Hide resolved
@ydirson ydirson force-pushed the install-driven-enhancements branch 2 times, most recently from 9275afd to c62734a Compare August 8, 2024 13:34
@ydirson ydirson requested a review from stormi August 8, 2024 13:35
@ydirson ydirson force-pushed the install-driven-enhancements branch from c62734a to ef6c0d3 Compare August 8, 2024 14:16
@ydirson
Copy link
Contributor Author

ydirson commented Aug 9, 2024

Also adding to next iteration LocalCommandFailed: fix ctor upcall commit, fixing one pylint error.

@ydirson ydirson force-pushed the install-driven-enhancements branch from ef6c0d3 to 5efb070 Compare August 9, 2024 09:00
@ydirson ydirson requested a review from stormi August 9, 2024 09:05
stormi
stormi reviewed Aug 9, 2024
View reviewed changes
conftest.py Outdated Show resolved Hide resolved
@ydirson ydirson force-pushed the install-driven-enhancements branch from 5efb070 to fd2ca15 Compare August 9, 2024 09:55
@ydirson
Host: rename main_sr to main_sr_uuid
1450f8c 
Other methods to get SRs return `SR` objects, this was inconsistent.

Signed-off-by: Yann Dirson <[email protected]>
@ydirson
local_cmd: log command to be run, like ssh does
7a987e4 
Signed-off-by: Yann Dirson <[email protected]>
@ydirson
VM: fix name of method dealing with CD insertion, provide a generic API
84af37b 
"vm-cd-insert" does not mount anything, as the need for a subsequent
"mount" in the only test using it shows.

Signed-off-by: Yann Dirson <[email protected]>
@ydirson
VM: add logging for CD insert/eject
15ce1bd 
Signed-off-by: Yann Dirson <[email protected]>
@ydirson
_ssh: remove duplicate default values
0d985f3 
The entrypoint is ssh(), and the default values are already there.

Signed-off-by: Yann Dirson <[email protected]>
@ydirson
Remove redundant logs on VM creation
e3dacc0 
Since f9b5365 the VM ctor takes care of
reporting this info.

Signed-off-by: Yann Dirson <[email protected]>
@ydirson
host.import_vm: report when VM not found in cache
7f963e1 
Signed-off-by: Yann Dirson <[email protected]>
@ydirson
host: split cached_vm identification from import_vm
820b38d 
This will be useful to the plugin that allows not rerunning a cached
dependency: it needs to probe the cache.

Signed-off-by: Yann Dirson <[email protected]>
@ydirson
requirements: cleanup cruft, avoid duplication in README
16e44d4 
Signed-off-by: Yann Dirson <[email protected]>
@ydirson
try_get_and_store_ip: raise boot timeout for nested tests
7282e94 
When testing in a nested setup, 2 min is not enough to boot a Debian 12
to the point the XS agent has published to xenstore.

5 min should be enough for everyone.

Signed-off-by: Yann Dirson <[email protected]>
@ydirson
test_guest_tools_unix: make requirements more explicit
95f7c1b 
Signed-off-by: Yann Dirson <[email protected]>
@ydirson
test_basic_without_ssh: split start/stop test from those needing a ru…
6d4a5b1 
…nning VM.

There is no other inter-test dependency in this file, this allows to stop
using @pytest.mark.incremental here.

Signed-off-by: Yann Dirson <[email protected]>
@ydirson
test_basic_without_ssh: clarify requirements a bit (fixes #244)
4f79b05 
Signed-off-by: Yann Dirson <[email protected]>
@ydirson
test_guest_tools_unix: use a fixture instead of expecting test_install
e445e03 
Same as for test_basic_without_ssh, and this allows to stop using
@pytest.mark.incremental here too.

Signed-off-by: Yann Dirson <[email protected]>
@ydirson
Drop "incremental" marker.
14eeee6 
Previous commits removed its two only uses, replaced by an autouse fixture.
Other possible uses would be covered (with a more fine-grained approach)
by pytest-dependency.

This frees the global pytest_runtest_makereport hook for potential reuse.

Signed-off-by: Yann Dirson <[email protected]>
@ydirson
conftest: regroup all pytest hooks together, separated from fixtures
980080c 
Signed-off-by: Yann Dirson <[email protected]>
@ydirson
CACHE_IMPORTED_VM: parse only once
c66ddcf 
We will need this value in other places.

Signed-off-by: Yann Dirson <[email protected]>
@ydirson
BaseVM: add missing param_* methods
a41cdbd 
And make the whole easier to copypaste.

Signed-off-by: Yann Dirson <[email protected]>
@ydirson
vm: remove duplicate file_exists()
04008cd 
Reported by mypy.

Signed-off-by: Yann Dirson <[email protected]>
@ydirson
Host.join_pool(): update the host's pool after new pool is joined
2fe3e54 
Signed-off-by: Yann Dirson <[email protected]>
@ydirson
pytest: do not restrict timestamping to CLI
b700f55 
We want to have it in log-file.

Signed-off-by: Yann Dirson <[email protected]>
@ydirson
pytest: add milliseconds to log timestamp
4de1e3d 
Signed-off-by: Yann Dirson <[email protected]>
@ydirson
LocalCommandFailed: fix ctor upcall
462ff23 
Signed-off-by: Yann Dirson <[email protected]>
@ydirson ydirson force-pushed the install-driven-enhancements branch from dba826f to 462ff23 Compare August 28, 2024 16:22
@ydirson
Copy link
Contributor Author

ydirson commented Aug 28, 2024

rebased, no change

@ydirson ydirson mentioned this pull request Aug 29, 2024
Merged
@stormi
Copy link
Member

stormi commented Aug 30, 2024

Full CI 8.2 run was OK on this branch. Ready to merge!

@stormi stormi merged commit 6b4e59e into master Aug 30, 2024
4 checks passed
@stormi stormi deleted the install-driven-enhancements branch August 30, 2024 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gthvn1 gthvn1 gthvn1 approved these changes

@stormi stormi stormi approved these changes

@gduperrey gduperrey gduperrey approved these changes

@benjamreis benjamreis Awaiting requested review from benjamreis

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ydirson @gduperrey @stormi @gthvn1 @benjamreis