Skip to content

test: additional check to ensure hosts are left in up state #8383

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
Merged
shwstppr merged 4 commits into from
Dec 21, 2023
Merged

test: additional check to ensure hosts are left in up state #8383

shwstppr merged 4 commits into from
Dec 21, 2023

Conversation

@shwstppr
Copy link
Contributor

@shwstppr shwstppr commented Dec 20, 2023 •
edited
Loading

Description

With this change, a fix is added for failures seen with test_08_migrate_vm or other migration-related tests because the target host is in Connecting state,
#8356 (comment)
#8374 (comment)
and more

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

@boring-cyborg boring-cyborg bot added component:integration-test Python Warning... Python code Ahead! labels Dec 20, 2023
@shwstppr
test: additional check to ensure hosts are left in up state
1951b0c 
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@shwstppr shwstppr force-pushed the test-addcheck-vmlifecycle branch from b84b654 to 1951b0c Compare December 20, 2023 11:20
@codecov
Copy link

codecov bot commented Dec 20, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (64ecd00) 13.12% compared to head (b4c74d3) 13.12%.
Report is 2 commits behind head on 4.18.

Additional details and impacted files 
@@            Coverage Diff            @@
##               4.18    #8383   +/-   ##
=========================================
  Coverage     13.12%   13.12%           
  Complexity     9141     9141           
=========================================
  Files          2720     2720           
  Lines        257723   257723           
  Branches      40177    40177           
=========================================
  Hits          33838    33838           
  Misses       219595   219595           
  Partials       4290     4290

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

weizhouapache
weizhouapache reviewed Dec 20, 2023
View reviewed changes
test/integration/smoke/test_vm_life_cycle.py Outdated
hostid=host.id,
type='Routing'
)[0]
if restarted_host.state == state:
Copy link
Member

@weizhouapache weizhouapache Dec 20, 2023

Choose a reason for hiding this comment

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

should == state be == "Up" ?

Copy link
Contributor Author

@shwstppr shwstppr Dec 20, 2023

Choose a reason for hiding this comment

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

@weizhouapache yes 🙈
But I'm not able to reproduce the issue with hosts remaining in Connecting state so not sure if this will solve the problem

shwstppr
shwstppr commented Dec 20, 2023
View reviewed changes
shwstppr
shwstppr commented Dec 20, 2023
View reviewed changes
@shwstppr
Copy link
Contributor Author

shwstppr commented Dec 20, 2023

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Dec 20, 2023

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@shwstppr
Copy link
Contributor Author

shwstppr commented Dec 20, 2023

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Dec 20, 2023

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

blueorangutan commented Dec 20, 2023

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8106

@weizhouapache
Copy link
Member

weizhouapache commented Dec 20, 2023

@blueorangutan test

@blueorangutan
Copy link

blueorangutan commented Dec 20, 2023

@weizhouapache a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

blueorangutan commented Dec 21, 2023

[SF] Trillian test result (tid-8648)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40634 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8383-t8648-kvm-centos7.zip
Smoke tests completed. 109 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

weizhouapache
weizhouapache approved these changes Dec 21, 2023
View reviewed changes
Copy link
Member

@weizhouapache weizhouapache left a comment

Choose a reason for hiding this comment

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

code lgtm

@shwstppr
smoke test results look perfect. is it ready for review ?

@shwstppr
Copy link
Contributor Author

shwstppr commented Dec 21, 2023

@weizhouapache there seems some issue with GH action run on test cleanup. Checking that

DaanHoogland
DaanHoogland approved these changes Dec 21, 2023
View reviewed changes
Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

lgtm, note that this test is not safe in life environment to begin with, so this does not have an influence to its

@weizhouapache
Copy link
Member

weizhouapache commented Dec 21, 2023 •
edited
Loading

@weizhouapache there seems some issue with GH action run on test cleanup. Checking that

@shwstppr right..
so we should consider "Unsecure" as a valid and good state for kvm hosts.
(ignore this, the state of Unsecure hosts is also Up...)

@shwstppr
fix non-kvm
b4c74d3 
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@shwstppr
Copy link
Contributor Author

shwstppr commented Dec 21, 2023

@weizhouapache I guess Unsecure is just shown in UI and we don't have such a state in API. In API it returns host.details.secured flag. GH action error was related to the zone variable missing. Fixed it.

With this change, I'm testing a fix for failures seen with test_08_migrate_vm or other migration-related test because the target host is in Connecting state,
#8356 (comment)
#8374 (comment)
and more

@weizhouapache
Copy link
Member

weizhouapache commented Dec 21, 2023 •
edited
Loading

@weizhouapache I guess Unsecure is just shown in UI and we don't have such a state in API. In API it returns host.details.secured flag.

yes, right.

GH action error was related to the zone variable missing. Fixed it.

With this change, I'm testing a fix for failures seen with test_08_migrate_vm or other migration-related test because the target host is in Connecting state, #8356 (comment) #8374 (comment) and more

Great !

@shwstppr shwstppr closed this Dec 21, 2023
@shwstppr shwstppr reopened this Dec 21, 2023
@shwstppr
Copy link
Contributor Author

shwstppr commented Dec 21, 2023

Will merge this once GH actions return success

@shwstppr shwstppr marked this pull request as ready for review December 21, 2023 12:39
@shwstppr shwstppr merged commit d83d994 into apache:4.18 Dec 21, 2023
@shwstppr shwstppr deleted the test-addcheck-vmlifecycle branch December 21, 2023 15:26
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Dec 27, 2023
@shwstppr @dhslove
test: additional check to ensure hosts are left in up state (apache#8383
c94af0a 
)

With this change, a fix is added for failures seen with test_08_migrate_vm or other migration-related tests because the target host is in `Connecting` state,
apache#8356 (comment)
apache#8374 (comment)
and more
@DaanHoogland DaanHoogland added this to the 4.18.3 milestone Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DaanHoogland DaanHoogland DaanHoogland approved these changes

@weizhouapache weizhouapache weizhouapache approved these changes

Assignees

No one assigned

Labels

component:integration-test Python Warning... Python code Ahead!

Projects

None yet

Milestone

4.18.3

Development

Successfully merging this pull request may close these issues.

4 participants

@shwstppr @blueorangutan @weizhouapache @DaanHoogland