snapd icon indicating copy to clipboard operation
snapd copied to clipboard

tests/core/gadget-update-pc: add mbr/dos variant

Open anonymouse64 opened this issue 2 years ago • 4 comments

Test that we can update gadget assets when using DOS schema as well. This requires some tweaking in prepare.sh, namely to add the schema, but also on UC20 to drop the ubuntu-save partition since on DOS we can only have 4 partitions.

anonymouse64 avatar Mar 24 '22 16:03 anonymouse64

Codecov Report

Merging #11573 (38afb87) into master (6541efe) will decrease coverage by 0.00%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #11573      +/-   ##
==========================================
- Coverage   78.00%   78.00%   -0.01%     
==========================================
  Files         939      939              
  Lines      109526   109526              
==========================================
- Hits        85441    85435       -6     
- Misses      18796    18800       +4     
- Partials     5289     5291       +2     
Flag Coverage Δ
unittests 78.00% <ø> (-0.01%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
overlord/hookstate/hookmgr.go 74.67% <0.00%> (-0.65%) :arrow_down:
overlord/ifacestate/helpers.go 76.05% <0.00%> (-0.47%) :arrow_down:

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

codecov-commenter avatar Mar 24 '22 17:03 codecov-commenter

I'm not sure about hardcoding sda

This could prove to be a problem yes, but I wasn't quite sure what we wanted to do about it but as you say it can be fixed when it proves to be a problem :slightly_smiling_face:

anonymouse64 avatar Mar 25 '22 13:03 anonymouse64

I'm a bit confused, all the UC16 systems on this branch fail to be prepared with this:

+ /snap/bin/ubuntu-image snap -w /home/image /home/image/pc.model --channel edge --snap /home/image/core_16-2.55.2+git3709.84f473c15_amd64.snap --output-dir /home/image
WARNING: proceeding to download snaps ignoring validations, this default will change in the future. For now use --validation=enforce for validations to be taken into account, pass instead --validation=ignore to preserve current behavior going forward
Fetching pc-kernel
Fetching pc
WARNING: "core" installed from local snaps disconnected from a store cannot be refreshed subsequently!
Copying "/home/image/core_16-2.55.2+git3709.84f473c15_amd64.snap" (core)
WARNING: volumes:pc:structure:2:filesystem_label used for defining partition roles; use role instead
Error: Error running mkfs with content: 
-----
mkfs.ext4: invalid option -- 'd'
Usage: mkfs.ext4 [-c|-l filename] [-b block-size] [-C cluster-size]
	[-i bytes-per-inode] [-I inode-size] [-J journal-options]
	[-G flex-group-size] [-N number-of-inodes]
	[-m reserved-blocks-percentage] [-o creator-os]
	[-g blocks-per-group] [-L volume-label] [-M last-mounted-directory]
	[-O feature[,...]] [-r fs-revision] [-E extended-option[,...]]
	[-t fs-type] [-T usage-type ] [-U UUID] [-jnqvDFKSV] device [blocks-count]

It's not clear to me if this is a bug with ubuntu-image or if there was a regression with mkfs.ext4 in xenial or if somehow the changes here in this PR are actually somehow related. However I don't think its the latter because the changes to preparing the system should only be taking place on one particular system, but actually as I type this I'm realizing that this is now making all tests which happen to run on the same UC16 system as gadget-update-pc:mbr test also in effect test the MBR UC16 system, which maybe is a feature but could affect other tests which rely on using GPT features in the tests.

So I'm actually not sure what should be done with this PR. Perhaps this should be scrapped and a nested test should be used instead to ensure that only one system is booted exactly with the MBR changes, but that is unfortunate to have to add ever more tests to the nested suite :-/

anonymouse64 avatar Mar 25 '22 13:03 anonymouse64

I'm a bit confused, all the UC16 systems on this branch fail to be prepared with this: [...]

The issue was unrelated to this branch, and I've seen it happening on other branches as well. Merging the master branch fixed it, so I think that this PR is good to go.

mardy avatar Mar 29 '22 12:03 mardy