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

storage: Add new integration layer between anaconda and cockpit storage #72

Merged
merged 9 commits into from Feb 16, 2024

Conversation

KKoukiou
Copy link
Contributor

@KKoukiou KKoukiou commented Nov 30, 2023

@@ -58,6 +58,7 @@ import {
import "./MountPointMapping.scss";

const _ = cockpit.gettext;
const cmpm = JSON.parse(window.localStorage.getItem("cockpit_mount_points"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cockpit is no longer exporting this information.

@mvollmer
Copy link
Collaborator

@KKoukiou, could you rebase this? Thanks!

@mvollmer
Copy link
Collaborator

I made a version of this that works with cockpit-project/cockpit#19352: https://github.com/mvollmer/anaconda-webui/tree/cockpit-storage

cockpit-project/cockpit#19352 will not change incompatibly any more.

@mvollmer
Copy link
Collaborator

mvollmer commented Jan 5, 2024

I have pushed my version here.

@KKoukiou KKoukiou force-pushed the cockpit-storage branch 3 times, most recently from 4a187e1 to 79d8d89 Compare January 12, 2024 15:29
@KKoukiou KKoukiou force-pushed the cockpit-storage branch 5 times, most recently from c6f8b87 to 8a26fe7 Compare January 19, 2024 15:06
@rvykydal
Copy link
Contributor

rvykydal commented Jan 25, 2024

I am seeing an issue: if I go to "Mount point assignment", then back and then to "Mount point assignment" again, required and recommended rows disappear. I hit it for example after creating one 10G ext4 partition (created in shell or via cockpit-storage). Rescan before going to the "Mount point assignment" makes the required/recommendend mount points to appear again.
I am not seeing the issue neither on main nor with these patches: #95
UPDATE:
I am seeing it on this patch in this PR stack: Break-line in the InstallationScenarioSelector component properties, I'll try to update tests for this issue -> #147

@mvollmer
Copy link
Collaborator

Nice!

I keep thinking that as a user, I would like to see the summary of what is being done immediately after "returning to installation", to double check whether Anaconda has correctly understood what I want to do. Now I have to fill out the account information before I can see the summary. Thus, the account stuff gets "in the way" of finishing the storage configuration.

@KKoukiou
Copy link
Contributor Author

Nice!

I keep thinking that as a user, I would like to see the summary of what is being done immediately after "returning to installation", to double check whether Anaconda has correctly understood what I want to do. Now I have to fill out the account information before I can see the summary. Thus, the account stuff gets "in the way" of finishing the storage configuration.

I agree. @garrett can you give this some thought?

@garrett
Copy link
Contributor

garrett commented Feb 14, 2024

Sure, that makes sense. I thought Cockpit doesn't track the changes, however, which prevented us from showing this?

I wanted that too, but was told that Cockpit doesn't track the changes and Anaconda doesn't know about the changes either. So I don't know how we could even get this information or what it would be like.

Plus, at this stage, there is nothing to be done, as it was already done. Cockpit, via udisks, doesn't queue up changes, but acts on them immediately.

@KKoukiou
Copy link
Contributor Author

Sure, that makes sense. I thought Cockpit doesn't track the changes, however, which prevented us from showing this?

I wanted that too, but was told that Cockpit doesn't track the changes and Anaconda doesn't know about the changes either. So I don't know how we could even get this information or what it would be like.

Plus, at this stage, there is nothing to be done, as it was already done. Cockpit, via udisks, doesn't queue up changes, but acts on them immediately.

All the information we present in the review page, (which partitions have which mount points etc) are already present when that dialog appears that allows us to exit storage mode.

We can add these information in that dialog if you like.

@garrett
Copy link
Contributor

garrett commented Feb 14, 2024

When I go to create a biosboot partition, it wants to give it all the available space. Why is space even configurable for this? Isn't it supposed to be 1 megabyte. Why does it have overwrite as an option too?

image

@garrett
Copy link
Contributor

garrett commented Feb 14, 2024

image

It says "Apply storage configuration and install". But didn't we already do storage configuration in Cockpit? Shouldn't it just be "Install" at this point in time?

@garrett
Copy link
Contributor

garrett commented Feb 14, 2024

Also, I went back and the configuration was lost, so I selected "mount point mapping" (to see what it'd say for the "X and install" button), which then won't let me proceed for some reason:

image

@KKoukiou
Copy link
Contributor Author

KKoukiou commented Feb 14, 2024

Also, I went back and the configuration was lost, so I selected "mount point mapping" (to see what it'd say for the "X and install" button), which then won't let me proceed for some reason:

image

This is a 'bug' / 'feature'. you can remove the row with the trash icon the the next will be enabled. We dont want the user to submit a visibly incomplete form.

Copy link
Contributor

@rvykydal rvykydal left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you.
Brave work!

@KKoukiou KKoukiou force-pushed the cockpit-storage branch 7 times, most recently from 05c442e to edb0f47 Compare February 14, 2024 16:57
This commit introduces a higher level of integration between Anaconda Web UI and Cockpit storage.
When users perform advanced storage configuration, their storage settings will now seamlessly sync
with Anaconda web UI. Specifically, selected mount points and configured LUKS devices will be
automatically detected and utilized during installation, streamlining the process for users.
This enhancement aims to improve the user experience and reduce manual configuration efforts during
installation.

Resolves: rhbz#2263971
Note: This is a temporary workaround which should be obsoleted and removed
once https://issues.redhat.com/browse/INSTALLER-3898 is fixed.
@KKoukiou KKoukiou merged commit ddd1dff into rhinstaller:main Feb 16, 2024
5 of 7 checks passed
@KKoukiou KKoukiou deleted the cockpit-storage branch February 16, 2024 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants