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

Generate all Agama json profiles using jsonnet profiles #21108

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
15 changes: 15 additions & 0 deletions data/yam/agama/auto/lib/base.libsonnet
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
bootloader: {
stopOnBootMenu: true
},
user: {
fullName: 'Bernhard M. Wiedemann',
password: '$6$vYbbuJ9WMriFxGHY$gQ7shLw9ZBsRcPgo6/8KmfDvQ/lCqxW8/WnMoLCoWGdHO6Touush1nhegYfdBbXRpsQuy/FTZZeg7gQL50IbA/',
hashedPassword: true,
userName: 'bernhard'
},
root: {
password: '$6$vYbbuJ9WMriFxGHY$gQ7shLw9ZBsRcPgo6/8KmfDvQ/lCqxW8/WnMoLCoWGdHO6Touush1nhegYfdBbXRpsQuy/FTZZeg7gQL50IbA/',
hashedPassword: true
}
}
32 changes: 32 additions & 0 deletions data/yam/agama/auto/lib/scripts.libsonnet
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

I find really unlikely that we use a pre script also as post script, wouldn't make more sent to separate them in different files, so the user of this feature can see clearly the catalog for each.

enable_root_login: {
name: 'enable root login',
chroot: true,
body: |||
#!/usr/bin/env bash
echo 'PermitRootLogin yes' > /etc/ssh/sshd_config.d/root.conf
|||
},
multipath: {
Copy link
Contributor

Choose a reason for hiding this comment

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

in case we do some more multipath actions, better "activate_multipath".

name: 'activate multipath',
body: |||
#!/bin/bash
if ! systemctl status multpathd ; then
echo 'Activating multipath'
systemctl start multipathd.socket
systemctl start multipathd
fi
|||
},
wipe_filesystem: {
name: 'wipefs',
body: |||
#!/usr/bin/env bash
for i in `lsblk -n -l -o NAME -d -e 7,11,254`
do wipefs -af /dev/$i
sleep 1
sync
done
|||
},
}
45 changes: 45 additions & 0 deletions data/yam/agama/auto/lib/storage.libsonnet
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
local get_root_filesystem(filesystem='ext4') = {
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't be any default for this, looks to me like some external variable.

drives: [
{
partitions: [
{ search: "*", delete: true },
{ generate: 'default' },
{ filesystem: { path: '/', type: filesystem } },
],
},
],
};

{
lvm(encrypted=false): {
drives: [
{
alias: 'pvs-disk',
partitions: [
{ search: "*", delete: true }
]
},
],
volumeGroups: [
{
name: 'system',
physicalVolumes: [
{
[if encrypted == true then 'generate']: {
targetDevices: ['pvs-disk'],
encryption: {
luks2: { password: "nots3cr3t" }
}
},
[if encrypted == false then 'generate']: ['pvs-disk'],
},
],
logicalVolumes: [
{ generate: 'default' },
],
},
]
},
root_filesystem_ext4: get_root_filesystem('ext4'),
root_filesystem_xfs: get_root_filesystem('xfs'),
}
20 changes: 20 additions & 0 deletions data/yam/agama/auto/template.jsonnet
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
local base_lib = import 'lib/base.libsonnet';
local scripts = import 'lib/scripts.libsonnet';
local storage = import 'lib/storage.libsonnet';

function(storage_schema='', product='SLES', pre_scripts='',
Copy link
Contributor

Choose a reason for hiding this comment

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

let's not invent words here, the only schema that we have is the one in the Agama repo against which the profile is validated, this should just be storage, we should make all the libraries looks like so, so above local storage_lib same for any other library.

Copy link
Contributor

Choose a reason for hiding this comment

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

for the same reason to make it flexible for openSUSE, product=''

post_scripts='', encrypted=false) {
Copy link
Contributor

@jknphy jknphy Feb 5, 2025

Choose a reason for hiding this comment

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

perhaps this could be scripts_pre so we don't need to invent names in the future, but to follow from outer to inner the structure and use that always as parameter names.

product: {
id: product
},
user: base_lib['user'],
root: base_lib['root'],
[if pre_scripts != '' || post_scripts != '' then 'scripts']: {
[if pre_scripts != '' then 'pre']: [ scripts[x] for x in std.split(pre_scripts, ',') ],
[if post_scripts != '' then 'post']: [ scripts[x] for x in std.split(post_scripts, ',') ],
},

[if storage_schema == 'lvm' then 'storage']: storage.lvm(encrypted),
[if storage_schema == 'root_filesystem_ext4' then 'storage']: storage['root_filesystem_ext4'],
[if storage_schema == 'root_filesystem_xfs' then 'storage']: storage['root_filesystem_xfs'],
}
68 changes: 68 additions & 0 deletions lib/autoyast.pm
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
=head1 autoyast

