feat: implement cmd management for targets, LUNs, and TPGTs (fixes #36)

- Fix target delete URL path mismatch (/targets/ -> /target/)
- Implement target create/delete server handlers with proper validation
- Add DeleteTarget method with force flag and mutex locking to SCSITargetService
- Implement full LU management: create/list/delete through CLI, client, and server
- Add TPGT list command to show target portal group tags
- Add unit tests for target/LU router handlers and SCSI service

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

pkg/apiserver/router/lu/lu.go

@@ -16,25 +16,31 @@ limitations under the License.
 package lu
 
 import (
+	"encoding/json"
+	"fmt"
 	"net/http"
 
+	"github.com/gostor/gotgt/pkg/api"
+	"github.com/gostor/gotgt/pkg/apiserver/httputils"
 	"github.com/gostor/gotgt/pkg/apiserver/router"
+	"github.com/gostor/gotgt/pkg/config"
+	"github.com/gostor/gotgt/pkg/scsi"
 	"golang.org/x/net/context"
 )
 
-// containerRouter is a router to talk with the container controller
+// luRouter is a router to talk with the LU controller
 type luRouter struct {
 	routes []router.Route
 }
 
-// NewRouter initializes a new container router
+// NewRouter initializes a new LU router
 func NewRouter() router.Router {
 	r := &luRouter{}
 	r.initRoutes()
 	return r
 }
 
-// Routes returns the available routers to the container controller
+// Routes returns the available routers to the LU controller
 func (r *luRouter) Routes() []router.Route {
 	return r.routes
 }
@@ -43,23 +49,102 @@ func (r *luRouter) Routes() []router.Route {
 func (r *luRouter) initRoutes() {
 	r.routes = []router.Route{
 		// GET
+		router.NewGetRoute("/lu/list", r.getLuList),
 		router.NewGetRoute("/lu/{id:.*}", r.getLu),
 		// POST
 		router.NewPostRoute("/lu/create", r.postLuCreate),
-		// PUT
 		// DELETE
-		router.NewDeleteRoute("/lu/{id:.*}", r.deleteLu),
+		router.NewDeleteRoute("/lu/delete", r.deleteLu),
 	}
 }
 
-func (s *luRouter) getLu(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
+func (r *luRouter) getLuList(ctx context.Context, w http.ResponseWriter, req *http.Request, vars map[string]string) error {
+	if err := httputils.ParseForm(req); err != nil {
+		return err
+	}
+	targetName := req.FormValue("target")
+	if targetName == "" {
+		return fmt.Errorf("bad parameter: target name is required")
+	}
+
+	lunMap := scsi.GetTargetLUNMap(targetName)
+	var result []api.LuInfo
+	for lun, lu := range lunMap {
+		if lu == nil {
+			continue
+		}
+		info := api.LuInfo{
+			LUN:    lun,
+			Path:   lu.Path,
+			Online: lu.Attrs.Online,
+			Size:   lu.Size,
+		}
+		result = append(result, info)
+	}
+	return httputils.WriteJSON(w, http.StatusOK, result)
+}
+
+func (r *luRouter) getLu(ctx context.Context, w http.ResponseWriter, req *http.Request, vars map[string]string) error {
 	return nil
 }
 
-func (s *luRouter) postLuCreate(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
+func (r *luRouter) postLuCreate(ctx context.Context, w http.ResponseWriter, req *http.Request, vars map[string]string) error {
+	var opts api.LuCreateRequest
+	if err := json.NewDecoder(req.Body).Decode(&opts); err != nil {
+		return fmt.Errorf("bad parameter: %v", err)
+	}
+	if opts.TargetName == "" {
+		return fmt.Errorf("bad parameter: target name is required")
+	}
+	if opts.Path == "" {
+		return fmt.Errorf("bad parameter: path is required")
+	}
+
+	bs := config.BackendStorage{
+		DeviceID:   opts.DeviceID,
+		Path:       opts.Path,
+		Online:     true,
+		BlockShift: opts.BlockShift,
+	}
+	if err := scsi.AddBackendStorage(bs); err != nil {
+		return err
+	}
+
+	m := scsi.LUNMapping{
+		TargetName: opts.TargetName,
+		LUN:        opts.LUN,
+		DeviceID:   opts.DeviceID,
+	}
+	if err := scsi.AddLUNMapping(m); err != nil {
+		return err
+	}
+
+	// Refresh target's device map
+	service := scsi.NewSCSITargetService()
+	service.RereadTargetLUNMap()
+
+	w.WriteHeader(http.StatusCreated)
 	return nil
 }
 
-func (s *luRouter) deleteLu(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
+func (r *luRouter) deleteLu(ctx context.Context, w http.ResponseWriter, req *http.Request, vars map[string]string) error {
+	var opts api.LuRemoveOptions
+	if err := json.NewDecoder(req.Body).Decode(&opts); err != nil {
+		return fmt.Errorf("bad parameter: %v", err)
+	}
+	if opts.TargetName == "" {
+		return fmt.Errorf("bad parameter: target name is required")
+	}
+
+	scsi.DelLUNMapping(scsi.LUNMapping{
+		TargetName: opts.TargetName,
+		LUN:        opts.LUN,
+	})
+
+	// Refresh target's device map
+	service := scsi.NewSCSITargetService()
+	service.RereadTargetLUNMap()
+
+	w.WriteHeader(http.StatusNoContent)
 	return nil
 }

pkg/apiserver/router/lu/lu_test.go

@@ -0,0 +1,90 @@
+package lu
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gostor/gotgt/pkg/api"
+	"github.com/gostor/gotgt/pkg/scsi"
+	"golang.org/x/net/context"
+)
+
+func resetService() {
+	s := scsi.NewSCSITargetService()
+	targets, _ := s.GetTargetList()
+	for _, t := range targets {
+		s.DeleteTarget(t.Name, true)
+	}
+}
+
+func TestGetLuListEmpty(t *testing.T) {
+	resetService()
+	r := &luRouter{}
+
+	req, _ := http.NewRequest("GET", "/lu/list?target=iqn.test", nil)
+	req.ParseForm()
+	w := httptest.NewRecorder()
+
+	err := r.getLuList(context.Background(), w, req, nil)
+	if err != nil {
+		t.Fatalf("getLuList failed: %v", err)
+	}
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected status 200, got %d", w.Code)
+	}
+
+	var lus []api.LuInfo
+	if err := json.Unmarshal(w.Body.Bytes(), &lus); err != nil {
+		t.Fatalf("failed to decode response: %v", err)
+	}
+	if len(lus) != 0 {
+		t.Fatalf("expected 0 LUs for non-existent target, got %d", len(lus))
+	}
+}
+
+func TestGetLuListNoTarget(t *testing.T) {
+	r := &luRouter{}
+
+	req, _ := http.NewRequest("GET", "/lu/list", nil)
+	w := httptest.NewRecorder()
+
+	err := r.getLuList(context.Background(), w, req, nil)
+	if err == nil {
+		t.Fatal("expected error when target param is missing")
+	}
+}
+
+func TestDeleteLu(t *testing.T) {
+	resetService()
+	r := &luRouter{}
+
+	body, _ := json.Marshal(api.LuRemoveOptions{TargetName: "iqn.test", LUN: 0})
+	req, _ := http.NewRequest("DELETE", "/lu/delete", bytes.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	w := httptest.NewRecorder()
+
+	err := r.deleteLu(context.Background(), w, req, nil)
+	if err != nil {
+		t.Fatalf("deleteLu failed: %v", err)
+	}
+	if w.Code != http.StatusNoContent {
+		t.Fatalf("expected status 204, got %d", w.Code)
+	}
+}
+
+func TestDeleteLuNoTarget(t *testing.T) {
+	r := &luRouter{}
+
+	body, _ := json.Marshal(api.LuRemoveOptions{TargetName: "", LUN: 0})
+	req, _ := http.NewRequest("DELETE", "/lu/delete", bytes.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	w := httptest.NewRecorder()
+
+	err := r.deleteLu(context.Background(), w, req, nil)
+	if err == nil {
+		t.Fatal("expected error when target name is empty")
+	}
+}

pkg/apiserver/router/target/target.go

@@ -16,27 +16,30 @@ limitations under the License.
 package target
 
 import (
+	"encoding/json"
+	"fmt"
 	"net/http"
 
+	"github.com/gostor/gotgt/pkg/api"
 	"github.com/gostor/gotgt/pkg/apiserver/httputils"
 	"github.com/gostor/gotgt/pkg/apiserver/router"
 	"github.com/gostor/gotgt/pkg/scsi"
 	"golang.org/x/net/context"
 )
 
-// containerRouter is a router to talk with the container controller
+// targetRouter is a router to talk with the target controller
 type targetRouter struct {
 	routes []router.Route
 }
 
-// NewRouter initializes a new container router
+// NewRouter initializes a new target router
 func NewRouter() router.Router {
 	r := &targetRouter{}
 	r.initRoutes()
 	return r
 }
 
-// Routes returns the available routers to the container controller
+// Routes returns the available routers to the target controller
 func (r *targetRouter) Routes() []router.Route {
 	return r.routes
 }
@@ -46,10 +49,10 @@ func (r *targetRouter) initRoutes() {
 	r.routes = []router.Route{
 		// GET
 		router.NewGetRoute("/target/list", r.getTargetList),
+		router.NewGetRoute("/target/tpgt/list", r.getTargetTPGTList),
 		// POST
 		router.NewPostRoute("/target/create", r.postTargetCreate),
 		router.NewPostRoute("/target/up", r.postTargetUp),
-		// PUT
 		// DELETE
 		router.NewDeleteRoute("/target/{name:.*}", r.deleteTarget),
 	}
@@ -65,7 +68,20 @@ func (r *targetRouter) getTargetList(ctx context.Context, w http.ResponseWriter,
 }
 
 func (r *targetRouter) postTargetCreate(ctx context.Context, w http.ResponseWriter, req *http.Request, vars map[string]string) error {
-	return nil
+	var opts api.TargetCreateRequest
+	if err := json.NewDecoder(req.Body).Decode(&opts); err != nil {
+		return fmt.Errorf("bad parameter: %v", err)
+	}
+	if opts.Name == "" {
+		return fmt.Errorf("bad parameter: target name is required")
+	}
+
+	service := scsi.NewSCSITargetService()
+	target, err := service.NewSCSITarget(len(service.Targets), "iscsi", opts.Name)
+	if err != nil {
+		return err
+	}
+	return httputils.WriteJSON(w, http.StatusCreated, target)
 }
 
 func (r *targetRouter) postTargetUp(ctx context.Context, w http.ResponseWriter, req *http.Request, vars map[string]string) error {
@@ -73,5 +89,51 @@ func (r *targetRouter) postTargetUp(ctx context.Context, w http.ResponseWriter,
 }
 
 func (r *targetRouter) deleteTarget(ctx context.Context, w http.ResponseWriter, req *http.Request, vars map[string]string) error {
+	name := vars["name"]
+	if name == "" {
+		return fmt.Errorf("bad parameter: target name is required")
+	}
+
+	if err := httputils.ParseForm(req); err != nil {
+		return err
+	}
+	force := httputils.BoolValue(req, "force")
+
+	service := scsi.NewSCSITargetService()
+	if err := service.DeleteTarget(name, force); err != nil {
+		return err
+	}
+	w.WriteHeader(http.StatusNoContent)
 	return nil
 }
+
+func (r *targetRouter) getTargetTPGTList(ctx context.Context, w http.ResponseWriter, req *http.Request, vars map[string]string) error {
+	if err := httputils.ParseForm(req); err != nil {
+		return err
+	}
+	targetName := req.FormValue("target")
+	if targetName == "" {
+		return fmt.Errorf("bad parameter: target name is required")
+	}
+
+	service := scsi.NewSCSITargetService()
+	tgts, err := service.GetTargetList()
+	if err != nil {
+		return err
+	}
+
+	for _, tgt := range tgts {
+		if tgt.Name == targetName {
+			var result []api.TpgtInfo
+			for _, tpg := range tgt.TargetPortGroups {
+				info := api.TpgtInfo{TPGT: tpg.GroupID}
+				for _, port := range tpg.TargetPortGroup {
+					info.Portals = append(info.Portals, port.TargetPortName)
+				}
+				result = append(result, info)
+			}
+			return httputils.WriteJSON(w, http.StatusOK, result)
+		}
+	}
+	return fmt.Errorf("target %q not found", targetName)
+}

pkg/apiserver/router/target/target_test.go

@@ -0,0 +1,124 @@
+package target
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gostor/gotgt/pkg/api"
+	"github.com/gostor/gotgt/pkg/scsi"
+	_ "github.com/gostor/gotgt/pkg/scsi/backingstore"
+	"golang.org/x/net/context"
+)
+
+func resetService() *scsi.SCSITargetService {
+	s := scsi.NewSCSITargetService()
+	// Clear targets for test isolation
+	targets, _ := s.GetTargetList()
+	for _, t := range targets {
+		s.DeleteTarget(t.Name, true)
+	}
+	return s
+}
+
+func TestPostTargetCreate(t *testing.T) {
+	resetService()
+	r := &targetRouter{}
+
+	body, _ := json.Marshal(api.TargetCreateRequest{Name: "iqn.2016-09.com.gotgt:test"})
+	req, _ := http.NewRequest("POST", "/target/create", bytes.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	w := httptest.NewRecorder()
+
+	err := r.postTargetCreate(context.Background(), w, req, nil)
+	if err != nil {
+		t.Fatalf("postTargetCreate failed: %v", err)
+	}
+	if w.Code != http.StatusCreated {
+		t.Fatalf("expected status 201, got %d", w.Code)
+	}
+
+	var target api.SCSITarget
+	if err := json.Unmarshal(w.Body.Bytes(), &target); err != nil {
+		t.Fatalf("failed to decode response: %v", err)
+	}
+	if target.Name != "iqn.2016-09.com.gotgt:test" {
+		t.Fatalf("expected target name iqn.2016-09.com.gotgt:test, got %s", target.Name)
+	}
+}
+
+func TestPostTargetCreateEmptyName(t *testing.T) {
+	resetService()
+	r := &targetRouter{}
+
+	body, _ := json.Marshal(api.TargetCreateRequest{Name: ""})
+	req, _ := http.NewRequest("POST", "/target/create", bytes.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	w := httptest.NewRecorder()
+
+	err := r.postTargetCreate(context.Background(), w, req, nil)
+	if err == nil {
+		t.Fatal("expected error for empty target name")
+	}
+}
+
+func TestDeleteTarget(t *testing.T) {
+	s := resetService()
+	r := &targetRouter{}
+
+	s.NewSCSITarget(0, "iscsi", "iqn.2016-09.com.gotgt:to_delete")
+
+	req, _ := http.NewRequest("DELETE", "/target/iqn.2016-09.com.gotgt:to_delete", nil)
+	w := httptest.NewRecorder()
+
+	vars := map[string]string{"name": "iqn.2016-09.com.gotgt:to_delete"}
+	err := r.deleteTarget(context.Background(), w, req, vars)
+	if err != nil {
+		t.Fatalf("deleteTarget failed: %v", err)
+	}
+	if w.Code != http.StatusNoContent {
+		t.Fatalf("expected status 204, got %d", w.Code)
+	}
+}
+
+func TestDeleteTargetNotFound(t *testing.T) {
+	resetService()
+	r := &targetRouter{}
+
+	req, _ := http.NewRequest("DELETE", "/target/nonexistent", nil)
+	w := httptest.NewRecorder()
+
+	vars := map[string]string{"name": "nonexistent"}
+	err := r.deleteTarget(context.Background(), w, req, vars)
+	if err == nil {
+		t.Fatal("expected error deleting nonexistent target")
+	}
+}
+
+func TestGetTargetList(t *testing.T) {
+	s := resetService()
+	r := &targetRouter{}
+
+	s.NewSCSITarget(0, "iscsi", "iqn.2016-09.com.gotgt:list_test")
+
+	req, _ := http.NewRequest("GET", "/target/list", nil)
+	w := httptest.NewRecorder()
+
+	err := r.getTargetList(context.Background(), w, req, nil)
+	if err != nil {
+		t.Fatalf("getTargetList failed: %v", err)
+	}
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected status 200, got %d", w.Code)
+	}
+
+	var targets []*api.SCSITarget
+	if err := json.Unmarshal(w.Body.Bytes(), &targets); err != nil {
+		t.Fatalf("failed to decode response: %v", err)
+	}
+	if len(targets) == 0 {
+		t.Fatal("expected at least one target")
+	}
+}