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

Fix IC address set usage. #183

Merged
merged 1 commit into from
Oct 31, 2023
Merged

Fix IC address set usage. #183

merged 1 commit into from
Oct 31, 2023

Conversation

dceara
Copy link
Collaborator

@dceara dceara commented Oct 22, 2023

No description provided.

@dceara
Copy link
Collaborator Author

dceara commented Oct 22, 2023

CC: @LorenzoBianconi @igsilya

.cirrus.yml Outdated
Comment on lines 75 to 76
- 'for ip4 in True False; do
for ip6 in True False; do
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we create a matrix instead of putting 3 different tests into one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It took some effort and it has some downsides but I pushed it to the PR as it is cleaner. One thing we might have to adjust is the matrix size. Especially with the thought in mind that we'll be adding OpenStack scenarios too.

.cirrus.yml Outdated
runtime_cache:
folder: runtime-cache
fingerprint_script:
- echo ${CIRRUS_BUILD_ID}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a little concerning. We could consider going back to a simple loop, but we could also consider having a separate global and local to the job caches to save CPU cycles.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll remove the commit that increases CI coverage from this PR. We need the address set fix sooner rather than later. I'll open an issue to track adding global and local to the job caches to save CPU cycles.

.cirrus.yml Outdated
Comment on lines 91 to 92
memory: 8G
disk: 40
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the resource usage, we could, likely reduce the cpu count from the default 2 to 1.
We probably don't need 8G of RAM as well, but I'm not sure why ubuntu builds eat almost 4, while fedora builds only use ~500 MB.

Namespaces can (in theory) have different address family address sets in
different IC clusters.  Instead of checking the address set array
(always not None), check to see if the corresponding per-AZ address set
actually exists.

Fixes: 2562afb ("ovn-tester: extend density_light testing for ovn-ic")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
@dceara dceara changed the title Fix IC address set usage and enable missing CI combinations. Fix IC address set usage. Oct 31, 2023
@dceara dceara merged commit 3e8cb1d into ovn-org:main Oct 31, 2023
7 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.

3 participants