feat(storage): configure boot #1808

joseivanlopez · 2024-12-04T19:17:40Z

Add support in the config model for configuring boot.
The Configure boot option in the More options menu goes to the page for configuring boot.
Adapt the page for configuring boot to use the new config model hooks.

- Allows to indicate whether the device is the default one.

- ProposalSettings cannot use Configs::Boot.

coveralls · 2024-12-19T12:59:31Z

Pull Request Test Coverage Report for Build 12432686864

Details

56 of 60 (93.33%) changed or added relevant lines in 15 files are covered.
7 unchanged lines in 3 files lost coverage.
Overall coverage remained the same at 71.967%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
service/lib/agama/storage/config_conversions/from_model_conversions.rb	0	2	0.0%
service/lib/agama/storage/config_conversions/to_model_conversions.rb	0	2	0.0%

Files with Coverage Reduction	New Missed Lines	%
service/lib/agama/storage/config_conversions/from_model_conversions.rb	2	0.0%
service/lib/agama/storage/config_conversions/to_model_conversions.rb	2	0.0%
service/lib/agama/storage/proposal.rb	3	97.58%

Totals
Change from base Build 12409550989:	0.0%
Covered Lines:	17942
Relevant Lines:	24931

💛 - Coveralls

service/lib/agama/storage/config_checkers/boot.rb

service/lib/agama/storage/configs/with_alias.rb

If the space policy is not provided by the model: - Use 'keep' for a drive added for booting. - Use the default space policy defined by the product in other cases.

ancorgs · 2024-12-20T09:36:19Z

web/src/queries/storage/config-model.ts

