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: Support stopped Stratis pools #18679

Merged
merged 1 commit into from
Apr 30, 2023

Conversation

mvollmer
Copy link
Member

Stopped pools are a generalization of locked encrypted pools introduced in newer versions of Stratis. Now every kind of pool can be inactive, not just encrypted ones.

@mvollmer
Copy link
Member Author

Stopped pools look like this on the overview now:

image

Maybe there is a better icon to use for the "Start" action?

@mvollmer
Copy link
Member Author

Most of the uncovered stuff is about Stratis 2 (which only runs on rhel-8 and centos-8) and about showing stopped pools in the overview. I'll modify the test to cover the latter.

Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Thanks! A few comments, but if this is urgent (blocking too much), I can be talked into landing as-is and treating the comments as follow-ups.

pkg/storaged/client.js Show resolved Hide resolved
pkg/storaged/stratis-details.jsx Show resolved Hide resolved
pkg/storaged/stratis-panel.jsx Outdated Show resolved Hide resolved
test/verify/check-storage-stratis Show resolved Hide resolved
martinpitt
martinpitt previously approved these changes Apr 26, 2023
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Danke!

@martinpitt
Copy link
Member

EBUSY, that's #18707, retrying

Stopped pools are a generalization of locked encrypted pools
introduced in newer versions of Stratis.  Now every kind of pool can
be inactive, not just encrypted ones.
@martinpitt
Copy link
Member

Dang, conflict with Katerina's recent PF cleanup. I fixed the (simple) conflict and FPed your branch.

@@ -1029,7 +1029,7 @@ function stratis2_start() {
.input(passphrase);
};

client.stratis_unlock_pool = (uuid) => {
client.stratis_start_pool = (uuid) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

@@ -1138,10 +1138,10 @@
function stratis2_fetch_manager_properties(proxy) {
stratis2_fetch_properties(proxy, ["LockedPoolsWithDevs"]).then(values => {
if (values.LockedPoolsWithDevs) {
proxy.LockedPools = { };
proxy.StoppedPools = { };
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

for (const uuid in values.LockedPoolsWithDevs) {
const l = values.LockedPoolsWithDevs[uuid];
proxy.LockedPools[uuid] = {
proxy.StoppedPools[uuid] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

@@ -455,9 +455,9 @@ function block_description(client, block) {
if (config) {
type = C_("storage-id-desc", "Filesystem (encrypted)");
used_for = mount_point;
} else if (block_stratis_locked_pool) {
} else if (block_stratis_stopped_pool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

type = _("Stratis member");
used_for = block_stratis_locked_pool;
used_for = block_stratis_stopped_pool;
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

Comment on lines +720 to +723
!stopped_props.key_description.v[1].v[0]) {
dialog_open({
Title: _("Error"),
Body: _("This pool can not be unlocked here because its key description is not in the expected format.")
Copy link
Contributor

Choose a reason for hiding this comment

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

These 4 added lines are not executed by any test. Details

Title: _("Error"),
Body: _("This pool can not be unlocked here because its key description is not in the expected format.")
});
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

return start();
} else {
return (client.stratis_list_keys()
.catch(() => [{ }])
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

.catch(() => [{ }])
.then(keys => {
if (keys.indexOf(key_desc) >= 0)
return start("keyring");
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

@@ -80,17 +80,17 @@ export function stratis_rows(client) {
return client.stratis_pools[path_a].Name.localeCompare(client.stratis_pools[path_b].Name);
}

function cmp_locked_pool(uuid_a, uuid_b) {
function cmp_stopped_pool(uuid_a, uuid_b) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

@martinpitt martinpitt merged commit ae98a61 into cockpit-project:main Apr 30, 2023
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.

3 participants