From da6e258004570f5a2652c85444de5414446d8bdd Mon Sep 17 00:00:00 2001 From: David Edler Date: Wed, 24 Jan 2024 14:52:46 +0100 Subject: [PATCH] feat(storage) use config api for form fields help text on storage pools and volumes WD-7393 Signed-off-by: David Edler --- src/pages/storage/forms/StoragePoolForm.tsx | 13 ++-- .../storage/forms/StoragePoolFormCeph.tsx | 5 -- .../storage/forms/StoragePoolFormMain.tsx | 6 +- src/pages/storage/forms/StorageVolumeForm.tsx | 20 ++++- .../storage/forms/StorageVolumeFormBlock.tsx | 2 - .../forms/StorageVolumeFormSnapshots.tsx | 1 - .../storage/forms/StorageVolumeFormZFS.tsx | 6 -- src/types/config.d.ts | 10 +++ src/util/config.spec.ts | 18 ++++- src/util/config.tsx | 10 ++- src/util/configInheritance.spec.ts | 78 +++++++++++++++++++ src/util/configInheritance.tsx | 46 +++++++++-- src/util/storagePool.tsx | 54 +++++++------ src/util/storageVolume.tsx | 38 +++------ tests/storage.spec.ts | 2 +- 15 files changed, 224 insertions(+), 85 deletions(-) create mode 100644 src/util/configInheritance.spec.ts diff --git a/src/pages/storage/forms/StoragePoolForm.tsx b/src/pages/storage/forms/StoragePoolForm.tsx index 34e3514e0c..89a9bae1f7 100644 --- a/src/pages/storage/forms/StoragePoolForm.tsx +++ b/src/pages/storage/forms/StoragePoolForm.tsx @@ -10,7 +10,7 @@ import useEventListener from "@use-it/event-listener"; import { updateMaxHeight } from "util/updateMaxHeight"; import { LxdStoragePool } from "types/storage"; import { btrfsDriver, cephDriver } from "util/storageOptions"; -import { getCephConfigKey } from "util/storagePool"; +import { getPoolKey } from "util/storagePool"; import StoragePoolFormCeph from "./StoragePoolFormCeph"; export interface StoragePoolFormValues { @@ -42,12 +42,11 @@ export const storagePoolFormToPayload = ( const getConfig = () => { if (isCephDriver) { return { - [getCephConfigKey("ceph_cluster_name")]: values.ceph_cluster_name, - [getCephConfigKey("ceph_osd_pg_num")]: - values.ceph_osd_pg_num?.toString(), - [getCephConfigKey("ceph_rbd_clone_copy")]: values.ceph_rbd_clone_copy, - [getCephConfigKey("ceph_user_name")]: values.ceph_user_name, - [getCephConfigKey("ceph_rbd_features")]: values.ceph_rbd_features, + [getPoolKey("ceph_cluster_name")]: values.ceph_cluster_name, + [getPoolKey("ceph_osd_pg_num")]: values.ceph_osd_pg_num?.toString(), + [getPoolKey("ceph_rbd_clone_copy")]: values.ceph_rbd_clone_copy, + [getPoolKey("ceph_user_name")]: values.ceph_user_name, + [getPoolKey("ceph_rbd_features")]: values.ceph_rbd_features, source: values.source, }; } else { diff --git a/src/pages/storage/forms/StoragePoolFormCeph.tsx b/src/pages/storage/forms/StoragePoolFormCeph.tsx index a9a166aa51..bf81a78672 100644 --- a/src/pages/storage/forms/StoragePoolFormCeph.tsx +++ b/src/pages/storage/forms/StoragePoolFormCeph.tsx @@ -19,7 +19,6 @@ const StoragePoolFormCeph: FC = ({ formik }) => { label: "Cluster name", name: "ceph_cluster_name", defaultValue: "", - help: "Name of the Ceph cluster in which to create new storage pools", children: , }), getConfigurationRow({ @@ -27,7 +26,6 @@ const StoragePoolFormCeph: FC = ({ formik }) => { label: "Placement groups", name: "ceph_osd_pg_num", defaultValue: "", - help: "Number of placement groups for the OSD storage pool", children: ( = ({ formik }) => { label: "RBD clone copy", name: "ceph_rbd_clone_copy", defaultValue: "", - help: "Whether to use RBD lightweight clones rather than full dataset copies", children: , }), getConfigurationRow({ @@ -56,7 +52,6 @@ const StoragePoolFormCeph: FC = ({ formik }) => { label: "RBD features", name: "ceph_rbd_features", defaultValue: "", - help: "Comma-separated list of RBD features to enable on the volumes", children: , }), ]} diff --git a/src/pages/storage/forms/StoragePoolFormMain.tsx b/src/pages/storage/forms/StoragePoolFormMain.tsx index 64e09bcb4b..1444b670e3 100644 --- a/src/pages/storage/forms/StoragePoolFormMain.tsx +++ b/src/pages/storage/forms/StoragePoolFormMain.tsx @@ -12,7 +12,7 @@ import { import { StoragePoolFormValues } from "./StoragePoolForm"; import DiskSizeSelector from "components/forms/DiskSizeSelector"; import AutoExpandingTextArea from "components/AutoExpandingTextArea"; -import { cephStoragePoolDefaults } from "util/storagePool"; +import { getCephPoolFormFields } from "util/storagePool"; import { useSettings } from "context/useSettings"; import ScrollableForm from "components/ScrollableForm"; @@ -82,8 +82,8 @@ const StoragePoolFormMain: FC = ({ formik }) => { void formik.setFieldValue("source", ""); } if (val !== cephDriver) { - const cephConfigFields = Object.keys(cephStoragePoolDefaults); - for (const field of cephConfigFields) { + const cephFields = getCephPoolFormFields(); + for (const field of cephFields) { void formik.setFieldValue(field, undefined); } } diff --git a/src/pages/storage/forms/StorageVolumeForm.tsx b/src/pages/storage/forms/StorageVolumeForm.tsx index 943a614b94..9f47e9c45e 100644 --- a/src/pages/storage/forms/StorageVolumeForm.tsx +++ b/src/pages/storage/forms/StorageVolumeForm.tsx @@ -17,13 +17,18 @@ import StorageVolumeFormSnapshots from "pages/storage/forms/StorageVolumeFormSna import StorageVolumeFormBlock from "pages/storage/forms/StorageVolumeFormBlock"; import StorageVolumeFormZFS from "pages/storage/forms/StorageVolumeFormZFS"; import { FormikProps } from "formik/dist/types"; -import { getVolumeKey } from "util/storageVolume"; +import { + getFilesystemVolumeFormFields, + getVolumeKey, + getZfsVolumeFormFields, +} from "util/storageVolume"; import { LxdStorageVolume, LxdStorageVolumeContentType, LxdStorageVolumeType, } from "types/storage"; import { slugify } from "util/slugify"; +import { driversWithFilesystemSupport } from "util/storageOptions"; export interface StorageVolumeFormValues { name: string; @@ -130,6 +135,19 @@ const StorageVolumeForm: FC = ({ formik, section, setSection }) => { const poolDriver = pools.find((item) => item.name === formik.values.pool)?.driver ?? ""; + const invalidFields: (keyof StorageVolumeFormValues)[] = []; + if (!driversWithFilesystemSupport.includes(poolDriver)) { + invalidFields.push(...getFilesystemVolumeFormFields()); + } + if (poolDriver !== "zfs") { + invalidFields.push(...getZfsVolumeFormFields()); + } + for (const field of invalidFields) { + if (formik.values[field] !== undefined) { + void formik.setFieldValue(field, undefined); + } + } + return (
{/* hidden submit to enable enter key in inputs */} diff --git a/src/pages/storage/forms/StorageVolumeFormBlock.tsx b/src/pages/storage/forms/StorageVolumeFormBlock.tsx index 4cda52eabb..d3e9e2eb83 100644 --- a/src/pages/storage/forms/StorageVolumeFormBlock.tsx +++ b/src/pages/storage/forms/StorageVolumeFormBlock.tsx @@ -18,7 +18,6 @@ const StorageVolumeFormBlock: FC = ({ formik }) => { label: "Block filesystem", name: "block_filesystem", defaultValue: "", - help: "Filesystem of the storage volume", children: ( = ({ formik }) => { formik, label: "Expire after", name: "snapshots_expiry", - help: "Controls when snapshots are to be deleted", defaultValue: "", children: ( = ({ formik }) => { label: "ZFS blocksize", name: "zfs_blocksize", defaultValue: "", - help: "Size of the ZFS blocks", children: ( , }), @@ -70,7 +68,6 @@ const StorageVolumeFormZFS: FC = ({ formik }) => { label: "ZFS delegate", name: "zfs_delegate", defaultValue: "", - help: "Controls whether to delegate the ZFS dataset and anything underneath it to the container(s) using it. Allows the use of the zfs command in the container.", children: , }), @@ -88,7 +84,6 @@ const StorageVolumeFormZFS: FC = ({ formik }) => { label: "ZFS use refquota", name: "zfs_use_refquota", defaultValue: "", - help: "Use refquota instead of quota for space", children: , }), ]} diff --git a/src/types/config.d.ts b/src/types/config.d.ts index ca30411692..eb24a8e0c9 100644 --- a/src/types/config.d.ts +++ b/src/types/config.d.ts @@ -7,6 +7,7 @@ export type ConfigField = LxdConfigOption & { }; export interface LxdConfigOption { + default?: string; defaultdesc?: string; longdesc?: string; scope?: "global" | "local"; @@ -28,5 +29,14 @@ export interface LxdConfigOptions { instance: LxcConfigOptionCategories; project: LxcConfigOptionCategories; server: LxcConfigOptionCategories; + "storage-btrfs": LxcConfigOptionCategories; + "storage-ceph": LxcConfigOptionCategories; + "storage-cephfs": LxcConfigOptionCategories; + "storage-cephobject": LxcConfigOptionCategories; + "storage-dir": LxcConfigOptionCategories; + "storage-lvm": LxcConfigOptionCategories; + "storage-zfs": LxcConfigOptionCategories; }; } + +export type LxdConfigOptionsKeys = keyof LxdConfigOptions["configs"]; diff --git a/src/util/config.spec.ts b/src/util/config.spec.ts index d763f42d21..8c8027e1dd 100644 --- a/src/util/config.spec.ts +++ b/src/util/config.spec.ts @@ -84,6 +84,21 @@ describe("toConfigFields and replaceDocLinks", () => { 'Specify a Pongo2 template string that represents the snapshot name.
This template is used for scheduled snapshots and for unnamed snapshots.

See instance options snapshots names for more information.', ); }); + + it("converts link tag in config description to html link", () => { + const input = + "the value of {config:option}`storage-zfs-volume-conf:zfs.block_mode`,\nthe specified"; + + const result = configDescriptionToHtml( + input, + "https://docs.example.org", + objectsInvTxt.split("\n"), + ); + + expect(result).toBe( + 'the value of zfs.block_mode,
the specified', + ); + }); }); const objectsInvTxt = @@ -93,4 +108,5 @@ const objectsInvTxt = " instance-options-raw Raw instance configuration overrides : reference/instance_options/#instance-options-raw\n" + " instance-options-security Security policies : reference/instance_options/#instance-options-security\n" + " instance-options-snapshots Snapshot scheduling and configuration : reference/instance_options/#instance-options-snapshots\n" + - " instance-options-snapshots-names Automatic snapshot names : reference/instance_options/#instance-options-snapshots-names"; + " instance-options-snapshots-names Automatic snapshot names : reference/instance_options/#instance-options-snapshots-names\n" + + " zfs.block_mode : reference/storage_zfs/#storage-zfs-volume-conf:zfs.block_mode\n"; diff --git a/src/util/config.tsx b/src/util/config.tsx index c016c52fea..fcc1c8e64c 100644 --- a/src/util/config.tsx +++ b/src/util/config.tsx @@ -33,13 +33,12 @@ export const configDescriptionToHtml = ( let result = input .replaceAll("<", "<") .replaceAll(">", ">") - .replaceAll("\n", "
") - .replaceAll("```", ""); + .replaceAll("\n", "
"); // documentation links if (objectsInvTxt) { // tags like {ref}`instance-options-qemu` can be in the input string - const linkTags = input.match(/{(config|ref|config:option)}`[a-z-:.]+`/g); + const linkTags = input.match(/{(config|ref|config:option)}`[a-z-:._]+`/g); linkTags?.map((tag) => { const token = tag .substring(tag.indexOf("`") + 1, tag.lastIndexOf("`")) @@ -68,5 +67,10 @@ export const configDescriptionToHtml = ( result = result.replace("`", "").replace("`", ""); } + // remove invalid placeholders + result = result + .replaceAll("```", "") + .replaceAll("{{snapshot_pattern_detail}}", ""); + return result; }; diff --git a/src/util/configInheritance.spec.ts b/src/util/configInheritance.spec.ts new file mode 100644 index 0000000000..786f6ffdaa --- /dev/null +++ b/src/util/configInheritance.spec.ts @@ -0,0 +1,78 @@ +import { getConfigRowMetadata } from "util/configInheritance"; +import { StoragePoolFormValues } from "pages/storage/forms/StoragePoolForm"; +import { StorageVolumeFormValues } from "pages/storage/forms/StorageVolumeForm"; + +beforeEach(() => { + vi.mock("@tanstack/react-query", () => ({ + useQuery: vi.fn().mockReturnValue({ + data: { + configs: { + "storage-ceph": { + "pool-conf": { + keys: [ + { + "ceph.cluster_name": { + defaultdesc: "`ceph`", + longdesc: "", + shortdesc: + "Name of the Ceph cluster in which to create new storage pools", + type: "string", + }, + }, + ], + }, + }, + "storage-zfs": { + "volume-conf": { + keys: [ + { + "block.filesystem": { + condition: + "block-based volume with content type `filesystem`", + defaultdesc: "same as `volume.block.filesystem`", + longdesc: + "Valid options are: `btrfs`, `ext4`, `xfs`\nIf not set, `ext4` is assumed.", + shortdesc: "File system of the storage volume", + type: "string", + }, + }, + ], + }, + }, + }, + }, + }), + })); +}); + +describe("getConfigRowMetadata", () => { + it("responds with row metadata for a storage pool", () => { + const poolValues = { + driver: "ceph", + entityType: "storagePool", + } as StoragePoolFormValues; + + const result = getConfigRowMetadata(poolValues, "ceph_cluster_name"); + + expect(result.configField?.shortdesc).toBe( + "Name of the Ceph cluster in which to create new storage pools", + ); + expect(result.configField?.default).toBe("ceph"); + }); + + it("responds with row metadata for a storage volume", () => { + const volumeValues = { + entityType: "storageVolume", + } as StorageVolumeFormValues; + + const result = getConfigRowMetadata(volumeValues, "block_filesystem"); + + expect(result.configField?.shortdesc).toBe( + "File system of the storage volume", + ); + expect(result.configField?.longdesc).toBe( + "Valid options are: `btrfs`, `ext4`, `xfs`\n" + + "If not set, `ext4` is assumed.", + ); + }); +}); diff --git a/src/util/configInheritance.tsx b/src/util/configInheritance.tsx index 73a9179239..98d902fcff 100644 --- a/src/util/configInheritance.tsx +++ b/src/util/configInheritance.tsx @@ -17,9 +17,10 @@ import { fetchProfiles } from "api/profiles"; import { getProjectKey } from "util/projectConfigFields"; import { StorageVolumeFormValues } from "pages/storage/forms/StorageVolumeForm"; import { fetchStoragePool } from "api/storage-pools"; -import { getStorageVolumeDefault, getVolumeKey } from "util/storageVolume"; +import { getVolumeKey } from "util/storageVolume"; import { getNetworkDefault } from "util/networks"; -import { getCephStoragePoolDefault } from "./storagePool"; +import { getPoolKey, storagePoolFormDriverToOptionKey } from "./storagePool"; +import { StoragePoolFormValues } from "pages/storage/forms/StoragePoolForm"; export interface ConfigRowMetadata { value?: string; @@ -42,7 +43,7 @@ export const getConfigRowMetadata = ( case "network": return getNetworkRowMetadata(name); case "storagePool": - return getStoragePoolRowMetadata(name); + return getStoragePoolRowMetadata(values, name); } }; @@ -114,7 +115,22 @@ const getStorageVolumeRowMetadata = ( return { value: pool.config[poolField], source: `${pool.name} pool` }; } - return getStorageVolumeDefault(name); + const { data: configOptions } = useQuery({ + queryKey: [queryKeys.configOptions], + queryFn: fetchConfigOptions, + }); + + const optionKey = storagePoolFormDriverToOptionKey(pool?.driver ?? "zfs"); + const configFields = toConfigFields(configOptions?.configs[optionKey] ?? {}); + const configKey = getVolumeKey(name); + const configField = configFields.find((item) => item.key === configKey); + + const lxdDefault = + configField?.default && configField?.default.length > 0 + ? configField?.default + : "-"; + + return { value: lxdDefault, source: "LXD", configField }; }; const getNetworkRowMetadata = (name: string): ConfigRowMetadata => { @@ -122,8 +138,26 @@ const getNetworkRowMetadata = (name: string): ConfigRowMetadata => { }; // NOTE: this is only relevant for Ceph RBD storage pools at the moment -const getStoragePoolRowMetadata = (name: string): ConfigRowMetadata => { - return getCephStoragePoolDefault(name); +const getStoragePoolRowMetadata = ( + values: StoragePoolFormValues, + name: string, +): ConfigRowMetadata => { + const { data: configOptions } = useQuery({ + queryKey: [queryKeys.configOptions], + queryFn: fetchConfigOptions, + }); + + const optionKey = storagePoolFormDriverToOptionKey(values.driver); + const configFields = toConfigFields(configOptions?.configs[optionKey] ?? {}); + const configKey = getPoolKey(name); + const configField = configFields.find((item) => item.key === configKey); + + const lxdDefault = + configField?.default && configField?.default.length > 0 + ? configField?.default + : "-"; + + return { value: lxdDefault, source: "LXD", configField }; }; const getInstanceProfileProjectDefaults = ( diff --git a/src/util/storagePool.tsx b/src/util/storagePool.tsx index 45c36887f3..a250b2486d 100644 --- a/src/util/storagePool.tsx +++ b/src/util/storagePool.tsx @@ -1,25 +1,8 @@ import { AbortControllerState, checkDuplicateName } from "util/helpers"; import { AnyObject, TestFunction } from "yup"; -import { ConfigRowMetadata } from "./configInheritance"; +import { LxdConfigOptionsKeys } from "types/config"; -export const cephStoragePoolDefaults: Record = { - ceph_cluster_name: "ceph", - ceph_osd_pg_num: "32", - ceph_rbd_clone_copy: "true", - ceph_user_name: "admin", - ceph_rbd_features: "layering", -}; - -export const getCephStoragePoolDefault = ( - formField: string, -): ConfigRowMetadata => { - if (formField in cephStoragePoolDefaults) { - return { value: cephStoragePoolDefaults[formField], source: "LXD" }; - } - return { value: "", source: "LXD" }; -}; - -const cephStoragePoolFormFieldToPayloadName: Record = { +export const storagePoolFormFieldToPayloadName: Record = { ceph_cluster_name: "ceph.cluster_name", ceph_osd_pg_num: "ceph.osd.pg_num", ceph_rbd_clone_copy: "ceph.rbd.clone_copy", @@ -27,11 +10,36 @@ const cephStoragePoolFormFieldToPayloadName: Record = { ceph_rbd_features: "ceph.rbd.features", }; -export const getCephConfigKey = (key: string): string => { - if (key in cephStoragePoolFormFieldToPayloadName) { - return cephStoragePoolFormFieldToPayloadName[key]; +export const getPoolKey = (formField: string): string => { + if (!(formField in storagePoolFormFieldToPayloadName)) { + throw new Error( + `Could not find ${formField} in storagePoolFormFieldToPayloadName`, + ); + } + return storagePoolFormFieldToPayloadName[formField]; +}; + +export const getCephPoolFormFields = () => { + return Object.keys(storagePoolFormFieldToPayloadName).filter((item) => + item.startsWith("ceph_"), + ); +}; + +const storagePoolDriverToOptionKey: Record = { + dir: "storage-dir", + btrfs: "storage-btrfs", + lvm: "storage-lvm", + zfs: "storage-zfs", + ceph: "storage-ceph", +}; + +export const storagePoolFormDriverToOptionKey = ( + driver: string, +): LxdConfigOptionsKeys => { + if (!(driver in storagePoolDriverToOptionKey)) { + throw new Error(`Could not find ${driver} in storagePoolDriverToOptionKey`); } - return key; + return storagePoolDriverToOptionKey[driver]; }; export const testDuplicateStoragePoolName = ( diff --git a/src/util/storageVolume.tsx b/src/util/storageVolume.tsx index 9c0c8efe5d..7133e14a13 100644 --- a/src/util/storageVolume.tsx +++ b/src/util/storageVolume.tsx @@ -6,7 +6,6 @@ import { import { AnyObject, TestContext, TestFunction } from "yup"; import { LxdStorageVolume } from "types/storage"; import { StorageVolumeFormValues } from "pages/storage/forms/StorageVolumeForm"; -import { ConfigRowMetadata } from "util/configInheritance"; export const testDuplicateStorageVolumeName = ( project: string, @@ -48,36 +47,23 @@ const storageVolumeFormFieldToPayloadName: Record = { zfs_reserve_space: "zfs.reserve_space", }; -export const getVolumeKey = (key: string): string => { - if (Object.keys(storageVolumeFormFieldToPayloadName).includes(key)) { - return storageVolumeFormFieldToPayloadName[key]; - } - return key; +export const getFilesystemVolumeFormFields = () => { + return Object.keys(storageVolumeFormFieldToPayloadName).filter((item) => + item.startsWith("block_"), + ) as (keyof StorageVolumeFormValues)[]; }; -const storageVolumeDefaults: Record = { - security_shifted: "false", - security_unmapped: "false", - snapshots_expiry: "-", - snapshots_pattern: "snap%d", - snapshots_schedule: "-", - block_filesystem: "auto", - block_mount_options: "-", - zfs_blocksize: "-", - zfs_block_mode: "-", - zfs_delegate: "false", - zfs_remove_snapshots: "false", - zfs_use_refquota: "false", - zfs_reserve_space: "false", +export const getZfsVolumeFormFields = () => { + return Object.keys(storageVolumeFormFieldToPayloadName).filter((item) => + item.startsWith("zfs_"), + ) as (keyof StorageVolumeFormValues)[]; }; -export const getStorageVolumeDefault = ( - formField: string, -): ConfigRowMetadata => { - if (Object.keys(storageVolumeDefaults).includes(formField)) { - return { value: storageVolumeDefaults[formField], source: "LXD" }; +export const getVolumeKey = (key: string): string => { + if (Object.keys(storageVolumeFormFieldToPayloadName).includes(key)) { + return storageVolumeFormFieldToPayloadName[key]; } - return { value: "", source: "LXD" }; + return key; }; export const renderVolumeType = (volume: LxdStorageVolume) => { diff --git a/tests/storage.spec.ts b/tests/storage.spec.ts index aff07ac15e..2c163a3db3 100644 --- a/tests/storage.spec.ts +++ b/tests/storage.spec.ts @@ -62,7 +62,7 @@ test("storage volume edit snapshot configuration", async ({ page }) => { "snap123", ); await setInput(page, "Expire after", "Enter expiry expression", "3m"); - await activateOverride(page, "Schedule"); + await activateOverride(page, "Schedule Schedule for automatic"); await page.getByPlaceholder("Enter cron expression").last().fill("@daily"); await page.getByRole("button", { name: "Save" }).click(); await page.waitForSelector(`text=Configuration updated.`, TIMEOUT);