+function setBoot(originalModel: configModel.Config, boot: configModel.Boot) {
+  const model = copyModel(originalModel);
+  const name = model.boot?.device?.name;
+  const remove = name !== undefined && isExplicitBoot(model, name) && !isUsedDrive(model, name);


Just to check I understood it correctly (I'm not against it). When a drive is not longer necessary after a change at the boot device, that drive is removed here. On the other hand, when a new drive needs to get added because a change at the boot device, that drive is added at the backend (during the conversion from the JSON model to config).

Right?

Yes. The backend takes care of adding drives if needed. Right now it only has the boot device use case, but maybe we have more in the future (raid?). On the other hand, removing unnecessary drives is something the client should do. The client can decide what is not needed anymore, but the backend has not enough info to decide by itself. For example, if the client adds a new disk for installation, then the backend should not remove it.

ancorgs · 2024-12-20T09:53:31Z

web/src/components/storage/BootSelection.tsx

    setState({ ...state, bootDevice: v });
  };

  return (
    <Page>
      <Page.Header>
-        <h2>{_("Select booting partition")}</h2>
+        <h2>{_("Boot options")}</h2>


A question for @dgdavid

When we navigate to page for defining a custom policy to find space, the title and subtitles are

Find space in /dev/sda
Select what to do with each partition bla bla

When we navigate to the boot options, they are:

Boot options
To ensure the new system is able to boot, the installer may need to create bla bla

Both are fine to me. But I wonder whether they should follow a more similar schema. I mean, the first one is explicitly exhorting the user to do something while the other is explaining where we are and what it means.

Good point!

Anyway, this is not a blocker for merging. Let's do a demo at the next review meeting that discuss those details afterwards.

I'm more for going to the second approach: let the user know where they are and what can do there.

That said, unless I have not strong opinion right now, it looks perfectly fine to me to now follow/force an schema just because. If it is better to use a different approach in a very specific page, why not? It will be better than providing nor enough informatio.

In any case, I fully agree with you on not blocking the PR for this and letting titles evolve with the rest of development. Do not forget that most probably Agama will end up getting breadcrumbs at some point, which can make us to rethink these texts.

service/lib/agama/storage/config_checkers/boot.rb

ancorgs

LGTM

Co-authored-by: Ancor Gonzalez Sosa <[email protected]>

Update to release version 11. * #1495 * #1564 * #1617 * #1618 * #1625 * #1626 * #1627 * #1628 * #1630 * #1631 * #1632 * #1633 * #1634 * #1635 * #1636 * #1639 * #1640 * #1641 * #1642 * #1643 * #1644 * #1645 * #1646 * #1647 * #1648 * #1649 * #1650 * #1651 * #1652 * #1654 * #1655 * #1656 * #1657 * #1660 * #1663 * #1666 * #1667 * #1668 * #1670 * #1671 * #1673 * #1674 * #1675 * #1676 * #1677 * #1681 * #1682 * #1683 * #1684 * #1687 * #1688 * #1689 * #1690 * #1691 * #1692 * #1693 * #1694 * #1695 * #1696 * #1698 * #1699 * #1702 * #1703 * #1704 * #1705 * #1707 * #1708 * #1709 * #1710 * #1711 * #1712 * #1713 * #1714 * #1715 * #1716 * #1717 * #1718 * #1720 * #1721 * #1722 * #1723 * #1727 * #1728 * #1729 * #1731 * #1732 * #1733 * #1734 * #1735 * #1736 * #1737 * #1740 * #1741 * #1743 * #1744 * #1745 * #1746 * #1751 * #1753 * #1754 * #1755 * #1757 * #1762 * #1763 * #1764 * #1765 * #1766 * #1767 * #1769 * #1771 * #1772 * #1773 * #1774 * #1777 * #1778 * #1785 * #1786 * #1787 * #1788 * #1789 * #1790 * #1791 * #1792 * #1793 * #1794 * #1795 * #1796 * #1797 * #1798 * #1799 * #1800 * #1802 * #1803 * #1804 * #1805 * #1807 * #1808 * #1809 * #1810 * #1811 * #1812 * #1814 * #1815 * #1821 * #1822 * #1823 * #1824 * #1825 * #1826 * #1827 * #1828 * #1830 * #1831 * #1832 * #1833 * #1834 * #1835 * #1836 * #1837 * #1838 * #1839 * #1840 * #1841 * #1842 * #1843 * #1844 * #1845 * #1847 * #1848 * #1849 * #1850 * #1851 * #1854 * #1855 * #1856 * #1857 * #1860 * #1861 * #1863 * #1864 * #1865 * #1866 * #1867 * #1871 * #1872 * #1873 * #1875 * #1876 * #1877 * #1878 * #1880 * #1881 * #1882 * #1883 * #1884 * #1885 * #1886 * #1888 * #1889 * #1890

joseivanlopez changed the title ~~Storage boot model~~ feat(storage): model boot config Dec 4, 2024

joseivanlopez changed the title ~~feat(storage): model boot config~~ feat(storage): configure boot Dec 4, 2024

joseivanlopez force-pushed the storage-boot-model branch 5 times, most recently from 9d8129f to 573b811 Compare December 12, 2024 09:28

joseivanlopez force-pushed the storage-boot-model branch 12 times, most recently from 54756b8 to e5a510f Compare December 18, 2024 10:34

joseivanlopez marked this pull request as ready for review December 18, 2024 10:37

joseivanlopez mentioned this pull request Dec 19, 2024

feat(storage): allow selecting space policy #1791

Merged

joseivanlopez added 9 commits December 19, 2024 09:07

fix(schema): use alias for boot device

6043d7b

fix(storage): get boot device from alias

b5213c8

chore(storage): split config checker

eb3c5e1

feat(storage): check boot config

02be420

chore(storage): reorganize config solvers

35b6199

feat(storage): extend boot config

06fc6c0

- Allows to indicate whether the device is the default one.

feat(storage): add boot config solver

1ee52ea

fix(storage): avoid error in the old proposal settings

fe10bce

- ProposalSettings cannot use Configs::Boot.

feat(schema): add support for boot in storage model

e9036dc

joseivanlopez added 5 commits December 19, 2024 09:07

feat(storage): add boot to config model

24ddb84

feat(web): update config-model types

fe814ee

feat(web): extend and unify config model hooks

1401724

fix(web): adapt to new config model hooks

d807efe

feat(web): allow changing boot options

cbf0d39

joseivanlopez force-pushed the storage-boot-model branch from e5a510f to cbf0d39 Compare December 19, 2024 12:52

ancorgs reviewed Dec 19, 2024

View reviewed changes

service/lib/agama/storage/config_checkers/boot.rb Outdated Show resolved Hide resolved

ancorgs reviewed Dec 19, 2024

View reviewed changes

service/lib/agama/storage/configs/with_alias.rb Show resolved Hide resolved

joseivanlopez added 2 commits December 20, 2024 09:07

feat(storage): use proper space policy

a108175

If the space policy is not provided by the model: - Use 'keep' for a drive added for booting. - Use the default space policy defined by the product in other cases.

fix(storage): use securerandom to generate aliases

b12898a

ancorgs reviewed Dec 20, 2024

View reviewed changes

service/lib/agama/storage/config_checkers/boot.rb Outdated Show resolved Hide resolved

ancorgs approved these changes Dec 20, 2024

View reviewed changes

Fix typo

ce3e1e9

Co-authored-by: Ancor Gonzalez Sosa <[email protected]>

joseivanlopez merged commit e6d8bf4 into agama-project:storage-config-ui Dec 20, 2024
5 checks passed

imobachgs mentioned this pull request Jan 10, 2025

Update to release 11 #1887

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(storage): configure boot #1808

feat(storage): configure boot #1808

joseivanlopez commented Dec 4, 2024 •

edited

Loading

coveralls commented Dec 19, 2024 •

edited

Loading

ancorgs Dec 20, 2024

joseivanlopez Dec 20, 2024 •

edited

Loading

ancorgs Dec 20, 2024

joseivanlopez Dec 20, 2024

ancorgs Dec 20, 2024

dgdavid Dec 20, 2024

ancorgs left a comment

feat(storage): configure boot #1808

feat(storage): configure boot #1808

Conversation

joseivanlopez commented Dec 4, 2024 • edited Loading

coveralls commented Dec 19, 2024 • edited Loading

Pull Request Test Coverage Report for Build 12432686864

Details

💛 - Coveralls

ancorgs Dec 20, 2024

Choose a reason for hiding this comment

joseivanlopez Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

ancorgs Dec 20, 2024

Choose a reason for hiding this comment

joseivanlopez Dec 20, 2024

Choose a reason for hiding this comment

ancorgs Dec 20, 2024

Choose a reason for hiding this comment

dgdavid Dec 20, 2024

Choose a reason for hiding this comment

ancorgs left a comment

Choose a reason for hiding this comment

joseivanlopez commented Dec 4, 2024 •

edited

Loading

coveralls commented Dec 19, 2024 •

edited

Loading

joseivanlopez Dec 20, 2024 •

edited

Loading