Skip to content

Commit

Permalink
refactor(web): use queries to handle iSCSI configuration (#1598)
Browse files Browse the repository at this point in the history
Trello: https://trello.com/c/MsGbqiwA/

This is a follow-up of #1585 which
adapts the iSCSI-related code to use queries instead of the regular
Agama client. Additionally, it converts a good share of the code to
TypeScript and adds some iSCSI tests, that were not included in the
first version.
  • Loading branch information
imobachgs authored Sep 13, 2024
2 parents 8900663 + 684d796 commit 04bd65a
Show file tree
Hide file tree
Showing 20 changed files with 679 additions and 240 deletions.
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
*/
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" });
});
});
Loading

0 comments on commit 04bd65a

Please sign in to comment.