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

Support extract_as_repo for SLM pxe tar file #278

Conversation

alice-suse
Copy link
Contributor

@alice-suse alice-suse commented Nov 21, 2024

It is requested in https://progress.opensuse.org/issues/167015 to provide SLM pxe resource, which has tar format, and needs to be extracted as repository. This PR provides the capability for that.

Relevant images:

  • SL-Micro.x86_64-6.2-Default-SelfInstall-Build1.1.install.tar
  • SL-Micro.aarch64-6.2-Default-SelfInstall-Build1.1.install.tar

Rsync them to

And untar to

Relevant MR based on this:

@alice-suse alice-suse force-pushed the alice/support-extract_as_repo-for-slm-pxe-tar branch from 0972d98 to d585336 Compare November 21, 2024 04:25
@alice-suse alice-suse force-pushed the alice/support-extract_as_repo-for-slm-pxe-tar branch from d585336 to f92f35c Compare November 21, 2024 04:48
@alice-suse alice-suse changed the title [WIP] Support extract_as_repo for SLM pxe tar file Support extract_as_repo for SLM pxe tar file Nov 21, 2024
@alice-suse
Copy link
Contributor Author

@jlausuch @andrii-suse Welcome to review!

Copy link
Member

@foursixnine foursixnine left a comment

Choose a reason for hiding this comment

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

@rfan1 you were also looking at providing this too right?

@rfan1
Copy link

rfan1 commented Nov 21, 2024

@rfan1 you were also looking at providing this too right?

Yes, but I was looking for the extracted iso images for ppc64le arch only to start the selfinstall tests.

Copy link
Collaborator

@andrii-suse andrii-suse left a comment

Choose a reason for hiding this comment

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

hm, do you have a confirmation from openQA devs that putting .tar to factory/iso is a good idea? If you are sure about that, then looks good to me

@@ -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

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

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

@andrii-suse andrii-suse merged commit c29b827 into os-autoinst:master Nov 25, 2024
5 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.

5 participants