Skip to content

Commit

Permalink
Description for adjust_by_ram (#1081)
Browse files Browse the repository at this point in the history
## Problem

In Agama a volume can have automatic sizes based on one or several of
the following reasons:

- Snapshots
- Fallbacks from other volumes (eg. "/" max size can exist or not based
on the existence of "/home")
- Size of the RAM

The three are already implemented and working. But the third one lacks
the corresponding explanations.

| No information icon in the "auto" label | Incomplete sentence in the
form |
|-|-|
|
![by_ram](https://github.com/openSUSE/agama/assets/3638289/a6a2aedd-b630-4346-8d16-6c2cc2fa6d85)
|
![enlarge](https://github.com/openSUSE/agama/assets/3638289/2f0443ae-a732-40af-a858-f8314d48ad19)
|

## Solution

This pull request introduces the missing messages, so now the proper
information is displayed both in the table of volumes and in the form.

| The "auto" label | The form |
|-|-|
|
![ram-table](https://github.com/openSUSE/agama/assets/3638289/ec742117-6859-462c-8a9c-460c1509bf43)
|
![ram-form](https://github.com/openSUSE/agama/assets/3638289/3fd74029-196c-4b4f-9da5-74e5efbe52e0)
|

The message is "as good as it gets" for the time being. The plan is to
add the size of the RAM to it, but for that we need to improve the Agama
API as reflected at

https://trello.com/c/suiA6MzZ/354-agama-api-for-system-information-like-ram-size

Additionally, this pull request enables `adjust_by_ram` for swap at the
Tumbleweed product, to raise awareness and get feedback.

### About the approach

We can already foresee people asking "where is my 'Enlarge to RAM Size
for Suspend' checkbox"? So let's explain how the approach has changed.
That approach, not only for swap but for so-called volumes in general,
is explained in the section "[Automatic Size
Limits](https://github.com/openSUSE/agama/blob/master/doc/storage_ui.md#automatic-size-limits)"
of the document describing the storage UI.

In the traditional YaST storage proposal users has no direct way to
specify the size of the partitions/LVs being created. The product
specifies the size limits. If those limits cannot be known beforehand
because they depend on other factors, the product configuration specify
those factors. All that is explained at the [YaST configuration
document](https://github.com/yast/yast-installation/blob/master/doc/control-file.md#the-volumes-subsection),
which details the usage of `fallback_for_*`, `snapshots_size`,
`snapshots_percentage` and `adjust_by_ram`.

Since users cannot adjust the sizes directly, the mentioned checkbox
"Enlarge to RAM Size for Suspend" was introduced as a way to influence
them. But to be honest, it has been controversial from the beginning
since determining the proper size for the swap partition is an extremely
complex topic.

It's also worth noticing that swap is not the only volume that could use
that setting. Some products may adapt the size of other file-systems
based on the RAM size. For example, defining a bigger partition for "/"
or "/var" in order to allocate the kernel dumps generated by kdump.

As explained in the document mentioned above: "_so far the adaptation
consists in ensuring all the sizes are, at least, as big as the RAM. In
the future, an extra `adjust_by_ram_mode` option could be added to allow
other approaches_".

In Agama the philosophy of volumes is different in general. Users can
always adapt any size limit if they know what they are doing. The
product still provide sensible defaults (fixed or automatic) but the
users can always tweak those sizes. The case of "enlarge to RAM size"
fits quite well in that new philosophy. Users don't need to know the
technical details about how swap and RAM sizes correlate (and
suspend-to-ram is NOT the only reason, the topic goes way further). They
can just trust the automatic proposal that will be good enough for most
people or adjust the size limits if they know what they are doing.

If needed, we can extend the possibilities for the product in the future
(for example, a configuration option to only observe RAM in case of
physical machine, using fixed size limits for virtual machines).

## Testing

Tested manually (see screenshots).
  • Loading branch information
ancorgs authored Mar 11, 2024
2 parents 49dfd5b + 133076d commit 704b693
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 8 deletions.
8 changes: 5 additions & 3 deletions products.d/tumbleweed.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,12 @@ storage:
- mount_path: "swap"
filesystem: swap
size:
auto: false
min: 1 GiB
max: 2 GiB
auto: true
outline:
auto_size:
base_min: 1 GiB
base_max: 2 GiB
adjust_by_ram: true
required: false
filesystems:
- swap
Expand Down
2 changes: 1 addition & 1 deletion service/.rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ AllCops:
Exclude:
- vendor/**/*
- lib/agama/dbus/y2dir/**/*
- agama.gemspec
- agama-yast.gemspec
- package/*.spec

# a D-Bus method definition may take up more line lenght than usual
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def outline_conversion(target)
"Required" => outline.required?,
"FsTypes" => outline.filesystems.map(&:to_human_string),
"SupportAutoSize" => outline.adaptive_sizes?,
"AdjustByRam" => outline.adjust_by_ram?,
"SnapshotsConfigurable" => outline.snapshots_configurable?,
"SnapshotsAffectSizes" => outline.snapshots_affect_sizes?,
"SizeRelevantVolumes" => outline.size_relevant_volumes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
"SupportAutoSize" => false,
"SnapshotsConfigurable" => false,
"SnapshotsAffectSizes" => false,
"AdjustByRam" => false,
"SizeRelevantVolumes" => []
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
outline.snapshots_configurable = true
outline.snapshots_size = Y2Storage::DiskSize.new(1000)
outline.snapshots_percentage = 10
outline.adjust_by_ram = true
end

Agama::Storage::Volume.new("/test").tap do |volume|
Expand Down Expand Up @@ -70,6 +71,7 @@
"Required" => false,
"FsTypes" => [],
"SupportAutoSize" => false,
"AdjustByRam" => false,
"SnapshotsConfigurable" => false,
"SnapshotsAffectSizes" => false,
"SizeRelevantVolumes" => []
Expand All @@ -90,6 +92,7 @@
"Outline" => {
"Required" => true,
"FsTypes" => ["Ext3", "Ext4"],
"AdjustByRam" => true,
"SupportAutoSize" => true,
"SnapshotsConfigurable" => true,
"SnapshotsAffectSizes" => true,
Expand Down
2 changes: 2 additions & 0 deletions web/src/client/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ class ProposalManager {
* @property {boolean} required
* @property {string[]} fsTypes
* @property {boolean} supportAutoSize
* @property {boolean} adjustByRam
* @property {boolean} snapshotsConfigurable
* @property {boolean} snapshotsAffectSizes
* @property {string[]} sizeRelevantVolumes
Expand Down Expand Up @@ -543,6 +544,7 @@ class ProposalManager {
required: dbusOutline.Required.v,
fsTypes: dbusOutline.FsTypes.v.map(val => val.v),
supportAutoSize: dbusOutline.SupportAutoSize.v,
adjustByRam: dbusOutline.AdjustByRam.v,
snapshotsConfigurable: dbusOutline.SnapshotsConfigurable.v,
snapshotsAffectSizes: dbusOutline.SnapshotsAffectSizes.v,
sizeRelevantVolumes: dbusOutline.SizeRelevantVolumes.v.map(val => val.v)
Expand Down
6 changes: 6 additions & 0 deletions web/src/client/storage.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ const contexts = {
SupportAutoSize: { t: "b", v: true },
SnapshotsConfigurable: { t: "b", v: true },
SnapshotsAffectSizes: { t: "b", v: true },
AdjustByRam: { t: "b", v: false },
SizeRelevantVolumes: { t: "as", v: [{ t: "s", v: "/home" }] }
}
}
Expand All @@ -349,6 +350,7 @@ const contexts = {
SupportAutoSize: { t: "b", v: false },
SnapshotsConfigurable: { t: "b", v: false },
SnapshotsAffectSizes: { t: "b", v: false },
AdjustByRam: { t: "b", v: false },
SizeRelevantVolumes: { t: "as", v: [] }
}
}
Expand Down Expand Up @@ -1017,6 +1019,7 @@ describe("#proposal", () => {
SupportAutoSize: { t: "b", v: false },
SnapshotsConfigurable: { t: "b", v: false },
SnapshotsAffectSizes: { t: "b", v: false },
AdjustByRam: { t: "b", v: false },
SizeRelevantVolumes: { t: "as", v: [] }
}
}
Expand All @@ -1037,6 +1040,7 @@ describe("#proposal", () => {
SupportAutoSize: { t: "b", v: false },
SnapshotsConfigurable: { t: "b", v: false },
SnapshotsAffectSizes: { t: "b", v: false },
AdjustByRam: { t: "b", v: false },
SizeRelevantVolumes: { t: "as", v: [] }
}
}
Expand Down Expand Up @@ -1064,6 +1068,7 @@ describe("#proposal", () => {
supportAutoSize: false,
snapshotsConfigurable: false,
snapshotsAffectSizes: false,
adjustByRam: false,
sizeRelevantVolumes: []
}
});
Expand All @@ -1084,6 +1089,7 @@ describe("#proposal", () => {
supportAutoSize: false,
snapshotsConfigurable: false,
snapshotsAffectSizes: false,
adjustByRam: false,
sizeRelevantVolumes: []
}
});
Expand Down
12 changes: 8 additions & 4 deletions web/src/components/storage/ProposalVolumes.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,16 @@ import { noop } from "~/utils";
* @returns {(ReactComponent|null)} component to display (can be `null`)
*/
const AutoCalculatedHint = (volume) => {
// no hint, the size is not affected by snapshots or other volumes
const { snapshotsAffectSizes = false, sizeRelevantVolumes = [] } = volume.outline;
const { snapshotsAffectSizes = false, sizeRelevantVolumes = [], adjustByRam } = volume.outline;

if (!snapshotsAffectSizes && sizeRelevantVolumes.length === 0) {
// no hint, the size is not affected by known criteria
if (!snapshotsAffectSizes && !adjustByRam && sizeRelevantVolumes.length === 0) {
return null;
}

return (
<>
{/* TRANSLATORS: header for a list of items */}
{/* TRANSLATORS: header for a list of items referring to size limits for file systems */}
{_("These limits are affected by:")}
<List>
{snapshotsAffectSizes &&
Expand All @@ -65,6 +65,10 @@ const AutoCalculatedHint = (volume) => {
// TRANSLATORS: list item, this affects the computed partition size limits
// %s is replaced by a list of the volumes (like "/home, /boot")
<ListItem>{sprintf(_("Presence of other volumes (%s)"), sizeRelevantVolumes.join(", "))}</ListItem>}
{adjustByRam &&
// TRANSLATORS: list item, describes a factor that affects the computed size of a
// file system; eg. adjusting the size of the swap
<ListItem>{_("The amount of RAM in the system")}</ListItem>}
</List>
</>
);
Expand Down
4 changes: 4 additions & 0 deletions web/src/components/storage/VolumeForm.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,10 @@ const SizeAuto = ({ volume }) => {
// TRANSLATORS: conjunction for merging two list items
volume.outline.sizeRelevantVolumes.join(_(", "))));

if (volume.outline.adjustByRam)
// TRANSLATORS: item which affects the final computed partition size
conditions.push(_("the amount of RAM in the system"));

// TRANSLATORS: the %s is replaced by the items which affect the computed size
const conditionsText = sprintf(_("The final size depends on %s."),
// TRANSLATORS: conjunction for merging two texts
Expand Down

0 comments on commit 704b693

Please sign in to comment.