Check failure on line 1 in lib/autoyast.pm

View workflow job for this annotation

GitHub Actions / CI: Running static tests with perl v5.32

File lib/autoyast.pm needs tidying

Provide translations for autoyast XML file

Expand Down Expand Up @@ -41,6 +41,7 @@
expand_variables
adjust_user_password
upload_profile
generate_json_profile
inject_registration
init_autoyast_profile
test_ayp_url
Expand Down Expand Up @@ -783,6 +784,73 @@
return $profile_url;
}

=head2 workaround_install_jsonnet

workaround_install_jsonnet();

Workaround Install via zypper golang-github-google-jsonnet package in the worker.
We need this meanwhile the package is built for all other architectures (s390x/aarch64/ppc64le).
After that it should be in the Salt repository.

=cut

sub workaround_install_jsonnet {
if (system("sudo", "zypper", "-n", "repos", "systemsmanagement_Agama_Devel") != 0) {
system("sudo", "zypper", "-n", "addrepo", "-f", "-G",
"https://download.opensuse.org/repositories/systemsmanagement:Agama:Devel/15.6/systemsmanagement:Agama:Devel.repo");
}

if (system("command", "-v", "jsonnet") != 0) {
system("sudo", "zypper", "-n", "install", "-f", "golang-github-google-jsonnet");
}
}

=head2 parse_profile_options

parse_profile_options(options => $options);

Parse given options to match required parameters syntax of jsonnet.

=cut

sub parse_profile_options {
my ($options) = @_;

my @params = split(' ', trim($options));
for my $element (@params) {
if ($element =~ /true/ || $element =~/false/) {
$element = " --tla-code " . $element
} else {
$element = " --tla-str " . $element
}
}
return @params
Copy link
Contributor

@jknphy jknphy Feb 5, 2025

Choose a reason for hiding this comment

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

try to avoid loops at all cost and use higher-order functions, Perl provides grep, map, the code is much more concise, something like this:
return map { " --tla-" . (/true|false/ ? "code" : "str") . " $_" } split ' ', trim($options);
You could even avoid to create a function and just include it below.

}

=head2 generate_json_profile

generate_json_profile();

Return the URL of generated JSON profile

=cut

sub generate_json_profile {
my $profile_name = "generated_profile.json";
my $profile_options = parse_profile_options(get_var('AGAMA_PROFILE_OPTIONS'));
Copy link
Contributor

Choose a reason for hiding this comment

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

the lines which contains some logic should go below, the casedir is just an assignation and should go here instead.

my $casedir = get_required_var('CASEDIR');

workaround_install_jsonnet();
my $profile_content = `jsonnet $profile_options $casedir/data/yam/agama/auto/template.jsonnet`;
die $profile_content;

save_tmp_file($profile_name, $profile_content);

Copy link
Contributor

Choose a reason for hiding this comment

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

this extra line is not needed, the whole thing could be logic block, kind of blocks (1) preparation (2) execution (3) post-stuff and no need for comment in each block because it is obvious that is the structure.

my $profile_url = autoinst_url . "/files/$profile_name";
upload_profile(path => $profile_name, profile => $profile_content);
Copy link
Contributor

@jknphy jknphy Feb 4, 2025

Choose a reason for hiding this comment

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

in case of failure we might not have the uploaded json file, so I think we should also diag for example the generated json file to display it in the screen, otherwise could be hard to dissect.

return $profile_url;
}

=head2 upload_profile

upload_profile(profile => $profile, path => $path)
Expand Down
5 changes: 4 additions & 1 deletion tests/yam/agama/boot_agama.pm
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use strict;
use warnings;

use testapi;
use autoyast qw(expand_agama_profile);
use autoyast qw(expand_agama_profile generate_json_profile);
use Utils::Architectures;
use Utils::Backends;

Expand Down Expand Up @@ -58,6 +58,9 @@ sub run {
my $path = expand_agama_profile($agama_auto);
set_var('AGAMA_AUTO', $path);
set_var('EXTRABOOTPARAMS', get_var('EXTRABOOTPARAMS', '') . " agama.auto=\"$path\"");
} else {
my $profile_url = generate_json_profile();
set_var('EXTRABOOTPARAMS', get_var('EXTRABOOTPARAMS', '') . " agama.auto=\"$profile_url\"");
Copy link
Contributor

@jknphy jknphy Feb 5, 2025

Choose a reason for hiding this comment

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

I don't understand when we use it, I would expect something like:

if (agama_auto && x86)
   create json profile
else
   existing code (expand profile)

otherwise existing change would run for interactive installation.
we need to point in AGAMA_AUTO to the template yam/agama/auto/template.jsonnet

}
my @params = split ' ', trim(get_var('EXTRABOOTPARAMS', ''));

Expand Down
Loading