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.

Sign up for GitHub

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

Support extract_as_repo for SLM pxe tar file #278

Merged
merged 1 commit into from
Nov 25, 2024
Merged

Support extract_as_repo for SLM pxe tar file #278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions script/cfg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,11 @@ def rsync_commands(checksum):
asset_folder=other
[[ ! $dest =~ \.iso$ ]] || asset_folder=iso
[[ ! $dest =~ \.spdx\.json$ ]] || asset_folder=iso
[[ ! $dest =~ \.tar$ ]] || asset_folder=iso
Copy link
Member

@foursixnine foursixnine Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@andrii-suse looking at your comment should this be

Suggested change
[[ ! $dest =~ \.tar$ ]] || asset_folder=iso
[[ ! $dest =~ \.tar$ ]] || asset_folder=other

and deleted afterwards?

Copy link
Contributor Author

@alice-suse alice-suse Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

That's the original default logic, putting it to other(default option set on previous line), but it brings problem elsewhere. Originally the code only supports extract iso into repo, so for anywhere related with such action, it assumes the image is put in iso/.... If changing it, it means a lot of work to dig into other code parts and fix problems... Given that there is already [[ ! $dest =~ \.spdx\.json$ ]] || asset_folder=iso, I think it will be easier to just putting tar into iso folder.

I also thought about another way to implement this, like completely support any other format that needs to be extracted as repo, just like iso, but it will require too much effort from top(parsing settings) to down(generating scripts), which is not proper for me who is just freshman to this tool. So I chose the lower effort solution, like pretending to be "iso"...

If you guys think this is acceptable, I am happy to see it merged. Otherwise, I am fine to close the PR and maybe let other expert guys on the tool to implement the function in a more comprehensive way.

Copy link
Member

Choose a reason for hiding this comment

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

I think it will be easier to just putting tar into iso folder.

Makes sense

which is not proper for me who is just freshman to this tool.

by now I consider you an expert xD - I barely understand how this all works 📦

@andrii-suse do you know if its possible to delete an asset or could you provide proposal on to of Alice's change, so that the .tar file is removed after extraction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ISO files are kept in iso folder after extraction. I think the same is better to be applied to tar files -- just to be synced in behaviors. HDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you guys think this is acceptable, I am happy to see it merged.

Yeah, the questions are whether it will work as planned and whether it can bring any problems.

Otherwise, I am fine to close the PR

That is nobody's goal here. You have already made decent work here, let's focus on make it through if that is really needed.
I will investigate a bit more and get back to this.

Copy link
Member

Choose a reason for hiding this comment

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

ISO files are kept in iso folder after extraction. I think the same is better to be applied to tar files -- just to be synced in behaviors. HDYT?

I'm fine :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you guys think this is acceptable, I am happy to see it merged.

Yeah, the questions are whether it will work as planned and whether it can bring any problems.

Good point!

I believe you can get some useful information from this MR :

You will see how it works for SLM tar image. And after full script re-generation, it does not affect other project directories' *.before. However, for the areas which are not touched in xml/bs/, we have to rely on you experts to tell :).

Copy link
Contributor

Choose a reason for hiding this comment

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

I was asked about how well openQA handles .tar assets as ISO:

  • It looks like openQA uses the asset extension in some places to determine the asset type. That will break in cases the asset type is important it might not behave as required.
  • The cache service might also not consider the asset judging by its code which filters assets by their extensions (my $assets = $tree->map('to_string')->grep(qr/\.(?:img|qcow2|iso|vhd|vhdx)$/);).
  • Specifying a tar archive via the ISO variable is also problematic because os-autoinst backends will handle it like a cd/dvd image. That will probably not work and might lead to errors.

I'd say you can try it nevertheless but don't be surprised if it doesn't work on some level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the input. I can imagine these...

Good thing is that, in our test(SLM baremetal pxe based installation test), we will only need the REPO_0/ MIRROR_HTTP which refers to the repo http link and we do not need ISO at all.

See the expected openqa cli in https://gitlab.suse.de/openqa/openqa-trigger-from-ibs-plugin/-/merge_requests/213/diffs#0dc642f7285a06067e3783580e994758e243f934:

