iscsi still failing to save current attribute, check on disable and enable portal/iscsi targets

frontend/src/pages/ISCSITargetDetail.tsx

@@ -1,9 +1,9 @@
 import { useParams, useNavigate } from 'react-router-dom'
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'
-import { scstAPI, type SCSTHandler } from '@/api/scst'
+import { scstAPI, type SCSTTarget, type SCSTExtent } from '@/api/scst'
 import { Card, CardContent, CardHeader, CardTitle, CardDescription } from '@/components/ui/card'
 import { Button } from '@/components/ui/button'
-import { ArrowLeft, Plus, RefreshCw, HardDrive, Users } from 'lucide-react'
+import { ArrowLeft, Plus, RefreshCw, HardDrive, Users, Trash2 } from 'lucide-react'
 import { useState, useEffect } from 'react'
 
 export default function ISCSITargetDetail() {
@@ -23,11 +23,64 @@ export default function ISCSITargetDetail() {
     enabled: !!id,
   })
 
-  const { data: handlers } = useQuery<SCSTHandler[]>({
-    queryKey: ['scst-handlers'],
-    queryFn: scstAPI.listHandlers,
-    staleTime: 0, // Always fetch fresh data
-    refetchOnMount: true,
+  const removeLUNMutation = useMutation({
+    mutationFn: ({ targetId, lunId }: { targetId: string; lunId: string }) => 
+      scstAPI.removeLUN(targetId, lunId),
+    onMutate: async ({ lunId }) => {
+      // Cancel any outgoing refetches
+      await queryClient.cancelQueries({ queryKey: ['scst-target', id] })
+      await queryClient.cancelQueries({ queryKey: ['scst-targets'] })
+
+      // Snapshot the previous value
+      const previousTarget = queryClient.getQueryData(['scst-target', id])
+      const previousTargets = queryClient.getQueryData<SCSTTarget[]>(['scst-targets'])
+
+      // Optimistically update to remove the LUN
+      queryClient.setQueryData(['scst-target', id], (old: any) => {
+        if (!old) return old
+        return {
+          ...old,
+          luns: old.luns ? old.luns.filter((lun: any) => lun.id !== lunId) : []
+        }
+      })
+
+      // Optimistically update LUN count in targets list
+      queryClient.setQueryData<SCSTTarget[]>(['scst-targets'], (old) => {
+        if (!old) return old
+        return old.map(t => 
+          t.id === id 
+            ? { ...t, lun_count: Math.max(0, (t.lun_count || 0) - 1) }
+            : t
+        )
+      })
+
+      return { previousTarget, previousTargets }
+    },
+    onSuccess: () => {
+      // Invalidate queries to refetch data from the server.
+      // This is simpler and less prone to race conditions than the previous implementation.
+      queryClient.invalidateQueries({ queryKey: ['scst-target', id] });
+      queryClient.invalidateQueries({ queryKey: ['scst-targets'] });
+    },
+    onError: (error: any, _variables, context) => {
+      // If 404, treat as success (LUN already deleted)
+      if (error.response?.status === 404) {
+        // LUN already deleted, just refresh to sync UI
+        queryClient.invalidateQueries({ queryKey: ['scst-target', id] });
+        queryClient.invalidateQueries({ queryKey: ['scst-targets'] });
+        return
+      }
+      
+      // Rollback optimistic update
+      if (context?.previousTarget) {
+        queryClient.setQueryData(['scst-target', id], context.previousTarget)
+      }
+      if (context?.previousTargets) {
+        queryClient.setQueryData<SCSTTarget[]>(['scst-targets'], context.previousTargets)
+      }
+      
+      alert(`Failed to remove LUN: ${error.response?.data?.error || error.message}`)
+    },
   })
 
   if (isLoading) {
@@ -124,7 +177,7 @@ export default function ISCSITargetDetail() {
                 onClick={() => setShowAddLUN(true)}
               >
                 <Plus className="h-4 w-4 mr-2" />
-                Add LUN
+                Assign Extent
               </Button>
               <Button
                 variant="outline"
@@ -153,12 +206,11 @@ export default function ISCSITargetDetail() {
               size="sm" 
               onClick={(e) => {
                 e.stopPropagation()
-                console.log('Add LUN button clicked, setting showAddLUN to true')
                 setShowAddLUN(true)
               }}
             >
               <Plus className="h-4 w-4 mr-2" />
-              Add LUN
+              Assign Extent
             </Button>
           </div>
         </CardHeader>
@@ -183,6 +235,9 @@ export default function ISCSITargetDetail() {
                     <th className="px-6 py-3 text-left text-xs font-medium text-text-secondary uppercase">
                       Status
                     </th>
+                    <th className="px-6 py-3 text-right text-xs font-medium text-text-secondary uppercase">
+                      Actions
+                    </th>
                   </tr>
                 </thead>
                 <tbody className="bg-card-dark divide-y divide-border-dark">
@@ -211,6 +266,21 @@ export default function ISCSITargetDetail() {
                           {lun.is_active ? 'Active' : 'Inactive'}
                         </span>
                       </td>
+                      <td className="px-6 py-4 whitespace-nowrap text-right text-sm">
+                        <button
+                          onClick={(e) => {
+                            e.stopPropagation()
+                            if (confirm(`Remove LUN ${lun.lun_number} from this target?`)) {
+                              removeLUNMutation.mutate({ targetId: target.id, lunId: lun.id })
+                            }
+                          }}
+                          disabled={removeLUNMutation.isPending}
+                          className="p-1.5 hover:bg-red-500/10 rounded text-text-secondary hover:text-red-400 transition-colors disabled:opacity-50"
+                          title="Remove LUN"
+                        >
+                          <Trash2 size={16} />
+                        </button>
+                      </td>
                     </tr>
                   ))}
                 </tbody>
@@ -224,27 +294,28 @@ export default function ISCSITargetDetail() {
                 variant="outline" 
                 onClick={(e) => {
                   e.stopPropagation()
-                  console.log('Add First LUN button clicked, setting showAddLUN to true')
                   setShowAddLUN(true)
                 }}
               >
                 <Plus className="h-4 w-4 mr-2" />
-                Add First LUN
+                Assign First Extent
               </Button>
             </div>
           )}
         </CardContent>
       </Card>
 
-      {/* Add LUN Form */}
+      {/* Assign Extent Form */}
       {showAddLUN && (
-        <AddLUNForm
+        <AssignExtentForm
           targetId={target.id}
-          handlers={handlers || []}
           onClose={() => setShowAddLUN(false)}
-          onSuccess={() => {
+          onSuccess={async () => {
             setShowAddLUN(false)
-            queryClient.invalidateQueries({ queryKey: ['scst-target', id] })
+            // Invalidate queries to refetch data.
+            // Invalidate extents since one is now in use.
+            queryClient.invalidateQueries({ queryKey: ['scst-target', id] });
+            queryClient.invalidateQueries({ queryKey: ['scst-extents'] });
           }}
         />
       )}
@@ -264,47 +335,56 @@ export default function ISCSITargetDetail() {
   )
 }
 
-interface AddLUNFormProps {
+interface AssignExtentFormProps {
   targetId: string
-  handlers: SCSTHandler[]
   onClose: () => void
-  onSuccess: () => void
+  onSuccess: () => Promise<void>
 }
 
-function AddLUNForm({ targetId, handlers, onClose, onSuccess }: AddLUNFormProps) {
-  const [handlerType, setHandlerType] = useState('')
-  const [devicePath, setDevicePath] = useState('')
-  const [deviceName, setDeviceName] = useState('')
+function AssignExtentForm({ targetId, onClose, onSuccess }: AssignExtentFormProps) {
+  const [selectedExtent, setSelectedExtent] = useState('')
   const [lunNumber, setLunNumber] = useState(0)
 
-  useEffect(() => {
-    console.log('AddLUNForm mounted, targetId:', targetId, 'handlers:', handlers)
-  }, [targetId, handlers])
+  // Fetch available extents
+  const { data: extents = [], isLoading: extentsLoading } = useQuery<SCSTExtent[]>({
+    queryKey: ['scst-extents'],
+    queryFn: scstAPI.listExtents,
+    staleTime: 0,
+    refetchOnMount: true,
+  })
+
+  // Filter only extents that are not in use
+  const availableExtents = extents.filter(extent => !extent.is_in_use)
 
   const addLUNMutation = useMutation({
     mutationFn: (data: { device_name: string; device_path: string; lun_number: number; handler_type: string }) =>
       scstAPI.addLUN(targetId, data),
-    onSuccess: () => {
-      onSuccess()
+    onSuccess: async () => {
+      await onSuccess()
     },
     onError: (error: any) => {
-      console.error('Failed to add LUN:', error)
-      const errorMessage = error.response?.data?.error || error.message || 'Failed to add LUN'
+      const errorMessage = error.response?.data?.error || error.message || 'Failed to assign extent'
       alert(errorMessage)
     },
   })
 
   const handleSubmit = (e: React.FormEvent) => {
     e.preventDefault()
-    if (!handlerType || !devicePath || !deviceName || lunNumber < 0) {
-      alert('All fields are required')
+    if (!selectedExtent || lunNumber < 0) {
+      alert('Please select an extent and specify LUN number')
+      return
+    }
+
+    const extent = availableExtents.find(e => e.device_name === selectedExtent)
+    if (!extent) {
+      alert('Selected extent not found')
       return
     }
 
     addLUNMutation.mutate({
-      handler_type: handlerType.trim(),
-      device_path: devicePath.trim(),
-      device_name: deviceName.trim(),
+      device_name: extent.device_name,
+      device_path: extent.device_path,
+      handler_type: extent.handler_type,
       lun_number: lunNumber,
     })
   }
@@ -313,74 +393,68 @@ function AddLUNForm({ targetId, handlers, onClose, onSuccess }: AddLUNFormProps)
     <div className="fixed inset-0 bg-black/50 z-50 flex items-center justify-center p-4">
       <div className="bg-card-dark border border-border-dark rounded-xl max-w-2xl w-full max-h-[90vh] overflow-y-auto">
         <div className="p-6 border-b border-border-dark">
-          <h2 className="text-xl font-bold text-white">Add LUN</h2>
-          <p className="text-sm text-text-secondary mt-1">Bind a ZFS volume or storage device to this target</p>
+          <h2 className="text-xl font-bold text-white">Assign Extent</h2>
+          <p className="text-sm text-text-secondary mt-1">Assign an existing extent to this target as a LUN</p>
         </div>
         <form onSubmit={handleSubmit} className="p-6 space-y-4">
           <div>
-            <label htmlFor="handlerType" className="block text-sm font-medium text-white mb-1">
-              Handler Type *
+            <label htmlFor="extent" className="block text-sm font-medium text-white mb-1">
+              Available Extent *
             </label>
-            <select
-              id="handlerType"
-              value={handlerType}
-              onChange={(e) => setHandlerType(e.target.value)}
-              className="w-full px-3 py-2 bg-[#0f161d] border border-border-dark rounded-lg text-white text-sm focus:outline-none focus:border-primary focus:ring-1 focus:ring-primary"
-              required
-            >
-              <option value="">Select a handler</option>
-              {handlers.map((h) => (
-                <option key={h.name} value={h.name}>
-                  {h.label || h.name}
-                </option>
-              ))}
-            </select>
-          </div>
-
-          <div>
-            <label htmlFor="devicePath" className="block text-sm font-medium text-white mb-1">
-              ZFS Volume Path *
-            </label>
-            <input
-              id="devicePath"
-              type="text"
-              value={devicePath}
-              onChange={(e) => {
-                const path = e.target.value.trim()
-                setDevicePath(path)
-                // Auto-generate device name from path (e.g., /dev/zvol/pool/volume -> volume)
-                if (path && !deviceName) {
-                  const parts = path.split('/')
-                  const name = parts[parts.length - 1] || parts[parts.length - 2] || 'device'
-                  setDeviceName(name)
-                }
-              }}
-              placeholder="/dev/zvol/pool/volume or /dev/sda"
-              className="w-full px-3 py-2 bg-[#0f161d] border border-border-dark rounded-lg text-white text-sm focus:outline-none focus:border-primary focus:ring-1 focus:ring-primary font-mono"
-              required
-            />
+            {extentsLoading ? (
+              <div className="w-full px-3 py-2 bg-[#0f161d] border border-border-dark rounded-lg text-text-secondary text-sm">
+                Loading extents...
+              </div>
+            ) : availableExtents.length === 0 ? (
+              <div className="w-full px-3 py-2 bg-[#0f161d] border border-border-dark rounded-lg text-text-secondary text-sm">
+                No available extents. Please create an extent first in the Extents tab.
+              </div>
+            ) : (
+              <select
+                id="extent"
+                value={selectedExtent}
+                onChange={(e) => setSelectedExtent(e.target.value)}
+                className="w-full px-3 py-2 bg-[#0f161d] border border-border-dark rounded-lg text-white text-sm focus:outline-none focus:border-primary focus:ring-1 focus:ring-primary"
+                required
+              >
+                <option value="">Select an extent...</option>
+                {availableExtents.map((extent) => (
+                  <option key={extent.device_name} value={extent.device_name}>
+                    {extent.device_name} ({extent.handler_type}) - {extent.device_path}
+                  </option>
+                ))}
+              </select>
+            )}
             <p className="mt-1 text-xs text-text-secondary">
-              Enter ZFS volume path (e.g., /dev/zvol/pool/volume) or block device path
+              Select an extent that has been created in the Extents tab
             </p>
           </div>
 
-          <div>
-            <label htmlFor="deviceName" className="block text-sm font-medium text-white mb-1">
-              Device Name *
-            </label>
-            <input
-              id="deviceName"
-              type="text"
-              value={deviceName}
-              onChange={(e) => setDeviceName(e.target.value)}
-              placeholder="device1"
-              className="w-full px-3 py-2 bg-[#0f161d] border border-border-dark rounded-lg text-white text-sm focus:outline-none focus:border-primary focus:ring-1 focus:ring-primary"
-              required
-            />
-            <p className="mt-1 text-xs text-text-secondary">
-              Logical name for this device in SCST (auto-filled from volume path)
-            </p>
-          </div>
+          {selectedExtent && (
+            <div className="p-4 bg-[#0f161d] border border-border-dark rounded-lg">
+              <p className="text-sm text-text-secondary mb-2">Extent Details:</p>
+              {(() => {
+                const extent = availableExtents.find(e => e.device_name === selectedExtent)
+                if (!extent) return null
+                return (
+                  <div className="space-y-1 text-sm">
+                    <div className="flex justify-between">
+                      <span className="text-text-secondary">Device Name:</span>
+                      <span className="text-white font-mono">{extent.device_name}</span>
+                    </div>
+                    <div className="flex justify-between">
+                      <span className="text-text-secondary">Handler:</span>
+                      <span className="text-white">{extent.handler_type}</span>
+                    </div>
+                    <div className="flex justify-between">
+                      <span className="text-text-secondary">Path:</span>
+                      <span className="text-white font-mono text-xs">{extent.device_path}</span>
+                    </div>
+                  </div>
+                )
+              })()}
+            </div>
+          )}
 
           <div>
             <label htmlFor="lunNumber" className="block text-sm font-medium text-white mb-1">
@@ -392,6 +466,7 @@ function AddLUNForm({ targetId, handlers, onClose, onSuccess }: AddLUNFormProps)
               value={lunNumber}
               onChange={(e) => setLunNumber(parseInt(e.target.value) || 0)}
               min="0"
+              max="255"
               className="w-full px-3 py-2 bg-[#0f161d] border border-border-dark rounded-lg text-white text-sm focus:outline-none focus:border-primary focus:ring-1 focus:ring-primary"
               required
             />
@@ -404,8 +479,11 @@ function AddLUNForm({ targetId, handlers, onClose, onSuccess }: AddLUNFormProps)
             <Button type="button" variant="outline" onClick={onClose}>
               Cancel
             </Button>
-            <Button type="submit" disabled={addLUNMutation.isPending}>
-              {addLUNMutation.isPending ? 'Adding...' : 'Add LUN'}
+            <Button 
+              type="submit" 
+              disabled={addLUNMutation.isPending || availableExtents.length === 0}
+            >
+              {addLUNMutation.isPending ? 'Assigning...' : 'Assign Extent'}
             </Button>
           </div>
         </form>