diff --git a/tests/ginkgo/AGENTS.md b/tests/ginkgo/AGENTS.md index 9041c7797..ee6846e53 100644 --- a/tests/ginkgo/AGENTS.md +++ b/tests/ginkgo/AGENTS.md @@ -11,9 +11,9 @@ and fixtures. necessarily a kernel check. Check operation errors even with exit status 0. - Use shared command runners with contexts, timeouts, logs and failure handling. Execution failures must not satisfy assertions for expected rejections. -- Keep ownership and cleanup explicit for configuration, sessions and modules. - Restore changed global settings; global SCST clearing belongs only to - explicit clear-config tests. +- Keep ownership and cleanup explicit for configuration, backing files, + sessions and modules. Restore changed global settings; global SCST clearing + belongs only to explicit clear-config tests. - Failed cleanup or uncertain command completion must stop subsequent specs. Preserve artifacts and residual resources for inspection instead of forcing cleanup. diff --git a/tests/ginkgo/README.md b/tests/ginkgo/README.md index 7e71ed94d..c056347f4 100644 --- a/tests/ginkgo/README.md +++ b/tests/ginkgo/README.md @@ -50,12 +50,13 @@ select other labels. ```sh # List every spec and its labels without accessing SCST: make ginkgo-list GINKGO_LABEL='' -# Select rejection cases: -make ginkgo-test-build GINKGO_CONFIG=tests/ginkgo/config.local.json GINKGO_LABEL=negative +# All except the known-crash case: +make ginkgo-test-build GINKGO_CONFIG=tests/ginkgo/config.local.json GINKGO_LABEL='!known-crash' ``` -An empty label filter selects all specs. `clear-config` tests invoke global -`scstadmin -clear_config -force`. +An empty label filter selects all specs. The `known-crash` case (sync on +inactive fileio) also requires `allow_known_crash=true` and can crash affected +kernels. `clear-config` tests invoke global `scstadmin -clear_config -force`. An empty selection or suite failure stops the run. Parallel runs are unsupported: use the same `lock_file` and runner user on the host. @@ -68,6 +69,13 @@ Make invocation directory; relative config paths resolve against the JSON file, except interpreter arguments, which need absolute paths. Local profiles are ignored by Git. +`backing_dir` selects an existing directory for temporary fileio images; +empty uses the artifact directory. Tests never adopt existing backing files. +Avoid whitespace, commas, semicolons, quotes, braces and backslashes in backing +paths. Set `fileio_thin_provisioning=false` if the filesystem cannot support +the TP-enabled case. `stress_iterations` and `scale_luns` control lifecycle +repetition and many-LUN coverage. + ## Checks and recovery `make ginkgo-check` runs framework unit tests, vet and suite compilation; @@ -75,9 +83,9 @@ it does not access SCST. Neither it nor `ginkgo-list` proves live behavior. Use the Make targets for live runs: plain `go test` skips specs without `SCST_TEST_CONFIG` and needs `-count=1 -timeout=0` when running them. -Cleanup removes owned sessions/configuration before closing devices and -unloading modules. A failure or interrupted command stops the run and may -retain modules. Inspect and recover the host before +Cleanup removes owned sessions/configuration before closing devices, deleting +backing files and unloading modules. A failure or interrupted command stops +the run and may retain modules and files. Inspect and recover the host before retrying; never remove a lock file to bypass a held lock. The printed artifact directory under `report_dir` contains effective config, diff --git a/tests/ginkgo/config.example.json b/tests/ginkgo/config.example.json index 64c1b69aa..196332d0b 100644 --- a/tests/ginkgo/config.example.json +++ b/tests/ginkgo/config.example.json @@ -10,5 +10,10 @@ "poll_interval": "100ms", "suite_timeout": "15m", "build_timeout": "10m", - "build_jobs": 2 + "build_jobs": 2, + "backing_dir": "", + "fileio_thin_provisioning": true, + "stress_iterations": 8, + "scale_luns": 256, + "allow_known_crash": false } diff --git a/tests/ginkgo/internal/testcfg/config.go b/tests/ginkgo/internal/testcfg/config.go index 8c9eeb14f..9273ae92c 100644 --- a/tests/ginkgo/internal/testcfg/config.go +++ b/tests/ginkgo/internal/testcfg/config.go @@ -15,18 +15,23 @@ import ( // Config describes one local, disposable SCST host. There are no implicit // per-user overrides: the named file completely determines a run. type Config struct { - AdminCommand []string `json:"admin_command"` - DisposableHost bool `json:"disposable_host"` - NamePrefix string `json:"name_prefix"` - ReportDir string `json:"report_dir"` - LockFile string `json:"lock_file"` - CommandTimeout string `json:"command_timeout"` - CleanupTimeout string `json:"cleanup_timeout"` - SettleTimeout string `json:"settle_timeout"` - PollInterval string `json:"poll_interval"` - SuiteTimeout string `json:"suite_timeout"` - BuildTimeout string `json:"build_timeout"` - BuildJobs int `json:"build_jobs"` + AdminCommand []string `json:"admin_command"` + DisposableHost bool `json:"disposable_host"` + NamePrefix string `json:"name_prefix"` + ReportDir string `json:"report_dir"` + LockFile string `json:"lock_file"` + CommandTimeout string `json:"command_timeout"` + CleanupTimeout string `json:"cleanup_timeout"` + SettleTimeout string `json:"settle_timeout"` + PollInterval string `json:"poll_interval"` + SuiteTimeout string `json:"suite_timeout"` + BuildTimeout string `json:"build_timeout"` + BuildJobs int `json:"build_jobs"` + BackingDir string `json:"backing_dir"` + FileioThinProvisioning bool `json:"fileio_thin_provisioning"` + StressIterations int `json:"stress_iterations"` + ScaleLUNs int `json:"scale_luns"` + AllowKnownCrash bool `json:"allow_known_crash"` } func Load(path string) (Config, error) { @@ -36,6 +41,7 @@ func Load(path string) (Config, error) { CommandTimeout: "30s", CleanupTimeout: "2m", SettleTimeout: "10s", PollInterval: "100ms", SuiteTimeout: "15m", BuildTimeout: "10m", BuildJobs: 2, + StressIterations: 8, ScaleLUNs: 256, FileioThinProvisioning: true, } f, err := os.Open(path) if err != nil { @@ -66,6 +72,9 @@ func Load(path string) (Config, error) { *p = filepath.Join(base, *p) } } + if c.BackingDir != "" && !filepath.IsAbs(c.BackingDir) { + c.BackingDir = filepath.Join(base, c.BackingDir) + } if strings.ContainsRune(c.AdminCommand[0], '/') && !filepath.IsAbs(c.AdminCommand[0]) { c.AdminCommand[0] = filepath.Join(base, c.AdminCommand[0]) } @@ -104,6 +113,12 @@ func (c Config) Validate() error { if c.BuildJobs < 1 { return fmt.Errorf("build_jobs must be positive") } + if c.StressIterations < 1 || c.StressIterations > 1000 { + return fmt.Errorf("stress_iterations must be between 1 and 1000") + } + if c.ScaleLUNs < 2 || c.ScaleLUNs > 2048 { + return fmt.Errorf("scale_luns must be between 2 and 2048") + } return nil } diff --git a/tests/ginkgo/internal/testcfg/config_test.go b/tests/ginkgo/internal/testcfg/config_test.go index 9975b8de5..259470459 100644 --- a/tests/ginkgo/internal/testcfg/config_test.go +++ b/tests/ginkgo/internal/testcfg/config_test.go @@ -17,7 +17,7 @@ func configFile(t *testing.T, input string) string { } func TestLoadDefaultsAndPaths(t *testing.T) { - path := configFile(t, `{"admin_command":["./bin/admin","literal argument"],"report_dir":"out","lock_file":"host.lock"}`) + path := configFile(t, `{"admin_command":["./bin/admin","literal argument"],"report_dir":"out","lock_file":"host.lock","backing_dir":"scratch"}`) cfg, err := Load(path) if err != nil { t.Fatal(err) @@ -29,6 +29,9 @@ func TestLoadDefaultsAndPaths(t *testing.T) { if cfg.DisposableHost || cfg.CommandTimeout != "30s" || cfg.BuildJobs != 2 || cfg.BuildTimeout != "10m" { t.Fatalf("unsafe or missing defaults: %+v", cfg) } + if cfg.AllowKnownCrash || !cfg.FileioThinProvisioning || cfg.StressIterations != 8 || cfg.ScaleLUNs != 256 || cfg.BackingDir != filepath.Join(base, "scratch") { + t.Fatalf("unsafe or missing scenario defaults: %+v", cfg) + } } func TestRejectMalformedConfiguration(t *testing.T) { @@ -49,6 +52,10 @@ func TestRejectMalformedConfiguration(t *testing.T) { "zero build jobs": `{"build_jobs":0}`, "negative build jobs": `{"build_jobs":-2}`, "invalid build limit": `{"build_timeout":"never"}`, + "zero iterations": `{"stress_iterations":0}`, + "too many iterations": `{"stress_iterations":1001}`, + "too few LUNs": `{"scale_luns":1}`, + "too many LUNs": `{"scale_luns":2049}`, } { t.Run(name, func(t *testing.T) { if _, err := Load(configFile(t, input)); err == nil { @@ -57,3 +64,13 @@ func TestRejectMalformedConfiguration(t *testing.T) { }) } } + +func TestScenarioOverrides(t *testing.T) { + c, err := Load(configFile(t, `{"stress_iterations":12,"scale_luns":1000,"allow_known_crash":true,"fileio_thin_provisioning":false}`)) + if err != nil { + t.Fatal(err) + } + if c.StressIterations != 12 || c.ScaleLUNs != 1000 || !c.AllowKnownCrash || c.FileioThinProvisioning || c.BackingDir != "" { + t.Fatalf("scenario overrides lost: %+v", c) + } +} diff --git a/tests/ginkgo/suite/alua_state_test.go b/tests/ginkgo/suite/alua_state_test.go new file mode 100644 index 000000000..ca768b421 --- /dev/null +++ b/tests/ginkgo/suite/alua_state_test.go @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: GPL-2.0-only +package suite + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func (f *fixture) targetGroup(ctx context.Context, dg, suffix, target, id string) string { + GinkgoHelper() + tg := f.name(suffix) + f.do(ctx, "-add_tgrp", tg, "-dev_group", dg) + f.do(ctx, "-set_tgrp_attr", tg, "-dev_group", dg, "-attributes", "group_id="+id) + f.do(ctx, "-add_tgrp_tgt", target, "-dev_group", dg, "-tgt_group", tg) + return tg +} + +func (f *fixture) aluaState(ctx context.Context, dg, tg, state string) { + GinkgoHelper() + f.do(ctx, "-set_tgrp_attr", tg, "-dev_group", dg, "-attributes", "state="+state) + Expect(must(cli.Scalar(ctx, "state", "-list_tgrp_attr", tg, "-dev_group", dg))).To(Equal(state)) +} + +var _ = Describe("ALUA device activation", Label("basic", "alua", "fileio", "issue-55"), func() { + var f *fixture + BeforeEach(func() { f = newFixture() }) + + DescribeTable("deactivates and reopens devices for local target-group transitions", Label("negative"), + func(ctx SpecContext, mapped bool) { + d := f.fileDevice(ctx, "disk", f.backingFile("disk"), "bind_alua_state=1") + neighbor := f.fileDevice(ctx, "neighbor", f.backingFile("neighbor"), "bind_alua_state=1") + t, dg := f.target(ctx, "target"), f.deviceGroup(ctx, "devices") + if mapped { + f.at(ctx, t, "", "-add_lun", "0", "-device", d) + } + f.do(ctx, "-add_dgrp_dev", d, "-dev_group", dg) + tg := f.targetGroup(ctx, dg, "local", t, "11") + f.aluaState(ctx, dg, tg, "active") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + f.aluaState(ctx, dg, tg, "transitioning") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("0")) + Expect(f.deviceAttr(ctx, neighbor, "active")).To(Equal("1")) + path := f.backingFile("replacement") + f.setDevice(ctx, d, "filename="+path) + Expect(f.deviceAttr(ctx, d, "filename")).To(Equal(path)) + f.aluaState(ctx, dg, tg, "nonoptimized") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + f.setDevice(ctx, d, "sync=1") + f.aluaState(ctx, dg, tg, "standby") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("0")) + f.aluaState(ctx, dg, tg, "active") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + f.reject(ctx, "-set_tgrp_attr", tg, "-dev_group", dg, "-attributes", "state=invalid_state") + Expect(must(cli.Scalar(ctx, "state", "-list_tgrp_attr", tg, "-dev_group", dg))).To(Equal("active")) + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + Expect(f.deviceAttr(ctx, neighbor, "active")).To(Equal("1")) + if mapped { + Expect(f.luns(ctx, t, "")).To(Equal(map[int]string{0: d})) + } + }, Entry("with a LUN mapping", true), Entry("without a LUN mapping", false)) + + It("keeps local devices active when only a remote group changes", func(ctx SpecContext) { + d := f.fileDevice(ctx, "disk", f.backingFile("disk"), "bind_alua_state=1") + t, dg := f.target(ctx, "target"), f.deviceGroup(ctx, "devices") + f.do(ctx, "-add_dgrp_dev", d, "-dev_group", dg) + local := f.targetGroup(ctx, dg, "local", t, "11") + remote := f.targetGroup(ctx, dg, "remote", f.name("remote_target"), "12") + f.aluaState(ctx, dg, local, "active") + f.aluaState(ctx, dg, remote, "transitioning") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + Expect(must(cli.Scalar(ctx, "state", "-list_tgrp_attr", local, "-dev_group", dg))).To(Equal("active")) + f.aluaState(ctx, dg, remote, "standby") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + f.setDevice(ctx, d, "sync=1") + }) + + It("leaves unbound fileio active during a local ALUA transition", func(ctx SpecContext) { + d := f.fileDevice(ctx, "disk", f.backingFile("disk"), "bind_alua_state=0") + t, dg := f.target(ctx, "target"), f.deviceGroup(ctx, "devices") + f.do(ctx, "-add_dgrp_dev", d, "-dev_group", dg) + tg := f.targetGroup(ctx, dg, "local", t, "11") + f.aluaState(ctx, dg, tg, "active") + f.aluaState(ctx, dg, tg, "transitioning") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + f.aluaState(ctx, dg, tg, "active") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + }) +}) diff --git a/tests/ginkgo/suite/config_test.go b/tests/ginkgo/suite/config_test.go index 47d586168..c695913d9 100644 --- a/tests/ginkgo/suite/config_test.go +++ b/tests/ginkgo/suite/config_test.go @@ -61,6 +61,16 @@ var _ = Describe("Configuration files", Label("basic", "config"), func() { var f *fixture BeforeEach(func() { f = newFixture() }) + It("checks a valid configuration without applying it or changing existing objects", Label("issue-296"), func(ctx SpecContext) { + path := f.configuration() + d, t := f.device(ctx, "neighbor_disk"), f.target(ctx, "neighbor_target") + f.at(ctx, t, "", "-add_lun", "0", "-device", d) + f.do(ctx, "-check_config", path) + Expect(must(cli.Devices(ctx))).To(Equal(map[string]string{d: "vdisk_nullio"})) + Expect(must(cli.Targets(ctx))).To(Equal(map[string]string{t: "scst_local"})) + Expect(f.luns(ctx, t, "")).To(Equal(map[int]string{0: d})) + }) + It("applies the same configuration twice without changing its graph", func(ctx SpecContext) { path := f.configuration() f.do(ctx, "-config", path) diff --git a/tests/ginkgo/suite/fileio_test.go b/tests/ginkgo/suite/fileio_test.go new file mode 100644 index 000000000..360491606 --- /dev/null +++ b/tests/ginkgo/suite/fileio_test.go @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: GPL-2.0-only +package suite + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("File-backed devices", Label("basic", "fileio"), func() { + var f *fixture + BeforeEach(func() { f = newFixture() }) + + It("syncs an active device and keeps its configuration across reactivation", func(ctx SpecContext) { + path := f.backingFile("disk") + d := f.fileDevice(ctx, "disk", path) + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + f.setDevice(ctx, d, "sync=1") + f.setDevice(ctx, d, "active=0") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("0")) + f.setDevice(ctx, d, "active=1") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + Expect(f.deviceAttr(ctx, d, "filename")).To(Equal(path)) + Expect(f.deviceAttr(ctx, d, "size_mb")).To(Equal("16")) + f.setDevice(ctx, d, "sync=1") + }) + + It("rejects an active backing change and recovers from a failed activation", Label("negative"), func(ctx SpecContext) { + first, second := f.backingFile("first"), f.backingFile("second") + d := f.fileDevice(ctx, "disk", first, "thin_provisioned=0") + f.reject(ctx, "-set_dev_attr", d, "-attributes", "filename="+second) + Expect(f.deviceAttr(ctx, d, "filename")).To(Equal(first)) + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + f.reject(ctx, "-set_dev_attr", d, "-attributes", "active=2") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + f.setDevice(ctx, d, "active=0") + f.setDevice(ctx, d, "filename="+f.file("missing.img")) + f.reject(ctx, "-set_dev_attr", d, "-attributes", "active=1") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("0")) + f.setDevice(ctx, d, "filename="+second) + f.setDevice(ctx, d, "active=1") + Expect(f.deviceAttr(ctx, d, "filename")).To(Equal(second)) + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + Expect(f.deviceAttr(ctx, d, "thin_provisioned")).To(Equal("0")) + Expect(f.deviceAttr(ctx, d, "size_mb")).To(Equal("16")) + f.setDevice(ctx, d, "sync=1") + }) + + It("rejects a missing backing file without leaving a device behind", Label("negative"), func(ctx SpecContext) { + neighbor := f.fileDevice(ctx, "neighbor", f.backingFile("neighbor")) + d := f.name("missing") + f.devices = append(f.devices, d) + f.reject(ctx, "-open_dev", d, "-handler", "vdisk_fileio", "-attributes", "filename="+f.file("missing.img")) + Expect(must(cli.Devices(ctx))).To(Equal(map[string]string{neighbor: "vdisk_fileio"})) + Expect(f.deviceAttr(ctx, neighbor, "active")).To(Equal("1")) + // A failed create must not reserve the name. + f.fileDevice(ctx, "missing", f.backingFile("recovered")) + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + }) + + // Issue #13: the explicit TP setting must remain a configuration key even + // when equal to the default. This does not reproduce the reported I/O CRC. + DescribeTable("preserves explicit thin provisioning through save and restore", Label("config", "clear-config", "issue-13"), + func(ctx SpecContext, tp string) { + if tp == "1" && !cfg.FileioThinProvisioning { + Skip("fileio_thin_provisioning=false: TP-enabled coverage explicitly disabled for this filesystem") + } + path := f.backingFile("disk") + d := f.fileDevice(ctx, "disk", path, "thin_provisioned="+tp, "bind_alua_state=1") + attrs := must(cli.Attributes(ctx, "-list_device", d)) + Expect(attrs).To(HaveKey("thin_provisioned")) + Expect(attrs["thin_provisioned"].Key).To(BeTrue()) + Expect(f.deviceAttr(ctx, d, "thin_provisioned")).To(Equal(tp), "TP-enabled coverage requires a capable backing filesystem; see backing_dir and fileio_thin_provisioning") + // Exercise backend reactivation before persisting the setting. + f.setDevice(ctx, d, "active=0") + path = f.backingFile("replacement") + f.setDevice(ctx, d, "filename="+path) + f.setDevice(ctx, d, "active=1") + Expect(f.deviceAttr(ctx, d, "thin_provisioned")).To(Equal(tp)) + saved := f.file("fileio.conf") + f.do(ctx, "-write_config", saved) + f.do(ctx, "-check_config", saved) + f.do(ctx, "-clear_config", "-force") + Expect(cli.RequireEmpty(ctx)).To(Succeed()) + f.do(ctx, "-config", saved) + Expect(must(cli.Devices(ctx))).To(Equal(map[string]string{d: "vdisk_fileio"})) + Expect(f.deviceAttr(ctx, d, "filename")).To(Equal(path)) + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + Expect(f.deviceAttr(ctx, d, "thin_provisioned")).To(Equal(tp)) + Expect(must(cli.Attributes(ctx, "-list_device", d))["thin_provisioned"].Key).To(BeTrue()) + Expect(f.deviceAttr(ctx, d, "bind_alua_state")).To(Equal("1")) + }, Entry("disabled", "0"), Entry("enabled on a capable filesystem", "1")) +}) + +var _ = It("handles sync on an inactive fileio device without losing manageability", Label("fileio", "known-crash", "issue-394"), func(ctx SpecContext) { + if !cfg.AllowKnownCrash { + Skip("issue #394 can crash the kernel; requires allow_known_crash=true and an explicit label selection") + } + f := newFixture() + path := f.backingFile("disk") + d := f.fileDevice(ctx, "disk", path) + f.setDevice(ctx, d, "active=0") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("0")) + // The public contract does not specify rejection versus a no-op. Either + // completed CLI outcome is acceptable; an execution failure is not. + r, err := cli.Execute(ctx, "-set_dev_attr", d, "-attributes", "sync=1") + Expect(err).NotTo(HaveOccurred(), r.String()) + AddReportEntry("inactive sync result", r.String()) + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("0")) + Expect(f.deviceAttr(ctx, d, "filename")).To(Equal(path)) + f.setDevice(ctx, d, "active=1") + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + f.setDevice(ctx, d, "sync=1") +}) diff --git a/tests/ginkgo/suite/fixture_test.go b/tests/ginkgo/suite/fixture_test.go index 100570f35..21cf2ea41 100644 --- a/tests/ginkgo/suite/fixture_test.go +++ b/tests/ginkgo/suite/fixture_test.go @@ -5,7 +5,10 @@ import ( "context" "errors" "fmt" + "os" "path/filepath" + "slices" + "strings" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -16,6 +19,7 @@ import ( type fixture struct { prefix string devices, targets, deviceGroups []string + backingFiles []string } var fixtureNumber int @@ -54,18 +58,61 @@ func (f *fixture) reject(ctx context.Context, args ...string) { } func (f *fixture) device(ctx context.Context, suffix string) string { + GinkgoHelper() + return f.openDevice(ctx, suffix, "vdisk_nullio", "dummy=1,size_mb=8") +} + +func (f *fixture) openDevice(ctx context.Context, suffix, handler, attributes string) string { GinkgoHelper() d := f.name(suffix) // Record ownership before invoking a mutation, including partial failures. - f.devices = append(f.devices, d) - f.do(ctx, "-open_dev", d, "-handler", "vdisk_nullio", "-attributes", "dummy=1,size_mb=8") + if !slices.Contains(f.devices, d) { + f.devices = append(f.devices, d) + } + f.do(ctx, "-open_dev", d, "-handler", handler, "-attributes", attributes) return d } +// Only newly created files belong to this fixture. Preserve them if SCST +// cleanup fails: unlinking a still-open backing file would hide failure state. +func (f *fixture) backingFile(suffix string) string { + GinkgoHelper() + dir := cfg.BackingDir + if dir == "" { + dir = artifactDir + } + file := must(os.CreateTemp(dir, f.name(suffix)+"-*.img")) + path := file.Name() + f.backingFiles = append(f.backingFiles, path) + err := file.Truncate(16 * 1024 * 1024) + closeErr := file.Close() + Expect(err).NotTo(HaveOccurred()) + Expect(closeErr).NotTo(HaveOccurred()) + Expect(strings.ContainsAny(path, " \t\r\n,;\"{}\\")).To(BeFalse(), "backing path must be usable in scstadmin attributes/configuration: %s", path) + return path +} + +func (f *fixture) fileDevice(ctx context.Context, suffix, path string, attributes ...string) string { + GinkgoHelper() + return f.openDevice(ctx, suffix, "vdisk_fileio", strings.Join(append([]string{"filename=" + path}, attributes...), ",")) +} + +func (f *fixture) deviceAttr(ctx context.Context, device, attribute string) string { + GinkgoHelper() + return must(cli.Scalar(ctx, attribute, "-list_device", device)) +} + +func (f *fixture) setDevice(ctx context.Context, device, attributes string) { + GinkgoHelper() + f.do(ctx, "-set_dev_attr", device, "-attributes", attributes) +} + func (f *fixture) target(ctx context.Context, suffix string, attributes ...string) string { GinkgoHelper() t := f.name(suffix) - f.targets = append(f.targets, t) + if !slices.Contains(f.targets, t) { + f.targets = append(f.targets, t) + } args := []string{"-add_target", t, "-driver", "scst_local"} if len(attributes) != 0 { args = append(args, "-attributes", attributes[0]) @@ -153,8 +200,8 @@ func (f *fixture) cleanup(ctx context.Context) error { return errors.Join(append(problems, err)...) } for _, d := range f.devices { - if _, ok := devices[d]; ok { - remove("-close_dev", d, "-handler", "vdisk_nullio") + if handler, ok := devices[d]; ok { + remove("-close_dev", d, "-handler", handler) } } if len(problems) != 0 { @@ -176,5 +223,10 @@ func (f *fixture) cleanup(ctx context.Context) error { if len(devices) != 0 || len(targets) != 0 || len(dgs) != 0 { return fmt.Errorf("host is no longer empty: devices=%v targets=%v device_groups=%v", devices, targets, dgs) } + for _, path := range f.backingFiles { + if err := os.Remove(path); err != nil { + return fmt.Errorf("remove backing file: %w", err) + } + } return nil } diff --git a/tests/ginkgo/suite/lifecycle_test.go b/tests/ginkgo/suite/lifecycle_test.go new file mode 100644 index 000000000..6703d5c47 --- /dev/null +++ b/tests/ginkgo/suite/lifecycle_test.go @@ -0,0 +1,156 @@ +// SPDX-License-Identifier: GPL-2.0-only +package suite + +import ( + "context" + "strconv" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "scst-component-tests/internal/admin" + "scst-component-tests/internal/testcfg" +) + +func (f *fixture) sessionLUNs(ctx context.Context, target, session string, expected map[int]string) { + GinkgoHelper() + Eventually(func(g Gomega) { + sessions, err := cli.SessionDetails(ctx) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(sessions).To(HaveKey("scst_local/" + target)) + g.Expect(sessions["scst_local/"+target]).To(HaveKey(session)) + g.Expect(sessions["scst_local/"+target][session].LUNs).To(Equal(expected)) + }, testcfg.Duration(cfg.SettleTimeout), testcfg.Duration(cfg.PollInterval)).WithContext(ctx).Should(Succeed()) +} + +var _ = Describe("Relative target IDs", Label("basic", "targets", "negative", "issue-230"), func() { + It("rejects conflicts and invalid IDs, then reuses an ID after target removal", func(ctx SpecContext) { + f := newFixture() + a, b, d := f.target(ctx, "first"), f.target(ctx, "second"), f.device(ctx, "disk") + f.at(ctx, b, "", "-add_lun", "0", "-device", d) + // Account for the internal copy-manager target as well as local targets. + targets := must(admin.Pairs(must(cli.Query(ctx, "-list_target")), "Driver", "Target")) + used := map[string]bool{} + for name, driver := range targets { + used[must(cli.Scalar(ctx, "rel_tgt_id", "-list_tgt_attr", name, "-driver", driver))] = true + } + var ids []string + for n := 60000; len(ids) < 2; n++ { + id := strconv.Itoa(n) + if !used[id] { + ids = append(ids, id) + } + } + set := func(target, id string) { + f.do(ctx, "-set_tgt_attr", target, "-driver", "scst_local", "-attributes", "rel_tgt_id="+id) + } + get := func(target string) string { + return must(cli.Scalar(ctx, "rel_tgt_id", "-list_tgt_attr", target, "-driver", "scst_local")) + } + set(a, ids[0]) + set(b, ids[1]) + for _, invalid := range []string{ids[0], "0", "65536"} { + f.reject(ctx, "-set_tgt_attr", b, "-driver", "scst_local", "-attributes", "rel_tgt_id="+invalid) + Expect(get(a)).To(Equal(ids[0])) + Expect(get(b)).To(Equal(ids[1])) + Expect(f.luns(ctx, b, "")).To(Equal(map[int]string{0: d})) + } + f.do(ctx, "-rem_target", a, "-driver", "scst_local") + set(b, ids[0]) + Expect(get(b)).To(Equal(ids[0])) + Expect(f.luns(ctx, b, "")).To(Equal(map[int]string{0: d})) + }) +}) + +// The CLI's session LUN table reports the selected group's mapping. It cannot +// prove SCSI command routing or inspect the lifetime of each kernel tgt_dev. +func (f *fixture) exerciseGroups(ctx context.Context, iterations int) { + GinkgoHelper() + session := f.name("session") + t := f.target(ctx, "target", "session_name="+session) + a, b := f.name("first"), f.name("second") + da, db := f.device(ctx, "first"), f.device(ctx, "second") + f.at(ctx, t, "", "-add_group", b) + f.at(ctx, t, b, "-add_lun", "0", "-device", db) + for i := 0; i < iterations; i++ { + f.at(ctx, t, "", "-add_group", a) + f.at(ctx, t, a, "-add_lun", "0", "-device", da) + f.at(ctx, t, a, "-add_init", session) + f.sessionLUNs(ctx, t, session, map[int]string{0: da}) + f.at(ctx, t, a, "-move_init", session, "-to", b) + f.sessionLUNs(ctx, t, session, map[int]string{0: db}) + Expect(f.state(ctx, t).Groups[a].Initiators).To(BeEmpty()) + Expect(f.state(ctx, t).Groups[b].Initiators).To(ConsistOf(session)) + f.at(ctx, t, b, "-move_init", session, "-to", a) + f.sessionLUNs(ctx, t, session, map[int]string{0: da}) + before := f.state(ctx, t) + for _, force := range []bool{false, true} { + args := []string{"-rem_group", a} + if force { + args = append(args, "-force") + } + f.reject(ctx, append(args, scope(t, "")...)...) + Expect(f.state(ctx, t)).To(Equal(before)) + f.sessionLUNs(ctx, t, session, map[int]string{0: da}) + } + // -force bypasses the CLI's populated-group check, not the kernel's + // live-session check with forcibly_close_sessions disabled. Move the + // session away before deleting the group and its remaining LUNs. + f.at(ctx, t, a, "-move_init", session, "-to", b) + f.sessionLUNs(ctx, t, session, map[int]string{0: db}) + f.at(ctx, t, "", "-rem_group", a, "-force") + Expect(f.state(ctx, t).Groups).NotTo(HaveKey(a)) + Expect(must(cli.Sessions(ctx))["scst_local/"+t]).To(ConsistOf(session)) + f.sessionLUNs(ctx, t, session, map[int]string{0: db}) + f.at(ctx, t, b, "-rem_init", session) + Expect(f.state(ctx, t).Groups[b].Initiators).To(BeEmpty()) + } + f.do(ctx, "-rem_target", t, "-driver", "scst_local") + Expect(must(cli.Sessions(ctx))).NotTo(HaveKey("scst_local/" + t)) + Expect(must(cli.Devices(ctx))).To(HaveLen(2)) +} + +var _ = It("rejects deletion of a live session's group and removes it after moving the session", Label("basic", "groups", "sessions", "negative", "issue-179"), func(ctx SpecContext) { + newFixture().exerciseGroups(ctx, 1) +}) + +var _ = It("repeatedly recreates groups while keeping a local session alive", Label("stress", "groups", "sessions", "issue-179"), func(ctx SpecContext) { + newFixture().exerciseGroups(ctx, cfg.StressIterations) +}) + +func copyManagerLUNs(ctx context.Context) map[int]string { + GinkgoHelper() + return must(cli.DriverAssignments(ctx, "copy_manager", "copy_manager_tgt")).LUNs +} + +var _ = It("rolls back a rejected read-only copy-manager mapping and permits a valid retry", Label("basic", "copy-manager", "negative", "issue-204"), func(ctx SpecContext) { + f := newFixture() + d, neighbor := f.device(ctx, "disk"), f.device(ctx, "neighbor") + original := copyManagerLUNs(ctx) + Expect(original).To(HaveLen(2)) + lun := -1 + for n, device := range original { + if device == d { + lun = n + } + } + Expect(lun).To(BeNumerically(">=", 0)) + args := []string{"-driver", "copy_manager", "-target", "copy_manager_tgt"} + f.do(ctx, append([]string{"-rem_lun", strconv.Itoa(lun)}, args...)...) + delete(original, lun) + Expect(copyManagerLUNs(ctx)).To(Equal(original)) + // This reaches the management rollback path with nullio. The original + // issue's stale per-LUN thread_pid node is not observable via scstadmin. + f.reject(ctx, append([]string{"-add_lun", strconv.Itoa(lun), "-device", d, "-attributes", "read_only=1"}, args...)...) + Expect(copyManagerLUNs(ctx)).To(Equal(original)) + Expect(must(cli.Devices(ctx))).To(Equal(map[string]string{d: "vdisk_nullio", neighbor: "vdisk_nullio"})) + Expect(must(cli.Sessions(ctx))["copy_manager/copy_manager_tgt"]).To(ConsistOf("copy_manager_sess")) + f.do(ctx, append([]string{"-add_lun", strconv.Itoa(lun), "-device", d}, args...)...) + original[lun] = d + Expect(copyManagerLUNs(ctx)).To(Equal(original)) + f.do(ctx, append([]string{"-rem_lun", strconv.Itoa(lun)}, args...)...) + f.do(ctx, "-close_dev", d, "-handler", "vdisk_nullio") + delete(original, lun) + Expect(copyManagerLUNs(ctx)).To(Equal(original)) + Expect(must(cli.Devices(ctx))).To(Equal(map[string]string{neighbor: "vdisk_nullio"})) +}) diff --git a/tests/ginkgo/suite/stress_test.go b/tests/ginkgo/suite/stress_test.go new file mode 100644 index 000000000..1b94ab3be --- /dev/null +++ b/tests/ginkgo/suite/stress_test.go @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: GPL-2.0-only +package suite + +import ( + "context" + "fmt" + "os" + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func (f *fixture) exerciseIdentifiers(ctx context.Context, iterations int) { + GinkgoHelper() + neighbor := f.device(ctx, "neighbor") + neighborID := f.deviceAttr(ctx, neighbor, "t10_dev_id") + neighborUSN := f.deviceAttr(ctx, neighbor, "usn") + for i := 0; i < iterations; i++ { + a, b := f.device(ctx, "first"), f.device(ctx, "second") + before := copyManagerLUNs(ctx) + Expect(before).To(HaveLen(3)) + // Updating separate devices schedules independent inquiry work. CLI + // calls are serialized; this is not a deterministic race reproducer. + for j, d := range []string{a, b} { + id := fmt.Sprintf("%s%03x%d", runID, i, j) + f.setDevice(ctx, d, "t10_dev_id="+id+",usn="+id) + } + for j, d := range []string{a, b} { + id := fmt.Sprintf("%s%03x%d", runID, i, j) + Expect(f.deviceAttr(ctx, d, "t10_dev_id")).To(Equal(id)) + Expect(f.deviceAttr(ctx, d, "usn")).To(Equal(id)) + } + Expect(copyManagerLUNs(ctx)).To(Equal(before)) + for _, d := range []string{a, b} { + f.do(ctx, "-close_dev", d, "-handler", "vdisk_nullio") + } + Expect(must(cli.Devices(ctx))).To(Equal(map[string]string{neighbor: "vdisk_nullio"})) + remaining := copyManagerLUNs(ctx) + Expect(remaining).To(HaveLen(1)) + for _, d := range remaining { + Expect(d).To(Equal(neighbor)) + } + Expect(f.deviceAttr(ctx, neighbor, "t10_dev_id")).To(Equal(neighborID)) + Expect(f.deviceAttr(ctx, neighbor, "usn")).To(Equal(neighborUSN)) + } +} + +var _ = It("updates device identifiers without changing neighboring devices or copy-manager mappings", Label("basic", "devices", "copy-manager", "issue-99"), func(ctx SpecContext) { + newFixture().exerciseIdentifiers(ctx, 1) +}) + +var _ = It("repeatedly updates identifiers and recreates copy-manager devices", Label("stress", "devices", "copy-manager", "issue-99"), func(ctx SpecContext) { + newFixture().exerciseIdentifiers(ctx, cfg.StressIterations) +}) + +var _ = It("removes a local session with many LUNs and reuses its target name", Label("stress", "scale", "sessions", "issue-229"), func(ctx SpecContext) { + f := newFixture() + d, neighbor := f.device(ctx, "disk"), f.device(ctx, "neighbor") + session := f.name("session") + t := f.target(ctx, "target", "session_name="+session) + g := f.name("group") + var config strings.Builder + fmt.Fprintf(&config, "HANDLER vdisk_nullio {\n DEVICE %s {\n dummy 1\n size_mb 8\n }\n}\n", d) + fmt.Fprintf(&config, "TARGET_DRIVER scst_local {\n TARGET %s {\n GROUP %s {\n INITIATOR %s\n", t, g, session) + expected := map[int]string{} + for n := 0; n < cfg.ScaleLUNs; n++ { + fmt.Fprintf(&config, " LUN %d %s\n", n, d) + expected[n] = d + } + config.WriteString(" }\n }\n}\n") + path := f.file("many-luns.conf") + Expect(os.WriteFile(path, []byte(config.String()), 0600)).To(Succeed()) + f.do(ctx, "-config", path) + Expect(f.luns(ctx, t, g)).To(Equal(expected)) + f.sessionLUNs(ctx, t, session, expected) + r, err := cli.Execute(ctx, "-rem_target", t, "-driver", "scst_local") + Expect(err).NotTo(HaveOccurred(), r.String()) + Expect(r.ExitCode).To(BeZero(), r.String()) + Expect(r.ReportedError()).To(BeFalse(), r.String()) + AddReportEntry("session teardown", fmt.Sprintf("%d LUNs: %s (includes scstadmin overhead)", cfg.ScaleLUNs, r.Duration)) + Expect(must(cli.Targets(ctx))).NotTo(HaveKey(t)) + Expect(must(cli.Sessions(ctx))).NotTo(HaveKey("scst_local/" + t)) + Expect(must(cli.Devices(ctx))).To(Equal(map[string]string{d: "vdisk_nullio", neighbor: "vdisk_nullio"})) + // Reusing the name and attaching a fresh session exposes lingering state. + f.target(ctx, "target", "session_name="+session) + f.at(ctx, t, "", "-add_group", g) + f.at(ctx, t, g, "-add_lun", "0", "-device", neighbor) + f.at(ctx, t, g, "-add_init", session) + f.sessionLUNs(ctx, t, session, map[int]string{0: neighbor}) +}) diff --git a/tests/ginkgo/suite/suite_test.go b/tests/ginkgo/suite/suite_test.go index c01e22ffd..369a06565 100644 --- a/tests/ginkgo/suite/suite_test.go +++ b/tests/ginkgo/suite/suite_test.go @@ -52,7 +52,7 @@ var _ = BeforeSuite(func(ctx SpecContext) { drivers, err := cli.Names(ctx, "Driver", "-list_driver") Expect(err).NotTo(HaveOccurred(), "scstadmin must be able to query the modules loaded by the suite") Expect(drivers).To(ContainElement("scst_local"), "prepare scst_local with add_default_tgt=0") - Expect(must(cli.Names(ctx, "Handler", "-list_handler"))).To(ContainElement("vdisk_nullio")) + Expect(must(cli.Names(ctx, "Handler", "-list_handler"))).To(ContainElements("vdisk_nullio", "vdisk_fileio")) Expect(cli.RequireEmpty(ctx)).To(Succeed()) })