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

Jump to bottom

Install from local DVD if it is present #1372

Merged
merged 8 commits into from
Jun 26, 2024
Merged

Install from local DVD if it is present #1372

merged 8 commits into from
Jun 26, 2024

Conversation

lslezak
Copy link
Contributor

@lslezak lslezak commented Jun 25, 2024 •
edited
Loading

Problem

  • Local DVD cannot be used for installation, offline installation is not possible
  • We have a special branch with this feature and build special ISO but I think it should work out of box with upstream Agama
  • Sooner or later we will need to support installation from the local media anyway

Solution

  • A disk label defines the installation repository name for each product
  • If such disk label is found then that disk is used as the installation repository
  • If no label is found in the system it uses the online repositories
  • If multiple labels are found then the DVD device is preferred

TODO

  • Update unit tests
  • Add more comments
  • Update documentation

Testing

  • Tested manually, when the Tumbleweed DVD is present it is automatically used instead of the online repositories
  • When the product is switched to MicroOS and the DVD is not present it uses the online repositories

@imobachgs
Copy link
Contributor

The approach looks good to me. However, I am not sure if we should stop using the online repositories at all (that's what I understood from the description). What if I have some updates on the online repos that are not present in the DVD?

@lslezak
Copy link
Contributor Author

lslezak commented Jun 25, 2024

  1. For Tumbleweed the updates do not make much sense, the installation repository is always rebuilt as a whole.
  2. If we want to install updates we should explicitly ask the user to do so. There is a popup question in both Tumbleweed and Leap whether to use the online repositories. The same applies also for SLES after registration.

I consider this as the initial support, later we should think about all possible scenarios and decide which ones should be supported. IMHO installing with updates is a different problem and should be solved separately.

@coveralls
Copy link

Coverage Status

coverage: 70.958% (+0.01%) from 70.945%
when pulling 345e5bf on optionally_install_from_2nd_DVD
into 7f641e4 on master.

@imobachgs imobachgs marked this pull request as ready for review June 25, 2024 14:33
@imobachgs imobachgs marked this pull request as draft June 25, 2024 14:33
@imobachgs
Copy link
Contributor

imobachgs commented Jun 25, 2024 •
edited
Loading

  1. Yes, I know, but I think that even in Tumbleweed there is some kind of updates channel for urgent things.
  2. OK, no problem.

For me it is fine to postpone the decision a bit, but please, let's not forget about this (creating a card in Trello should suffice).

@coveralls
Copy link

Coverage Status

coverage: 70.964% (+0.02%) from 70.945%
when pulling b8180a8 on optionally_install_from_2nd_DVD
into 7f641e4 on master.

@lslezak lslezak force-pushed the optionally_install_from_2nd_DVD branch from b8180a8 to 1b94f64 Compare June 25, 2024 15:24
@lslezak lslezak marked this pull request as ready for review June 25, 2024 15:32
imobachgs
imobachgs approved these changes Jun 25, 2024
View reviewed changes
Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

Just one question. Otherwise, LGTM.

service/lib/agama/software/manager.rb Outdated Show resolved Hide resolved
service/lib/agama/software/manager.rb
# @return [Hash] parsed data
def list_disks
# we need only the kernel device name and the label
output = `lsblk --paths --json --output kname,label`
Copy link
Contributor

Choose a reason for hiding this comment

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

lsblk is included in util-linux-systemd. Should we add a dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add that...

service/lib/agama/software/manager.rb
optical || disks.first
else
# none or just one disk
disks.first
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it is a corner case but, what happens if I select the disk for installing the system? Do we need any kind of filtering?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The disk will not be available for partitioning/installation. But if you have more than one disk it should work (one for RPM packages, second as the installation target).

But I guess all these scenarios are rather theoretical and look a bit strange to me...

@imobachgs imobachgs added this to the Agama 9 milestone Jun 25, 2024
lslezak and others added 8 commits June 25, 2024 21:01
@lslezak
Install from local DVD if it is present
d5691a3 
Use the standard DVD installation medium if it is present,
otherwise use the online repositories.
@lslezak
Documentation
1919d5c
@lslezak
Error handling
8b0650f
@lslezak
Rubocop fix
8c754a6
@lslezak
Error handling
72e038d
@lslezak
Update unit tests
68a5a14
@lslezak @imobachgs
Update service/lib/agama/software/manager.rb
205260c 
comment note

Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>
@lslezak
Added util-linux-systemd dependency
b5080e0
@lslezak lslezak force-pushed the optionally_install_from_2nd_DVD branch from f502b4b to b5080e0 Compare June 25, 2024 19:01
@coveralls
Copy link

Coverage Status

coverage: 71.01% (+0.02%) from 70.99%
when pulling b5080e0 on optionally_install_from_2nd_DVD
into ce30ede on master.

@lslezak lslezak merged commit 1a0d1e0 into master Jun 26, 2024
6 checks passed
@lslezak lslezak deleted the optionally_install_from_2nd_DVD branch June 26, 2024 05:41
@imobachgs imobachgs mentioned this pull request Jun 27, 2024
Merged
imobachgs added a commit that referenced this pull request Jun 27, 2024
@imobachgs
Update to release 9 (#1404)
8fe19db 
Prepare for releasing Agama 9. It includes the following pull requests:

- #1101
- #1202
- #1228
- #1231
- #1236
- #1238
- #1239
- #1240
- #1242
- #1243
- #1244
- #1245
- #1246
- #1247
- #1248
- #1249
- #1250
- #1251
- #1252
- #1253
- #1254
- #1255
- #1256
- #1257
- #1258
- #1259
- #1260
- #1261
- #1264
- #1265
- #1267
- #1268
- #1269
- #1270
- #1271
- #1272
- #1273
- #1274
- #1279
- #1280
- #1284
- #1285
- #1286
- #1287
- #1288
- #1289
- #1290
- #1291
- #1292
- #1293
- #1294
- #1295
- #1296
- #1298
- #1299
- #1300
- #1301
- #1302
- #1303
- #1304
- #1305
- #1306
- #1307
- #1308
- #1309
- #1310
- #1311
- #1312
- #1313
- #1314
- #1315
- #1316
- #1317
- #1318
- #1319
- #1320
- #1321
- #1322
- #1323
- #1324
- #1325
- #1326
- #1328
- #1329
- #1331
- #1332
- #1334
- #1338
- #1340
- #1341
- #1342
- #1343
- #1344
- #1345
- #1348
- #1349
- #1351
- #1352
- #1353
- #1354
- #1355
- #1356
- #1357
- #1358
- #1359
- #1360
- #1361
- #1362
- #1363
- #1365
- #1366
- #1367
- #1368
- #1371
- #1372
- #1374
- #1375
- #1376
- #1379
- #1380
- #1381
- #1383
- #1384
- #1385
- #1386
- #1387
- #1388
- #1389
- #1391
- #1392
- #1394
- #1395
- #1397
- #1398
- #1399
- #1400
- #1403
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@imobachgs imobachgs imobachgs approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Agama 9
Development

Successfully merging this pull request may close these issues.
3 participants
@lslezak @imobachgs @coveralls