From 93e1476a0fd0d99befa4f2dacbfef7dd158b3ccc Mon Sep 17 00:00:00 2001 From: Lei Xue Date: Sat, 14 Mar 2026 20:30:47 +0800 Subject: [PATCH] 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) --- cmd/create.go | 26 +++- cmd/list.go | 64 ++++++++- cmd/remove.go | 29 ++-- pkg/api/client/lu_create.go | 13 ++ pkg/api/client/lu_delete.go | 13 ++ pkg/api/client/lu_list.go | 25 ++++ pkg/api/client/target_delete.go | 2 +- pkg/api/client/tpgt_list.go | 25 ++++ pkg/api/options.go | 33 +++++ pkg/apiserver/router/lu/lu.go | 101 ++++++++++++-- pkg/apiserver/router/lu/lu_test.go | 90 +++++++++++++ pkg/apiserver/router/target/target.go | 72 +++++++++- pkg/apiserver/router/target/target_test.go | 124 +++++++++++++++++ pkg/scsi/scsilumap.go | 7 + pkg/scsi/target.go | 24 +++- pkg/scsi/target_test.go | 148 +++++++++++++++++++++ 16 files changed, 760 insertions(+), 36 deletions(-) create mode 100644 pkg/api/client/lu_create.go create mode 100644 pkg/api/client/lu_delete.go create mode 100644 pkg/api/client/lu_list.go create mode 100644 pkg/api/client/tpgt_list.go create mode 100644 pkg/apiserver/router/lu/lu_test.go create mode 100644 pkg/apiserver/router/target/target_test.go create mode 100644 pkg/scsi/target_test.go diff --git a/cmd/create.go b/cmd/create.go index d0b6b42..d66152a 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -65,19 +65,24 @@ func newCreateTargetCmd(cli *client.Client) *cobra.Command { } func newCreateLuCmd(cli *client.Client) *cobra.Command { + opts := api.LuCreateRequest{} var cmd = &cobra.Command{ Use: "lu", - Short: "Create a new Lu into gotgt", - Long: `All software has versions. This is Gotgt 's`, + Short: "Create a new LU into gotgt", + Long: `Create a new Logical Unit and map it to a target`, RunE: func(cmd *cobra.Command, args []string) error { if err := NoArgs(cmd, args); err != nil { return err } - return createLu(cli) + return createLu(cli, opts) }, } flags := cmd.Flags() - _ = flags + flags.StringVar(&opts.TargetName, "target", "", "Specify target name") + flags.Uint64Var(&opts.LUN, "lun", 0, "Specify LUN number") + flags.Uint64Var(&opts.DeviceID, "device-id", 0, "Specify device ID") + flags.StringVar(&opts.Path, "path", "", "Specify backing store path (e.g., file:/tmp/disk.img)") + flags.UintVar(&opts.BlockShift, "block-shift", 9, "Specify block shift (default 9 = 512 bytes)") return cmd } @@ -93,6 +98,17 @@ func createTarget(cli *client.Client, opts api.TargetCreateRequest) error { return nil } -func createLu(cli *client.Client) error { +func createLu(cli *client.Client, opts api.LuCreateRequest) error { + if opts.TargetName == "" { + return fmt.Errorf("target name is required (--target)") + } + if opts.Path == "" { + return fmt.Errorf("backing store path is required (--path)") + } + err := cli.LuCreate(context.Background(), opts) + if err != nil { + return err + } + fmt.Printf("LU %d successfully created on target %s\n", opts.LUN, opts.TargetName) return nil } diff --git a/cmd/list.go b/cmd/list.go index ad5b899..0f2a88f 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -19,6 +19,7 @@ package cmd import ( "fmt" "os" + "strings" "text/tabwriter" "github.com/gostor/gotgt/pkg/api" @@ -39,6 +40,7 @@ func newListCommand(cli *client.Client) *cobra.Command { cmd.AddCommand( newListTargetCmd(cli), newListLuCmd(cli), + newListTpgtCmd(cli), ) return cmd } @@ -68,19 +70,38 @@ func newListTargetCmd(cli *client.Client) *cobra.Command { } func newListLuCmd(cli *client.Client) *cobra.Command { + opts := api.LuListOptions{} var cmd = &cobra.Command{ Use: "lu", - Short: "List Lu(s) of gotgt", - Long: `All software has versions. This is Gotgt 's`, + Short: "List LU(s) of gotgt", + Long: `List Logical Units for a target`, RunE: func(cmd *cobra.Command, args []string) error { if err := NoArgs(cmd, args); err != nil { return err } - return listLu(cli) + return listLu(cli, opts) }, } flags := cmd.Flags() - _ = flags + flags.StringVar(&opts.TargetName, "target", "", "Specify target name") + return cmd +} + +func newListTpgtCmd(cli *client.Client) *cobra.Command { + opts := api.TpgtListOptions{} + var cmd = &cobra.Command{ + Use: "tpgt", + Short: "List TPGT(s) of gotgt", + Long: `List Target Portal Group Tags for a target`, + RunE: func(cmd *cobra.Command, args []string) error { + if err := NoArgs(cmd, args); err != nil { + return err + } + return listTpgt(cli, opts) + }, + } + flags := cmd.Flags() + flags.StringVar(&opts.TargetName, "target", "", "Specify target name") return cmd } @@ -106,6 +127,39 @@ func listTarget(cli *client.Client, opts api.TargetListOptions) error { return nil } -func listLu(cli *client.Client) error { +func listLu(cli *client.Client, opts api.LuListOptions) error { + if opts.TargetName == "" { + return fmt.Errorf("target name is required (--target)") + } + results, err := cli.LuList(context.Background(), opts) + if err != nil { + return err + } + + w := tabwriter.NewWriter(os.Stdout, 15, 1, 3, ' ', 0) + fmt.Fprintln(w, "LUN\tPATH\tSIZE\tONLINE") + for _, lu := range results { + fmt.Fprintf(w, "%d\t%s\t%d\t%v\n", lu.LUN, lu.Path, lu.Size, lu.Online) + } + w.Flush() + return nil +} + +func listTpgt(cli *client.Client, opts api.TpgtListOptions) error { + if opts.TargetName == "" { + return fmt.Errorf("target name is required (--target)") + } + results, err := cli.TpgtList(context.Background(), opts) + if err != nil { + return err + } + + w := tabwriter.NewWriter(os.Stdout, 10, 1, 3, ' ', 0) + fmt.Fprintln(w, "TPGT\tPORTALS") + for _, tpgt := range results { + portals := strings.Join(tpgt.Portals, ", ") + fmt.Fprintf(w, "%d\t%s\n", tpgt.TPGT, portals) + } + w.Flush() return nil } diff --git a/cmd/remove.go b/cmd/remove.go index fa89495..feca8a5 100644 --- a/cmd/remove.go +++ b/cmd/remove.go @@ -28,7 +28,7 @@ import ( func newRemoveCommand(cli *client.Client) *cobra.Command { var cmd = &cobra.Command{ Use: "rm", - Short: "remove a new object", + Short: "Remove an object", Long: `All software has versions. This is Gotgt 's`, Run: func(cmd *cobra.Command, args []string) { fmt.Println(cmd.UsageString()) @@ -45,7 +45,7 @@ func newRemoveTargetCmd(cli *client.Client) *cobra.Command { opts := api.TargetRemoveOptions{} var cmd = &cobra.Command{ Use: "target", - Short: "Remove a new target into gotgt", + Short: "Remove a target from gotgt", Long: `All software has versions. This is Gotgt 's`, RunE: func(cmd *cobra.Command, args []string) error { return removeTarget(cli, opts) @@ -53,23 +53,28 @@ func newRemoveTargetCmd(cli *client.Client) *cobra.Command { } flags := cmd.Flags() flags.StringVar(&opts.Name, "name", "", "Specify target name") - flags.BoolVar(&opts.Force, "force", false, "Specify target name") + flags.BoolVar(&opts.Force, "force", false, "Force removal even with active sessions") return cmd } func newRemoveLuCmd(cli *client.Client) *cobra.Command { + opts := api.LuRemoveOptions{} var cmd = &cobra.Command{ Use: "lu", - Short: "Remove a new Lu into gotgt", - Long: `All software has versions. This is Gotgt 's`, + Short: "Remove a LU from gotgt", + Long: `Remove a Logical Unit from a target`, RunE: func(cmd *cobra.Command, args []string) error { - return removeLu(cli) + if err := NoArgs(cmd, args); err != nil { + return err + } + return removeLu(cli, opts) }, } flags := cmd.Flags() - _ = flags + flags.StringVar(&opts.TargetName, "target", "", "Specify target name") + flags.Uint64Var(&opts.LUN, "lun", 0, "Specify LUN number") return cmd } @@ -82,6 +87,14 @@ func removeTarget(cli *client.Client, opts api.TargetRemoveOptions) error { return nil } -func removeLu(cli *client.Client) error { +func removeLu(cli *client.Client, opts api.LuRemoveOptions) error { + if opts.TargetName == "" { + return fmt.Errorf("target name is required (--target)") + } + err := cli.LuRemove(context.Background(), opts) + if err != nil { + return err + } + fmt.Printf("LU %d successfully removed from target %s\n", opts.LUN, opts.TargetName) return nil } diff --git a/pkg/api/client/lu_create.go b/pkg/api/client/lu_create.go new file mode 100644 index 0000000..d211075 --- /dev/null +++ b/pkg/api/client/lu_create.go @@ -0,0 +1,13 @@ +package client + +import ( + "github.com/gostor/gotgt/pkg/api" + "golang.org/x/net/context" +) + +// LuCreate creates a LU in the SCSI Target. +func (cli *Client) LuCreate(ctx context.Context, options api.LuCreateRequest) error { + resp, err := cli.post(ctx, "/lu/create", nil, options, nil) + ensureReaderClosed(resp) + return err +} diff --git a/pkg/api/client/lu_delete.go b/pkg/api/client/lu_delete.go new file mode 100644 index 0000000..1fd6f80 --- /dev/null +++ b/pkg/api/client/lu_delete.go @@ -0,0 +1,13 @@ +package client + +import ( + "github.com/gostor/gotgt/pkg/api" + "golang.org/x/net/context" +) + +// LuRemove removes a LU from a target in the SCSI Target. +func (cli *Client) LuRemove(ctx context.Context, options api.LuRemoveOptions) error { + resp, err := cli.post(ctx, "/lu/delete", nil, options, nil) + ensureReaderClosed(resp) + return err +} diff --git a/pkg/api/client/lu_list.go b/pkg/api/client/lu_list.go new file mode 100644 index 0000000..d6fc932 --- /dev/null +++ b/pkg/api/client/lu_list.go @@ -0,0 +1,25 @@ +package client + +import ( + "encoding/json" + "net/url" + + "github.com/gostor/gotgt/pkg/api" + "golang.org/x/net/context" +) + +// LuList lists LUs for a target in the SCSI Target. +func (cli *Client) LuList(ctx context.Context, options api.LuListOptions) ([]api.LuInfo, error) { + var lus []api.LuInfo + query := url.Values{} + if options.TargetName != "" { + query.Set("target", options.TargetName) + } + resp, err := cli.get(ctx, "/lu/list", query, nil) + if err != nil { + return lus, err + } + err = json.NewDecoder(resp.body).Decode(&lus) + ensureReaderClosed(resp) + return lus, err +} diff --git a/pkg/api/client/target_delete.go b/pkg/api/client/target_delete.go index b35d833..a81faf9 100644 --- a/pkg/api/client/target_delete.go +++ b/pkg/api/client/target_delete.go @@ -29,7 +29,7 @@ func (cli *Client) TargetRemove(ctx context.Context, options api.TargetRemoveOpt if options.Force { query.Set("force", "1") } - resp, err := cli.delete(ctx, "/targets/"+options.Name, query, nil) + resp, err := cli.delete(ctx, "/target/"+options.Name, query, nil) ensureReaderClosed(resp) return err } diff --git a/pkg/api/client/tpgt_list.go b/pkg/api/client/tpgt_list.go new file mode 100644 index 0000000..4c16501 --- /dev/null +++ b/pkg/api/client/tpgt_list.go @@ -0,0 +1,25 @@ +package client + +import ( + "encoding/json" + "net/url" + + "github.com/gostor/gotgt/pkg/api" + "golang.org/x/net/context" +) + +// TpgtList lists TPGTs for a target in the SCSI Target. +func (cli *Client) TpgtList(ctx context.Context, options api.TpgtListOptions) ([]api.TpgtInfo, error) { + var tpgts []api.TpgtInfo + query := url.Values{} + if options.TargetName != "" { + query.Set("target", options.TargetName) + } + resp, err := cli.get(ctx, "/target/tpgt/list", query, nil) + if err != nil { + return tpgts, err + } + err = json.NewDecoder(resp.body).Decode(&tpgts) + ensureReaderClosed(resp) + return tpgts, err +} diff --git a/pkg/api/options.go b/pkg/api/options.go index bf8d517..bb80939 100644 --- a/pkg/api/options.go +++ b/pkg/api/options.go @@ -29,3 +29,36 @@ type TargetListOptions struct { Name string Verbose bool } + +type LuCreateRequest struct { + TargetName string `json:"targetName"` + DeviceID uint64 `json:"deviceID"` + LUN uint64 `json:"lun"` + Path string `json:"path"` + BlockShift uint `json:"blockShift"` +} + +type LuListOptions struct { + TargetName string +} + +type LuRemoveOptions struct { + TargetName string `json:"targetName"` + LUN uint64 `json:"lun"` +} + +type LuInfo struct { + LUN uint64 `json:"lun"` + Path string `json:"path"` + Online bool `json:"online"` + Size uint64 `json:"size"` +} + +type TpgtListOptions struct { + TargetName string +} + +type TpgtInfo struct { + TPGT uint16 `json:"tpgt"` + Portals []string `json:"portals"` +} diff --git a/pkg/apiserver/router/lu/lu.go b/pkg/apiserver/router/lu/lu.go index 000a9a7..502e1b1 100644 --- a/pkg/apiserver/router/lu/lu.go +++ b/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 } diff --git a/pkg/apiserver/router/lu/lu_test.go b/pkg/apiserver/router/lu/lu_test.go new file mode 100644 index 0000000..098f820 --- /dev/null +++ b/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") + } +} diff --git a/pkg/apiserver/router/target/target.go b/pkg/apiserver/router/target/target.go index 2c36fc8..ee80739 100644 --- a/pkg/apiserver/router/target/target.go +++ b/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) +} diff --git a/pkg/apiserver/router/target/target_test.go b/pkg/apiserver/router/target/target_test.go new file mode 100644 index 0000000..8f1a95d --- /dev/null +++ b/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") + } +} diff --git a/pkg/scsi/scsilumap.go b/pkg/scsi/scsilumap.go index a6fb823..7b81411 100644 --- a/pkg/scsi/scsilumap.go +++ b/pkg/scsi/scsilumap.go @@ -130,6 +130,13 @@ func DelLUNMapping(m LUNMapping) { delete(globalSCSILUMap.TargetsLUNMap[m.TargetName], m.LUN) } +// DelTargetLUNMap removes the entire LUN map for a target. +func DelTargetLUNMap(targetName string) { + globalSCSILUMap.mutex.Lock() + defer globalSCSILUMap.mutex.Unlock() + delete(globalSCSILUMap.TargetsLUNMap, targetName) +} + func InitSCSILUMap(config *config.Config) error { for _, bs := range config.Storages { if err := AddBackendStorage(bs); err != nil { diff --git a/pkg/scsi/target.go b/pkg/scsi/target.go index e96419d..584d8e5 100644 --- a/pkg/scsi/target.go +++ b/pkg/scsi/target.go @@ -26,11 +26,9 @@ import ( ) func (s *SCSITargetService) NewSCSITarget(tid int, driverName, name string) (*api.SCSITarget, error) { - // verify the target ID + s.mutex.Lock() + defer s.mutex.Unlock() - // verify the target's Name - - // verify the low level driver var target = &api.SCSITarget{ Name: name, TID: tid, @@ -45,6 +43,24 @@ func (s *SCSITargetService) NewSCSITarget(tid int, driverName, name string) (*ap return target, nil } +// DeleteTarget removes a target by name. If force is false and there are active sessions, it returns an error. +func (s *SCSITargetService) DeleteTarget(name string, force bool) error { + s.mutex.Lock() + defer s.mutex.Unlock() + + for i, t := range s.Targets { + if t.Name == name { + if !force && len(t.ITNexus) > 0 { + return fmt.Errorf("target %s has %d active sessions, use force to remove", name, len(t.ITNexus)) + } + s.Targets = append(s.Targets[:i], s.Targets[i+1:]...) + DelTargetLUNMap(name) + return nil + } + } + return fmt.Errorf("target %q not found", name) +} + func (s *SCSITargetService) RereadTargetLUNMap() { s.mutex.Lock() defer s.mutex.Unlock() diff --git a/pkg/scsi/target_test.go b/pkg/scsi/target_test.go new file mode 100644 index 0000000..0abe39a --- /dev/null +++ b/pkg/scsi/target_test.go @@ -0,0 +1,148 @@ +package scsi + +import ( + "testing" + + "github.com/google/uuid" + "github.com/gostor/gotgt/pkg/api" +) + +// nullBackingStore is a minimal backing store for tests. +type nullBackingStore struct { + BaseBackingStore +} + +func (bs *nullBackingStore) Open(dev *api.SCSILu, path string) error { return nil } +func (bs *nullBackingStore) Close(dev *api.SCSILu) error { return nil } +func (bs *nullBackingStore) Init(dev *api.SCSILu, Opts string) error { return nil } +func (bs *nullBackingStore) Exit(dev *api.SCSILu) error { return nil } +func (bs *nullBackingStore) Size(dev *api.SCSILu) uint64 { return 0 } +func (bs *nullBackingStore) Read(offset, tl int64) ([]byte, error) { return nil, nil } +func (bs *nullBackingStore) Write(wbuf []byte, offset int64) error { return nil } +func (bs *nullBackingStore) DataSync(offset, tl int64) error { return nil } +func (bs *nullBackingStore) DataAdvise(offset, length int64, advise uint32) error { return nil } +func (bs *nullBackingStore) Unmap([]api.UnmapBlockDescriptor) error { return nil } + +func init() { + if _, err := NewBackingStore("null"); err != nil { + RegisterBackingStore("null", func() (api.BackingStore, error) { + return &nullBackingStore{ + BaseBackingStore: BaseBackingStore{Name: "null"}, + }, nil + }) + } +} + +func resetTargetService() *SCSITargetService { + s := NewSCSITargetService() + s.mutex.Lock() + s.Targets = []*api.SCSITarget{} + s.mutex.Unlock() + return s +} + +func TestNewSCSITarget(t *testing.T) { + s := resetTargetService() + + target, err := s.NewSCSITarget(0, "iscsi", "iqn.2016-09.com.gotgt:test_target") + if err != nil { + t.Fatalf("NewSCSITarget failed: %v", err) + } + if target == nil { + t.Fatal("NewSCSITarget returned nil") + } + if target.Name != "iqn.2016-09.com.gotgt:test_target" { + t.Fatalf("expected target name iqn.2016-09.com.gotgt:test_target, got %s", target.Name) + } + if target.TID != 0 { + t.Fatalf("expected TID 0, got %d", target.TID) + } + if len(s.Targets) != 1 { + t.Fatalf("expected 1 target, got %d", len(s.Targets)) + } + if len(target.TargetPortGroups) != 1 { + t.Fatalf("expected 1 target port group, got %d", len(target.TargetPortGroups)) + } +} + +func TestDeleteTargetSuccess(t *testing.T) { + s := resetTargetService() + + _, err := s.NewSCSITarget(0, "iscsi", "iqn.2016-09.com.gotgt:delete_me") + if err != nil { + t.Fatalf("NewSCSITarget failed: %v", err) + } + + err = s.DeleteTarget("iqn.2016-09.com.gotgt:delete_me", false) + if err != nil { + t.Fatalf("DeleteTarget failed: %v", err) + } + if len(s.Targets) != 0 { + t.Fatalf("expected 0 targets after deletion, got %d", len(s.Targets)) + } +} + +func TestDeleteTargetNotFound(t *testing.T) { + s := resetTargetService() + + err := s.DeleteTarget("iqn.2016-09.com.gotgt:nonexistent", false) + if err == nil { + t.Fatal("expected error deleting nonexistent target") + } +} + +func TestDeleteTargetActiveSessions(t *testing.T) { + s := resetTargetService() + + target, err := s.NewSCSITarget(0, "iscsi", "iqn.2016-09.com.gotgt:busy_target") + if err != nil { + t.Fatalf("NewSCSITarget failed: %v", err) + } + + // Simulate active session + target.ITNexusMutex.Lock() + target.ITNexus[uuid.New()] = &api.ITNexus{ID: uuid.New()} + target.ITNexusMutex.Unlock() + + // Should fail without force + err = s.DeleteTarget("iqn.2016-09.com.gotgt:busy_target", false) + if err == nil { + t.Fatal("expected error deleting target with active sessions") + } + + // Should succeed with force + err = s.DeleteTarget("iqn.2016-09.com.gotgt:busy_target", true) + if err != nil { + t.Fatalf("DeleteTarget with force failed: %v", err) + } + if len(s.Targets) != 0 { + t.Fatalf("expected 0 targets after forced deletion, got %d", len(s.Targets)) + } +} + +func TestDeleteTargetMultiple(t *testing.T) { + s := resetTargetService() + + s.NewSCSITarget(0, "iscsi", "iqn.2016-09.com.gotgt:target_a") + s.NewSCSITarget(1, "iscsi", "iqn.2016-09.com.gotgt:target_b") + s.NewSCSITarget(2, "iscsi", "iqn.2016-09.com.gotgt:target_c") + + if len(s.Targets) != 3 { + t.Fatalf("expected 3 targets, got %d", len(s.Targets)) + } + + // Delete middle one + err := s.DeleteTarget("iqn.2016-09.com.gotgt:target_b", false) + if err != nil { + t.Fatalf("DeleteTarget failed: %v", err) + } + if len(s.Targets) != 2 { + t.Fatalf("expected 2 targets, got %d", len(s.Targets)) + } + if s.Targets[0].Name != "iqn.2016-09.com.gotgt:target_a" { + t.Fatalf("expected target_a first, got %s", s.Targets[0].Name) + } + if s.Targets[1].Name != "iqn.2016-09.com.gotgt:target_c" { + t.Fatalf("expected target_c second, got %s", s.Targets[1].Name) + } +}