Skip to content

Conversation

stephenfin
Copy link
Contributor

Go runs tests in parallel for different packages in parallel. The service update test was disabling and re-enabling all compute services, and if this ran at the same time as another test that created a server then the server creation (and therefore the test) would fail. There does not appear to be any way to resolve this. We could run all integration tests serially instead, but this would be a backwards steps that will massively increase test run time. We could also rewrite the test to only disable 1 of N nova-compute services, but that would require us to fence off this compute node in all other tests using non-default parameters to server create. Given the lack of better options, we simply remove the test and assume our testing to date on this method has been good enough and unit tests will be enough to carry us forward.

Signed-off-by: Stephen Finucane stephenfin@redhat.com

Go runs tests in parallel for different packages in parallel. The
service update test was disabling and re-enabling all compute services,
and if this ran at the same time as another test that created a server
then the server creation (and therefore the test) would fail. There does
not appear to be any way to resolve this. We could run all integration
tests serially instead, but this would be a backwards steps that will
massively increase test run time. We could also rewrite the test to only
disable 1 of N nova-compute services, but that would require us to fence
off this compute node in all other tests using non-default parameters to
server create. Given the lack of better options, we simply remove the
test and assume our testing to date on this method has been good enough
and unit tests will be enough to carry us forward.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
@github-actions github-actions bot added the semver:patch No API change label Jul 17, 2024
@coveralls
Copy link

Coverage Status

coverage: 78.747%. remained the same
when pulling a7f6d08 on skip-service-update-tests
into da87790 on master.

Copy link
Contributor

@EmilienM EmilienM left a comment

Choose a reason for hiding this comment

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

I'm fine with it

@pierreprinetti pierreprinetti added hold Do not merge and removed hold Do not merge labels Jul 17, 2024
@pierreprinetti
Copy link
Member

I wanted to test the new "hold" label, but Github is excruciatingly slow at running jobs today...

@pierreprinetti pierreprinetti added good first issue A good issue for first-time contributors and removed good first issue A good issue for first-time contributors labels Jul 17, 2024
@pierreprinetti pierreprinetti merged commit 9a4f157 into master Jul 17, 2024
@pierreprinetti pierreprinetti deleted the skip-service-update-tests branch July 17, 2024 14:51
@stephenfin
Copy link
Contributor Author

Got another example of this here #3120

@pierreprinetti
Copy link
Member

@stephenfin Do we need a backport here=

@mandre
Copy link
Contributor

mandre commented Jul 17, 2024

As I stated in #3120, I'm not sure the root of the problem is parallelization. Why does it only fail on master? To me this looks caused by a change in openstack, for example where service update was previously blocking and it's no longer the case.

@mandre
Copy link
Contributor

mandre commented Jul 17, 2024

According to our CI history, it started failing between May 22th and May 25th.

@stephenfin
Copy link
Contributor Author

According to our CI history, it started failing between May 22th and May 25th.

Nothing stands out for me from the two weeks prior to this.

❯ git log --since "2024-05-12" --until "2024-05-26" --oneline                                                                                                                                                                                                                                                                                                                                                                               
bded279a00 Merge "docs: Add more information about unified limits"
c7e49dfa16 docs: Follow up for persistent mdevs
d7d2fb1edd Merge "scheduler: AggregateMultitenancyIsolation to support unlimited tenant"
4e3a41f0a4 Merge "Stop using split UEC image (mostly)"
3a53d715cd Merge "[doc] Improve description for nova-manage db purge"
ac8729ac87 [doc] Improve description for nova-manage db purge
4bc5ff1c99 Merge "fix py312 tox definitions"
ee581a5c9d add functional repoducer for bug 2065927
c60b81fa4b Merge "tox: Drop envdir"
7ff24958ee fix py312 tox definitions
61ad4f1f27 Merge "Enable virtio-scsi in nova-next"
eed3e2b47f Stop using split UEC image (mostly)
84b0a481fe Enable OCaaS for several nova jobs
d45379c6b4 docs: Add more information about unified limits
ab3ca1e205 Merge "Make python 3.12 unit and functional voting"
b6a846c6c8 Merge "Fix hacking test with syntax error"
fa44978141 Merge "Fix notification object hashes for python 3.12"
bfd3525863 Fix formatting issues in extra-specs docs
d467edac49 Merge "do not use str(url) to stringify a URL for subsequent use"
50b180023f Make python 3.12 unit and functional voting
3f0879ccc3 Fix hacking test with syntax error
6ee938fd22 Fix notification object hashes for python 3.12
2e6041a76e Merge "Remove SQLAlchemy tips jobs"
acbe3e28e5 do not use str(url) to stringify a URL for subsequent use
5095336689 Merge "Upload glance image with --file in ceph job"

@pierreprinetti pierreprinetti added the backport-v2 This PR will be backported to v2 label Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v2 This PR will be backported to v2 semver:patch No API change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants