Closed gboutry closed 1 year ago
Merging #52 (4957c8e) into main (a2f8d01) will increase coverage by
1.48%
. The diff coverage is88.46%
.
@@ Coverage Diff @@
## main #52 +/- ##
==========================================
+ Coverage 82.48% 83.96% +1.48%
==========================================
Files 12 13 +1
Lines 685 786 +101
Branches 157 182 +25
==========================================
+ Hits 565 660 +95
- Misses 88 89 +1
- Partials 32 37 +5
Impacted Files | Coverage Δ | |
---|---|---|
craft_archives/repo/package_repository.py | 80.55% <78.57%> (-0.57%) |
:arrow_down: |
craft_archives/repo/apt_key_manager.py | 98.14% <100.00%> (+0.03%) |
:arrow_up: |
craft_archives/repo/apt_sources_manager.py | 93.75% <100.00%> (+0.72%) |
:arrow_up: |
craft_archives/repo/apt_uca.py | 100.00% <100.00%> (ø) |
|
craft_archives/repo/errors.py | 83.78% <100.00%> (+1.43%) |
:arrow_up: |
craft_archives/repo/installer.py | 52.63% <100.00%> (+2.63%) |
:arrow_up: |
craft_archives/repo/projects.py | 94.36% <100.00%> (+11.60%) |
:arrow_up: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
In last push:
ignore_keyring_path
RELEASE_MAP
, the compatibility is not checked with a locally maintained dict anymore, we fetch the resource on the UCA. Downside: if the UCA is down, the check will fail (which is not really an issue, because if the UCA is down but the check passes, it's the pulling that will fail later)There's one last thing, currently I template the sources file with snapcraft-cloud-{cloud}
. Which makes possible having multiple UCA enabled in the same Rock which I don't believe is a pattern we should allow, moreover, updating the repositories with another version would just add a sources list, and there would be 2 sources locally (until a clean).
Seems like there's a bug on the Rockcraft side where the pull stages are not re-triggered after an update on the repositories ? will look more into it on monday
Thanks for the work! Before I get on the nitty-gritty of the code review I think we should discuss the ubuntu-cloud-keyring
package thing. There are two issues with that:
1) This extra dependency on an archive package means calling apt install
"out of band" from the rest of the code; Specifically, as far as I can tell currently only craft-parts
manipulates the repository so we would have to check the consequences of this change;
2) More importantly (and I think this is what @sergiusens was referring to), this ubuntu-cloud-keyring
installs the keyring in /etc/apt/trusted.gpg.d/, which is the behavior we moved away from; keyrings are installed in /etc/apt/keyrings
and then referenced specifically in the repo's source definition.
I would very much like to have the cloud-archive repo be consistent with the other repo types (key in /etc/apt/keyrings, referenced by source.list file in /etc/apt/sources.list.d/). I think we could support that if the keyring is available in keyserver.ubuntu.com, @gboutry do you know if the key is there?
Yes, it looks like the key is present at http://keyserver.ubuntu.com/pks/lookup?search=391A9AA2147192839E9DB0315EDB1B62EC4926EA&fingerprint=on&op=index
In last fp:
This is very nice! I think the only thing left is, for sanity's sake, to test this in rockcraft itself (via a spread test) I can do it for you but I'll need a valid
cloud
repo and the name of a package it contains; do you have these handy? Thanks again!
Thanks @tigarmo, of course.
In antelope
, you will be able to find:
keystone
at 23.0.0-0ubuntu1~cloud0
nova-compute
at 27.0.0-0ubuntu1~cloud0
In zed
, you will be able to find:
keystone
at 22.0.0-0ubuntu1~cloud0
nova-compute
at 26.1.0-0ubuntu1~cloud0
Note that antelope
and zed
are the only cloud archive compatible with jammy
at the moment. If you want to add tests for focal
for example, you will need to test with either yoga
, xena
, wallaby
or victoria
Soon there will be bobcat
available for jammy
, but the code shouldn't need an update to support it!
Thanks! Here's a test case that failed in rockcraft but is missing in craft-archives (missing from test_projects.py
):
def test_validate_repository_empty_dict():
with pytest.raises(pydantic.ValidationError):
validate_repository({})
Thanks! Here's a test case that failed in rockcraft but is missing in craft-archives (missing from
test_projects.py
):def test_validate_repository_empty_dict(): with pytest.raises(pydantic.ValidationError): validate_repository({})
Indeed, there was an issue in the control flow. I fixed it and added this test case!
tox
?lint-pyright
fails, no logs about whyExample of rockcraft.yaml: