Skip to content

fix(storage pool): rely on operation when api extension is present #1899

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.

Sign up for GitHub

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

Open
kimanhou wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(storage pool): rely on operation when api extension is present #1899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 46 additions & 1 deletion src/api/operations.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { handleResponse } from "util/helpers";
import { handleResponse, LxdApiError } from "util/helpers";
import type { LxdOperation, LxdOperationList } from "types/operation";
import type { LxdApiResponse } from "types/apiResponse";
import { ROOT_PATH } from "util/rootPath";
Expand Down Expand Up @@ -38,3 +38,48 @@ export const cancelOperation = async (id: string): Promise<void> => {
method: "DELETE",
}).then(handleResponse);
};

export const waitForOperation = async (
id: string,
member?: string,
maxTimeoutMs = 120000,
): Promise<void> => {
const endpoint = `${ROOT_PATH}/1.0/operations/${encodeURIComponent(id)}`;
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

waitForOperation() accepts a member argument but it isn’t used to target the correct cluster member when polling. For operations created via requests with ?target=<member>, polling without the same target is likely to 404 or observe the wrong operation. Build the endpoint with URLSearchParams + addTarget(params, member) (or accept a target param) and include it in the fetch URL.

Suggested change
const endpoint = `${ROOT_PATH}/1.0/operations/${encodeURIComponent(id)}`;
const params = new URLSearchParams();
if (member) {
params.set("target", member);
}
const endpoint = `${ROOT_PATH}/1.0/operations/${encodeURIComponent(id)}${
params.toString() ? `?${params.toString()}` : ""
}`;

Copilot uses AI. Check for mistakes.
const startTime = Date.now();
let delay = 500;

while (true) {
if (Date.now() - startTime > maxTimeoutMs) {
const memberPrefix = member ? `[Member: ${member}] ` : "";
throw new Error(`${memberPrefix}Operation timed out.`);
}

try {
const response = (await fetch(endpoint).then(
handleResponse,
)) as LxdApiResponse<LxdOperation>;
Comment on lines +42 to +60
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

waitForOperation() accepts a member argument but the request URL is always /1.0/operations/<id> with no cluster target. In clustered setups, operation lookups are often member-targeted, so this can poll the wrong node and either time out or 404 even though the operation exists on the specified member. Consider adding ?target=<member> when member is provided (reusing addTarget), or remove the member parameter if it’s only meant for error-message prefixing.

Copilot uses AI. Check for mistakes.
const operation = response.metadata;

if (operation.status_code === 200) {
return;
}

if (operation.status_code >= 400) {
throw new Error(
operation.err ||
`Operation ${id} failed with status ${operation.status}`,
);
}
} catch (error) {
if (error instanceof LxdApiError && error.status === 404) {
const memberPrefix = member ? `[Member: ${member}] ` : "";
throw new Error(`${memberPrefix}Operation not found on server.`);
}
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

The catch block swallows most errors (including the Error thrown when status_code >= 400), so failed operations will keep polling until timeout instead of failing fast. Re-throw non-404 errors (and/or explicitly handle the operation-failure error) so callers get an immediate failure when the operation transitions to a failure state.

Suggested change
}
}
throw error;

Copilot uses AI. Check for mistakes.
// Re-throw all other errors
throw error;
}

await new Promise((resolve) => setTimeout(resolve, delay));
delay = Math.min(delay * 1.5, 5000);
}
};
127 changes: 95 additions & 32 deletions src/api/storage-pools.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ import type {
import type { LxdApiResponse } from "types/apiResponse";
import type { LxdClusterMember } from "types/cluster";
import type { ClusterSpecificValues } from "types/cluster";
import type { LxdOperationResponse } from "types/operation";
import { addEntitlements } from "util/entitlements/api";
import { addTarget } from "util/target";
import { ROOT_PATH } from "util/rootPath";
import { waitForOperation } from "api/operations";

export const storagePoolEntitlements = ["can_edit", "can_delete"];

Expand Down Expand Up @@ -101,17 +103,21 @@ export const fetchClusteredStoragePoolResources = async (
export const createPool = async (
pool: Partial<LxdStoragePool>,
target?: string,
): Promise<void> => {
): Promise<LxdOperationResponse> => {
const params = new URLSearchParams();
addTarget(params, target);

await fetch(`${ROOT_PATH}/1.0/storage-pools?${params.toString()}`, {
return fetch(`${ROOT_PATH}/1.0/storage-pools?${params.toString()}`, {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify(pool),
}).then(handleResponse);
})
.then(handleResponse)
.then((data: LxdOperationResponse) => {
return data;
});
};

const getClusterAndMemberPoolPayload = (pool: LxdStoragePool) => {
Expand Down Expand Up @@ -149,10 +155,11 @@ export const createClusteredPool = async (
sourcePerClusterMember?: ClusterSpecificValues,
zfsPoolNamePerClusterMember?: ClusterSpecificValues,
sizePerClusterMember?: ClusterSpecificValues,
): Promise<void> => {
): Promise<LxdOperationResponse> => {
const { memberPoolPayload, clusterPoolPayload } =
getClusterAndMemberPoolPayload(pool);
return Promise.allSettled(

const results = await Promise.allSettled(
clusterMembers.map(async (item) => {
const clusteredMemberPool = {
...memberPoolPayload,
Expand All @@ -163,23 +170,41 @@ export const createClusteredPool = async (
"zfs.pool_name": zfsPoolNamePerClusterMember?.[item.server_name],
},
};
return createPool(clusteredMemberPool, item.server_name);
const operation = await createPool(clusteredMemberPool, item.server_name);
return { operation, member: item.server_name };
}),
)
.then(handleSettledResult)
.then(async () => {
return createPool(clusterPoolPayload);
});
);

handleSettledResult(results);

const operationsWithMembers = results
.filter(
(
res,
): res is PromiseFulfilledResult<{
operation: LxdOperationResponse;
member: string;
}> => res.status === "fulfilled",
)
.map((res) => res.value);

await Promise.all(
operationsWithMembers.map(async ({ operation, member }) => {
await waitForOperation(operation.metadata.id, member);
}),
);

return createPool(clusterPoolPayload);
};

export const updatePool = async (
pool: LxdStoragePool,
target?: string,
): Promise<void> => {
): Promise<LxdOperationResponse> => {
const params = new URLSearchParams();
addTarget(params, target);

await fetch(
return fetch(
`${ROOT_PATH}/1.0/storage-pools/${encodeURIComponent(pool.name)}?${params.toString()}`,
{
method: "PATCH",
Expand All @@ -188,7 +213,11 @@ export const updatePool = async (
},
body: JSON.stringify(pool),
},
).then(handleResponse);
)
.then(handleResponse)
.then((data: LxdOperationResponse) => {
return data;
});
};

export const updateClusteredPool = async (
Expand All @@ -197,10 +226,11 @@ export const updateClusteredPool = async (
sourcePerClusterMember?: ClusterSpecificValues,
zfsPoolNamePerClusterMember?: ClusterSpecificValues,
sizePerClusterMember?: ClusterSpecificValues,
): Promise<void> => {
): Promise<LxdOperationResponse> => {
const { memberPoolPayload, clusterPoolPayload } =
getClusterAndMemberPoolPayload(pool);
return Promise.allSettled(

const results = await Promise.allSettled(
clusterMembers.map(async (item) => {
const clusteredMemberPool = {
...memberPoolPayload,
Expand All @@ -220,32 +250,65 @@ export const updateClusteredPool = async (
clusteredMemberPool.config["zfs.pool_name"] =
zfsPoolNamePerClusterMember[item.server_name];
}
return updatePool(clusteredMemberPool, item.server_name);
const operation = await updatePool(clusteredMemberPool, item.server_name);
return { operation, member: item.server_name };
}),
)
.then(handleSettledResult)
.then(async () => updatePool(clusterPoolPayload));
);

handleSettledResult(results);

const operationsWithMembers = results
.filter(
(
res,
): res is PromiseFulfilledResult<{
operation: LxdOperationResponse;
member: string;
}> => res.status === "fulfilled",
)
.map((res) => res.value);

await Promise.all(
operationsWithMembers.map(async ({ operation, member }) => {
await waitForOperation(operation.metadata.id, member);
}),
);

return updatePool(clusterPoolPayload);
};
Comment on lines 223 to 278
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

updateClusteredPool() kicks off per-member updatePool(..., item.server_name) calls and then immediately proceeds to update the cluster-wide payload, but it never waits for the per-member operations to complete. With storage_and_network_operations, those member updates can be asynchronous, so this function may resolve (and the UI may show success for the returned operation) while some member updates are still running or may later fail. Consider collecting the per-member LxdOperationResponses and waiting for them to complete (similar to createClusteredPool()) before returning the final operation.

Copilot uses AI. Check for mistakes.

export const renameStoragePool = async (
oldName: string,
newName: string,
): Promise<void> => {
await fetch(`${ROOT_PATH}/1.0/storage-pools/${encodeURIComponent(oldName)}`, {
method: "POST",
headers: {
"Content-Type": "application/json",
): Promise<LxdOperationResponse> => {
return fetch(
`${ROOT_PATH}/1.0/storage-pools/${encodeURIComponent(oldName)}`,
{
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({
name: newName,
}),
},
body: JSON.stringify({
name: newName,
}),
}).then(handleResponse);
)
.then(handleResponse)
.then((data: LxdOperationResponse) => {
return data;
});
};

export const deleteStoragePool = async (pool: string): Promise<void> => {
await fetch(`${ROOT_PATH}/1.0/storage-pools/${encodeURIComponent(pool)}`, {
export const deleteStoragePool = async (
pool: string,
): Promise<LxdOperationResponse> => {
return fetch(`${ROOT_PATH}/1.0/storage-pools/${encodeURIComponent(pool)}`, {
method: "DELETE",
}).then(handleResponse);
})
.then(handleResponse)
.then((data: LxdOperationResponse) => {
return data;
});
};

export const fetchPoolFromClusterMembers = async (
Expand Down
3 changes: 3 additions & 0 deletions src/context/useSupportedFeatures.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,8 @@ export const useSupportedFeatures = () => {
hasProjectDeleteOperation: apiExtensions.has("project_delete_operation"),
hasRemoteDropSource: apiExtensions.has("storage_remote_drop_source"),
hasClusteringControlPlane: apiExtensions.has("clustering_control_plane"),
hasStorageAndNetworkOperations: apiExtensions.has(
"storage_and_network_operations",
),
};
};
73 changes: 54 additions & 19 deletions src/pages/storage/CreateStoragePool.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import YamlSwitch from "components/forms/YamlSwitch";
import StoragePoolRichChip from "./StoragePoolRichChip";
import { ROOT_PATH } from "util/rootPath";
import { useSupportedFeatures } from "context/useSupportedFeatures";
import { useEventQueue } from "context/eventQueue";

const CreateStoragePool: FC = () => {
const navigate = useNavigate();
Expand All @@ -43,7 +44,9 @@ const CreateStoragePool: FC = () => {
const [section, setSection] = useState(slugify(MAIN_CONFIGURATION));
const controllerState = useState<AbortController | null>(null);
const { data: clusterMembers = [] } = useClusterMembers();
const { hasRemoteDropSource } = useSupportedFeatures();
const { hasRemoteDropSource, hasStorageAndNetworkOperations } =
useSupportedFeatures();
const eventQueue = useEventQueue();

if (!project) {
return <>Missing project</>;
Expand All @@ -55,6 +58,27 @@ const CreateStoragePool: FC = () => {
.required("This field is required"),
});

const notifySuccess = (poolName: string) => {
toastNotify.success(
<>
Storage pool{" "}
<StoragePoolRichChip poolName={poolName} projectName={project} />{" "}
created.
</>,
);
};

const onSuccess = (storagePoolName: string) => {
queryClient.invalidateQueries({
queryKey: [queryKeys.storage],
});
navigate(
`${ROOT_PATH}/ui/project/${encodeURIComponent(project)}/storage/pools`,
);
formik.setSubmitting(false);
notifySuccess(storagePoolName);
};

const formik = useFormik<StoragePoolFormValues>({
initialValues: {
isCreating: true,
Expand Down Expand Up @@ -85,27 +109,38 @@ const CreateStoragePool: FC = () => {
: async () => createPool(storagePool);

mutation()
.then(() => {
queryClient.invalidateQueries({
queryKey: [queryKeys.storage],
});
navigate(
`${ROOT_PATH}/ui/project/${encodeURIComponent(project)}/storage/pools`,
);
toastNotify.success(
<>
Storage pool{" "}
<StoragePoolRichChip
poolName={storagePool.name}
projectName={project}
/>{" "}
created.
</>,
);
.then((operation) => {
if (hasStorageAndNetworkOperations) {
toastNotify.info(
<>
Creation of storage pool{" "}
<StoragePoolRichChip
poolName={storagePool.name}
projectName={project}
/>{" "}
has started.
</>,
);
eventQueue.set(
operation.metadata.id,
() => {
onSuccess(storagePool.name);
},
(msg) => {
formik.setSubmitting(false);
toastNotify.failure(
`Creation of storage pool ${storagePool.name} failed`,
new Error(msg),
);
},
);
} else {
onSuccess(storagePool.name);
}
})
.catch((e) => {
formik.setSubmitting(false);
notify.failure("Storage pool creation failed", e);
notify.failure("Creation of storage pool failed", e);
});
},
});
Expand Down
Loading
Loading