/usr/bin/openqa-cli api -X post isos?async=1 \
 ARCH=x86_64 \
 ASSET_256=SL-Micro.x86_64-6.2-Default-SelfInstall-Build1.1.install.tar.sha256 \
 BUILD=1.1 \
 CHECKSUM_ISO=$(cut -b-64 /var/lib/openqa/factory/other/SL-Micro.x86_64-6.2-Default-SelfInstall-Build1.1.install.tar.sha256 | grep -E '[0-9a-f]{5,40}' | head -n1) \
 DISTRI=sle-micro \
 FLAVOR=Default-SelfInstall-pxe \
 ISO=SL-Micro.x86_64-6.2-Default-SelfInstall-Build1.1.install.tar \
 MIRROR_FTP=ftp://openqa.suse.de/SL-Micro.x86_64-6.2-Default-SelfInstall-Build1.1.install \
 MIRROR_HTTP=http://openqa.suse.de/assets/repo/SL-Micro.x86_64-6.2-Default-SelfInstall-Build1.1.install \
 MIRROR_HTTPS=https://openqa.suse.de/assets/repo/SL-Micro.x86_64-6.2-Default-SelfInstall-Build1.1.install \
 MIRROR_NFS=nfs://openqa.suse.de/var/lib/openqa/share/factory/repo/SL-Micro.x86_64-6.2-Default-SelfInstall-Build1.1.install \
 MIRROR_SMB=smb://openqa.suse.de/inst/SL-Micro.x86_64-6.2-Default-SelfInstall-Build1.1.install \
 REPO_0=SL-Micro.x86_64-6.2-Default-SelfInstall-Build1.1.install \
 VERSION=6.2 \
 _DEPRIORITIZEBUILD=1

[[ ! $dest =~ \.(qcow2|raw|vhd|vmdk|vhdx|xz)$ ]] || asset_folder=hdd
''' + rsync_commands(checksum) + '''
repo0folder=${dest%.iso}
repo0folder=${repo0folder%.tar}
''' + (repo0folder if repo0folder else "") + '''
[ -z "FLAVORASREPOORS" ] || [ $( echo "$flavor" | grep -E -c "^(FLAVORASREPOORS)$" ) -eq 0 ] || echo "[ -d /var/lib/openqa/factory/repo/$repo0folder ] || {
mkdir /var/lib/openqa/factory/repo/$repo0folder
Expand Down Expand Up @@ -250,6 +252,7 @@ def pre_openqa_call_start(repos):
[ -z "__STAGING" ] || build1=__STAGING.$build
''' + openqa_call_fix_destiso(distri, version, staging) + '''
repo0folder=${destiso%.iso}
repo0folder=${repo0folder%.tar}
''' + (repo0folder if repo0folder else "") + '''
[ "$arch" != . ] || arch=x86_64
''' + openqa_call_news(news, news_archs) + '''
Expand Down Expand Up @@ -290,6 +293,8 @@ def openqa_call_start_ex(checksum):
echo \" ''' + openqa_call_start_ex1(checksum, 'ISO') + '''\"
elif [[ $destiso =~ \.spdx.json$ ]]; then
echo \" ''' + openqa_call_start_ex1(checksum, 'ISO') + '''\"
elif [[ $destiso =~ \.tar$ ]]; then
echo \" ''' + openqa_call_start_ex1(checksum, 'ISO') + '''\"
elif [[ $destiso =~ \.(hdd|qcow2|raw|raw\.xz|raw\.gz|vhdx\.xz|vmdk|vmdk\.xz)$ ]]; then
echo \" ''' + openqa_call_start_ex1(checksum, 'HDD_1') + '''\"
elif [ -n "$destiso" ]; then
Expand Down
2 changes: 2 additions & 0 deletions script/scriptgen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,8 @@ def gen_read_files(self, f):
"",
"Media1?.iso$",
iso,
"(Media1?|install.*).iso$",
iso,
)
elif self.media1 != "0":
self.p(cfg.read_files_iso, f, "FOLDER", self.iso_folder.get(iso, ""), "SRCISO", iso)
Expand Down
Loading