Skip to content

chore: sync master branch#1953

Merged
dgdavid merged 41 commits intostorage-config-uifrom
storage-config-ui-master-sync-20250128
Jan 28, 2025
Merged

chore: sync master branch#1953
dgdavid merged 41 commits intostorage-config-uifrom
storage-config-ui-master-sync-20250128

Conversation

@dgdavid
Copy link
Copy Markdown
Contributor

@dgdavid dgdavid commented Jan 28, 2025

Yet another PR to keep storage-config-ui in sync with master branch.

imobachgs and others added 30 commits January 21, 2025 09:21
## Problem

When using the legacy AutoYaST mode, the web UI crashes at the end of
the installation when trying
to find our whether it is using TPM or not.

- #1918

## Solution

Properly handle that case:

* `fetchConfig` now returns `null` instead of `undefined`. According to
[TanStack Query

documentation](https://tanstack.com/query/latest/docs/framework/react/reference/useQuery),
`queryFn`
*Must return a promise that will either resolve data or throw an error.
The data cannot be
undefined*.
* `usingTpm` function now handles the case where the configuration is
`null`.

## Testing

- *Tested manually*
## Problem

`ListRepositories` D-Bus method just crashes. It is expected to return
an array with contains the
return value as its first element. However, when there are no
repositories, it just returns an empty
array. It causes Ruby D-Bus to fail because it cannot serialize a `nil`
value.

## Solution

Fix the return value. For comparison, you can check the following
`ListPatterns` method.

## Testing

- _Tested manually_
… web UI (#1933)

## Problem

- When the repository initialization fails fast in the autoinstallation
mode the root password might not be imported yet and the web UI is stuck
in the root password popup.


![agama-broken-user-config](https://github.com/user-attachments/assets/fcd9bb0c-8dd4-4b65-bcec-b0075301c4de)


## Solution

- Import the user setting earlier so if the repository refresh fails the
password is already set and the web UI automatically goes to the
overview page.


## Testing

- Tested manually with locally built Live ISO


## Screenshots

- The web UI displays the overview page


![agama-fixed-user-config-overview](https://github.com/user-attachments/assets/0f5f95a8-d6fd-464d-b913-f2c3c61f5db7)

- After switching to the software page the initial repository error is
displayed


![agama-fixed-user-config](https://github.com/user-attachments/assets/d28c90cb-a4aa-4ba7-85d2-f9a9a564b5c6)
## Problem

SLES and SLES4SAP product does not have defined installation labels, but
it is needed for full medium.


## Solution

Define them as agreed.
## Problem

- Package build fails in OBS with OOM killer
-
https://build.suse.de/package/live_build_log/Devel:YaST:Agama:Head/agama/SLES-16.0/s390x

## Details

Sometimes the build might fail in OBS because Rust compilation requires
huge amount of RAM. The problem happens when running many parallel jobs
on a machine with not enough RAM. The build on S390 failed when running
8 jobs with 8GB RAM.

Originally I wanted to increase the requirement for RAM in the
`_constraints` file from 8GB to 16GB. But that would decrease the number
of available workers significantly, esp. on exotic archs like s390. And
in most cases the workers run 4 or 8 jobs so requiring 16GB would be an
overkill.

## Solution

So to decrease the needed amount of RAM decrease the maximum number of
parallel jobs.

My experimental run `/usr/bin/time -v cargo build -j1` showed maximum
used memory 1.1GB on x86_64. The compilation on S390 failed with 1GB per
job. To be on the safe side let's require 1.3GB per job, different
architectures might require more and we need to also leave something for
the system and other services.

That means with 8GB RAM it should run at most 6 parallel jobs. That
should hopefully avoid triggering the OOM killer.

## Notes

- We can adjust the RAM per job constant later if needed, this initial
value is rather an experimental value, let's see how it will work.
- If some architecture needs quite different amount of RAM we can make
the setting arch dependent using the `%ifarch` macro.
The test for the product registration alert was removed since it’s
already covered in its own test file.

As with other page tests, mocking the ProductRegistrationAlert component
was necessary to prevent the suite from rendering nothing, due to the
component getting stuck in the useIssues hook request. An alternative
would be to mock the useIssues hook itself, but it requires more mocking
for the same result. This is something to improve globally, but outside
the scope of this commit.
## Problem

The registration workflow does not allow installing without registering
the system, even if the repositories are offline or are specified
through the `install_url` option. Thus, we need to make some changes to
the current implementation.

## Solution

Now Agama will complain if the following conditions are met:

* The product can be registered (`registration: true`).
* The base product is missing from the repositories.

If the product can be registered, but the base product is already in the
repositories, Agama will not complain. However, you can register the
product if you want to.

## The new approach

Instead of exposing information about whether the system should be
registered at the HTTP API level, we have decided to rely on
installation issues. If the conditions are not met, Agama will set an
issue.
lslezak and others added 11 commits January 24, 2025 10:51
Unfortunately SLE16 uses old rpm version 4.18 which does not
support %{getncpus:proc} (added in 4.19).

We need to compute the value manually. :-/
## Problem
- Related to #1939
- Unfortunately SLE16 uses old rpm version 4.18 which does not support
`%{getncpus:proc}` (added in 4.19).
- We need to compute the value manually. :-/

## Details

- I wanted to use some `%if` and use the new option in TW. However the
macro exists in both but in SLE16 it complains about extra argument
which is not accepted. And rpm evaluates macros in both `%if`
branches... 😟
- So I basically reverted to my original implementation
…1942)

## Problem

Agama determines in which point of the installation it is by the
combination of two values: the
installation phase (startup, config and install) and the isBusy flag.
This method has proven to be
rather fragile.

One of the typical issues is jumping into the "congratulations" screen
when the installation is not even finished. See #1616.

Although we mitigated this problem, it recently appear in our QA
testing.

## Solution

Let's introduce a new phase, "finish", which indicates that the
installation is finished. As simple
as that. So we do not need a combination of two values anymore at that
point. We plan to extend this
approach to other parts.

## Testing

- _Added a new unit test_
- _Tested manually_
…ation (#1945)

- Another iteration of #1939 and #1943 
- Now use an external macro instead of our implementation
- Tested in TW and SLE16 builds, works fine in both
@dgdavid dgdavid merged commit 1eecb3b into storage-config-ui Jan 28, 2025
@dgdavid dgdavid deleted the storage-config-ui-master-sync-20250128 branch January 28, 2025 07:38
@imobachgs imobachgs mentioned this pull request Feb 26, 2025
imobachgs added a commit that referenced this pull request Feb 26, 2025
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.

4 participants