Skip to content

Conversation

Gmerold
Copy link
Contributor

@Gmerold Gmerold commented Feb 6, 2025

This PR fixes an issue with the first iteration of the SRIOV resources availability check.
Original implementation raises KeyError if the the resources are not available. As a result while is exiting immediately and has no effect.
This simple fix catches the KeyError and allows the loop to continue as expected.

Additionally, the PR fixes the Run tests workflow.

Also verify you have:

Copy link
Contributor

@claudiubelu claudiubelu left a comment

Choose a reason for hiding this comment

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

Hello, I've left a few comments.

@@ -19,7 +19,7 @@ jobs:
run: |
sudo apt-get update
sudo apt-get install -y python3-setuptools nfs-common
sudo pip3 install --upgrade pip
sudo pip3 install --ignore-installed --upgrade pip
Copy link
Contributor

Choose a reason for hiding this comment

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

out of curiosity, what kind of error does this fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, I've seen that error as well. It's probably because pip3 is installed through apt, and upgrading it through pip3 won't work. I've opted to update the apt package instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess this fix doesn't belong to this PR anyway.
BTW, there's another issue in the main killing the tests now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you're referring to the previous run when it failed: https://github.com/canonical/microk8s-community-addons/actions/runs/14036415249/job/39295392373

Seems like there's no issue anymore.

@Gmerold
Copy link
Contributor Author

Gmerold commented Mar 24, 2025

Hi @claudiubelu
Thanks for reviewing that. All comments addressed.

@Gmerold Gmerold requested a review from claudiubelu March 24, 2025 13:26
@Gmerold Gmerold requested a review from claudiubelu March 28, 2025 10:18
Copy link
Contributor

@claudiubelu claudiubelu left a comment

Choose a reason for hiding this comment

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

Thanks for the updates!

@claudiubelu claudiubelu merged commit 660ab27 into canonical:main Mar 28, 2025
3 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.

2 participants