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

refactor(web): use queries to handle iSCSI configuration #1598

Merged
merged 17 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions rust/agama-server/src/storage/web/iscsi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ use serde::Deserialize;
mod stream;
use stream::ISCSINodeStream;
use tokio_stream::{Stream, StreamExt};
use zbus::fdo::{PropertiesChanged, PropertiesProxy};
use zbus::{
fdo::{PropertiesChanged, PropertiesProxy},
names::InterfaceName,
};

/// Returns the stream of iSCSI-related events.
///
Expand Down Expand Up @@ -59,7 +62,7 @@ async fn initiator_stream(
.receive_properties_changed()
.await?
.filter_map(|change| match handle_initiator_change(change) {
Ok(event) => Some(event),
Ok(event) => event,
Err(error) => {
log::warn!("Could not read the initiator change: {}", error);
None
Expand All @@ -68,12 +71,17 @@ async fn initiator_stream(
Ok(stream)
}

fn handle_initiator_change(change: PropertiesChanged) -> Result<Event, ServiceError> {
fn handle_initiator_change(change: PropertiesChanged) -> Result<Option<Event>, ServiceError> {
let args = change.args()?;
let iscsi_iface =
InterfaceName::from_str_unchecked("org.opensuse.Agama.Storage1.ISCSI.Initiator");
if iscsi_iface != args.interface_name {
return Ok(None);
}
let changes = to_owned_hash(args.changed_properties());
let name = get_optional_property(&changes, "InitiatorName")?;
let ibft = get_optional_property(&changes, "IBFT")?;
Ok(Event::ISCSIInitiatorChanged { ibft, name })
Ok(Some(Event::ISCSIInitiatorChanged { ibft, name }))
}

#[derive(Clone)]
Expand Down
2 changes: 2 additions & 0 deletions web/src/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { useProduct, useProductChanges } from "./queries/software";
import { useL10nConfigChanges } from "~/queries/l10n";
import { useIssuesChanges } from "./queries/issues";
import { useInstallerStatus, useInstallerStatusChanges } from "./queries/status";
import { useDeprecatedChanges } from "./queries/storage";
import { PATHS as PRODUCT_PATHS } from "./routes/products";
import SimpleLayout from "./SimpleLayout";
import { InstallationPhase } from "./types/status";
Expand All @@ -51,6 +52,7 @@ function App() {
useProductChanges();
useIssuesChanges();
useInstallerStatusChanges();
useDeprecatedChanges();

const Content = () => {
if (error) return <ServerError />;
Expand Down
5 changes: 5 additions & 0 deletions web/src/App.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ jest.mock("~/queries/issues", () => ({
useIssuesChanges: () => jest.fn(),
}));

jest.mock("~/queries/storage", () => ({
...jest.requireActual("~/queries/storage"),
useDeprecatedChanges: () => jest.fn(),
}));

const mockClientStatus = {
phase: InstallationPhase.Startup,
isBusy: true,
Expand Down
16 changes: 15 additions & 1 deletion web/src/api/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import { get, post } from "~/api/http";
import { Job } from "~/types/job";
import { calculate, fetchSettings } from "~/api/storage/proposal";

/**
* Starts the storage probing process.
Expand All @@ -40,4 +41,17 @@ const fetchStorageJobs = (): Promise<Job[]> => get("/api/storage/jobs");
const findStorageJob = (id: string): Promise<Job | undefined> =>
fetchStorageJobs().then((jobs: Job[]) => jobs.find((value) => value.id === id));

export { fetchStorageJobs, findStorageJob };
/**
* Refreshes the storage layer.
*
* It does the probing again and recalculates the proposal with the same
* settings. Internally, it is composed of three different API calls
* (retrieve the settings, probe the system, and calculate the proposal).
*/
const refresh = async (): Promise<void> => {
const settings = await fetchSettings();
await probe();
await calculate(settings);
};

export { fetchStorageJobs, findStorageJob, refresh };
121 changes: 121 additions & 0 deletions web/src/api/storage/iscsi.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* Copyright (c) [2024] SUSE LLC
*
* All Rights Reserved.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of version 2 of the GNU General Public License as published
* by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
* more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, contact SUSE LLC.
*
* To contact SUSE LLC about this file by physical or electronic mail, you may
* find current contact information at www.suse.com.
*/

import { del, get, patch, post } from "~/api/http";
import { ISCSIInitiator, ISCSINode } from "~/api/storage/types";

const ISCSI_NODES_NAMESPACE = "/api/storage/iscsi/nodes";

const nodePath = (node: ISCSINode): string => ISCSI_NODES_NAMESPACE + "/" + node.id;

/**
* Returns the iSCSI initiator.
*/
const fetchInitiator = (): Promise<ISCSIInitiator> => get("/api/storage/iscsi/initiator");

/**
* Updates the name of the iSCSI initiator.
*/
const updateInitiator = ({ name }) => patch("/api/storage/iscsi/initiator", { name });

/**
* Returns the iSCSI nodes.
*/
const fetchNodes = (): Promise<ISCSINode[]> => get("/api/storage/iscsi/nodes");

type LoginOptions = {
// Password for authentication by target
password?: string;
// Username for authentication by target
username?: string;
// Password for authentication by initiator
reversePassword?: string;
// Username for authentication by initiator
reverseUsername?: string;
};

/**
* Performs an iSCSI discovery.
*
* @param address - IP address of the ISCSI server
* @param port - Port of the iSCSI server
* @param options - Authentication options
* @return true on success, false on failure
*/
const discover = async (address: string, port: number, options: LoginOptions): Promise<boolean> => {
const data = { address, port, options };
try {
await post("/api/storage/iscsi/discover", data);
return true;
} catch (error) {
console.error("Error discovering iSCSI targets:", error.message);
return false;
}
};

const deleteNode = async (node: ISCSINode): Promise<void> => {
await del(nodePath(node));
};

/*
* Creates an iSCSI session on the given node.
*
* @param node - ISCSI node
* @param options - Authentication options
* @return operation result (0: success; 1: invalid startup; 2: failed)
*/
const login = async (node: ISCSINode, options: LoginOptions) => {
try {
await post(nodePath(node) + "/login", options);
return 0;
} catch (error) {
const { data: reason } = error.response;
console.warn("Could not login into the iSCSI node:", error.message, "reason:", reason);
return reason === "InvalidStartup" ? 1 : 2;
}
};

/*
* Closes an iSCSI session on the given node.
*
* @param node - ISCSI node
*/
const logout = async (node: ISCSINode) => post(nodePath(node) + "/logout");

/**
* Sets the startup status of the connection
*
* @param node - ISCSI node
* @param startup - startup status
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, startup is string, so it can accept any value? or is it kind of enum with limited statuses?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a generic string. I do not like it that much, but I decided to keep it as it was, considering it was out of this PR's scope. But sure we should improve how we handle the staus.

*/
const setStartup = async (node: ISCSINode, startup: string) => patch(nodePath(node), { startup });

export {
fetchInitiator,
fetchNodes,
updateInitiator,
discover,
deleteNode,
login,
logout,
setStartup,
};
export type { LoginOptions };
8 changes: 7 additions & 1 deletion web/src/api/storage/proposal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@
*/

import { get, put } from "../http";
import { Action, ProductParams, ProposalSettings, ProposalSettingsPatch, Volume } from "~/api/storage/types";
import {
Action,
ProductParams,
ProposalSettings,
ProposalSettingsPatch,
Volume,
} from "~/api/storage/types";

const fetchUsableDevices = (): Promise<number[]> => get(`/api/storage/proposal/usable_devices`);

Expand Down
23 changes: 12 additions & 11 deletions web/src/components/storage/ProposalPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,21 @@ import ProposalActionsSummary from "~/components/storage/ProposalActionsSummary"
import { ProposalActionsDialog } from "~/components/storage";
import { _ } from "~/i18n";
import { SPACE_POLICIES } from "~/components/storage/utils";
import { toValidationError, useCancellablePromise } from "~/utils";
import { toValidationError } from "~/utils";
import { useIssues } from "~/queries/issues";
import { IssueSeverity } from "~/types/issues";
import {
useAvailableDevices,
useDeprecated,
useDeprecatedChanges,
useDevices,
useProductParams,
useProposalMutation,
useProposalResult,
useVolumeDevices,
useVolumeTemplates,
} from "~/queries/storage";
import { probe } from "~/api/storage";
import { useQueryClient } from "@tanstack/react-query";
import { refresh } from "~/api/storage";

/**
* Which UI item is being changed by user
Expand All @@ -70,7 +70,6 @@ export const NOT_AFFECTED = {
};

export default function ProposalPage() {
const { cancellablePromise } = useCancellablePromise();
const drawerRef = useRef();
const systemDevices = useDevices("system");
const stagingDevices = useDevices("result");
Expand All @@ -81,18 +80,20 @@ export default function ProposalPage() {
const { actions, settings } = useProposalResult();
const updateProposal = useProposalMutation();
const deprecated = useDeprecated();
useDeprecatedChanges();
const queryClient = useQueryClient();

const errors = useIssues("storage")
.filter((s) => s.severity === IssueSeverity.Error)
.map(toValidationError);

useEffect(() => {
React.useEffect(() => {
if (deprecated) {
cancellablePromise(probe());
refresh().then(() => {
queryClient.invalidateQueries({ queryKey: ["storage"] });
});
}
}, [deprecated]);

const errors = useIssues("storage")
.filter((s) => s.severity === IssueSeverity.Error)
.map(toValidationError);

const changeSettings = async (changing, updated: object) => {
const newSettings = { ...settings, ...updated };
updateProposal.mutateAsync(newSettings).catch(console.error);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) [2023] SUSE LLC
* Copyright (c) [2023-2024] SUSE LLC
*
* All Rights Reserved.
*
Expand Down Expand Up @@ -83,8 +83,8 @@ export default function DiscoverForm({ onSubmit: onSubmitProp, onCancel }) {
return isValidAddress() && isValidPort() && isValidAuth;
};

const showAddressError = () => data.address.length > 0 && !isValidAddress(data.address);
const showPortError = () => data.port.length > 0 && !isValidPort(data.port);
const showAddressError = () => data.address.length > 0 && !isValidAddress();
const showPortError = () => data.port.length > 0 && !isValidPort();

const id = "iscsiDiscover";
const isDisabled = isLoading || !isValidForm();
Expand Down
67 changes: 67 additions & 0 deletions web/src/components/storage/iscsi/InitiatorPresenter.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Copyright (c) [2024] SUSE LLC
*
* All Rights Reserved.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of version 2 of the GNU General Public License as published
* by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
* more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, contact SUSE LLC.
*
* To contact SUSE LLC about this file by physical or electronic mail, you may
* find current contact information at www.suse.com.
*/

import React from "react";
import { screen, within } from "@testing-library/react";
import { plainRender } from "~/test-utils";

import InitiatorPresenter from "./InitiatorPresenter";
import { ISCSIInitiator } from "~/types/storage";

const initiator: ISCSIInitiator = {
name: "iqn.1996-04.de.suse:01:62b45cf7fc",
ibft: true,
offloadCard: "",
};

const mockInitiatorMutation = { mutateAsync: jest.fn() };

jest.mock("~/queries/storage/iscsi", () => ({
...jest.requireActual("~/queries/storage/iscsi"),
useInitiatorMutation: () => mockInitiatorMutation,
}));

describe("InitiatorPresenter", () => {
it("displays the initiator data", () => {
plainRender(<InitiatorPresenter initiator={initiator} />);
screen.getByText(initiator.name);
screen.getByRole("cell", { name: initiator.name });
});

it("updates the initiator data", async () => {
const { user } = plainRender(<InitiatorPresenter initiator={initiator} />);

const button = await screen.findByRole("button", { name: "Actions" });
await user.click(button);

const editButton = await screen.findByRole("menuitem", { name: "Edit" });
await user.click(editButton);

const dialog = await screen.findByRole("dialog");
const nameInput = await within(dialog).findByLabelText(/Name/);
await user.clear(nameInput);
await user.type(nameInput, "my-initiator");
const confirmButton = screen.getByRole("button", { name: "Confirm" });
await user.click(confirmButton);

expect(mockInitiatorMutation.mutateAsync).toHaveBeenCalledWith({ name: "my-initiator" });
imobachgs marked this conversation as resolved.
Show resolved Hide resolved
});
});
Loading
Loading