diff --git a/tests/ginkgo/README.md b/tests/ginkgo/README.md index 4280c2efc..ee22f7ad4 100644 --- a/tests/ginkgo/README.md +++ b/tests/ginkgo/README.md @@ -52,16 +52,15 @@ the backend. For example, `local && io && fileio` needs no physical disk. ```sh # List every spec and its labels without accessing either host: make ginkgo-list GINKGO_LABEL='' -# All except the known-crash case; prepare both hosts and a block device first: -make ginkgo-test-build GINKGO_CONFIG=tests/ginkgo/config.local.json GINKGO_LABEL='!known-crash' +# All specs; prepare both hosts and a block device first: +make ginkgo-test-build GINKGO_CONFIG=tests/ginkgo/config.local.json GINKGO_LABEL='' ``` -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`. -Only selected transport groups access hosts. Local teardown finishes before -iSCSI setup; an empty selection or suite failure stops the run. Parallel runs -are unsupported: use the same `lock_file` and runner user on each host. +An empty label filter selects all specs. `clear-config` tests invoke global +`scstadmin -clear_config -force`. Only selected transport groups access hosts. +Local teardown finishes before iSCSI setup; an empty selection or suite +failure stops the run. Parallel runs are unsupported: use the same +`lock_file` and runner user on each host. ## Configuration diff --git a/tests/ginkgo/config.example.json b/tests/ginkgo/config.example.json index 9864a18f9..cf78d6bbe 100644 --- a/tests/ginkgo/config.example.json +++ b/tests/ginkgo/config.example.json @@ -15,7 +15,6 @@ "fileio_thin_provisioning": true, "stress_iterations": 8, "scale_luns": 256, - "allow_known_crash": false, "io": { "block_device": "", "region_mb": 64, diff --git a/tests/ginkgo/internal/testcfg/config.go b/tests/ginkgo/internal/testcfg/config.go index 306e2a56e..a93871b32 100644 --- a/tests/ginkgo/internal/testcfg/config.go +++ b/tests/ginkgo/internal/testcfg/config.go @@ -32,7 +32,6 @@ type Config struct { FileioThinProvisioning bool `json:"fileio_thin_provisioning"` StressIterations int `json:"stress_iterations"` ScaleLUNs int `json:"scale_luns"` - AllowKnownCrash bool `json:"allow_known_crash"` IO IO `json:"io"` ISCSI *ISCSI `json:"iscsi,omitempty"` } diff --git a/tests/ginkgo/internal/testcfg/config_test.go b/tests/ginkgo/internal/testcfg/config_test.go index 40ffdab38..18448b833 100644 --- a/tests/ginkgo/internal/testcfg/config_test.go +++ b/tests/ginkgo/internal/testcfg/config_test.go @@ -29,7 +29,7 @@ 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") { + if !cfg.FileioThinProvisioning || cfg.StressIterations != 8 || cfg.ScaleLUNs != 256 || cfg.BackingDir != filepath.Join(base, "scratch") { t.Fatalf("unsafe or missing scenario defaults: %+v", cfg) } } @@ -72,11 +72,11 @@ 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}`)) + c, err := Load(configFile(t, `{"stress_iterations":12,"scale_luns":1000,"fileio_thin_provisioning":false}`)) if err != nil { t.Fatal(err) } - if c.StressIterations != 12 || c.ScaleLUNs != 1000 || !c.AllowKnownCrash || c.FileioThinProvisioning || c.BackingDir != "" { + if c.StressIterations != 12 || c.ScaleLUNs != 1000 || 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 index ca768b421..f7dec9067 100644 --- a/tests/ginkgo/suite/alua_state_test.go +++ b/tests/ginkgo/suite/alua_state_test.go @@ -31,9 +31,14 @@ var _ = Describe("ALUA device activation", Label("basic", "alua", "fileio", "iss 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") + session := f.name("session") + t, dg := f.target(ctx, "target", "session_name="+session), f.deviceGroup(ctx, "devices") + f.detachCopyManager(ctx, d) if mapped { - f.at(ctx, t, "", "-add_lun", "0", "-device", d) + f.at(ctx, t, "", "-add_group", "access") + f.at(ctx, t, "access", "-add_lun", "0", "-device", d) + f.at(ctx, t, "access", "-add_init", session) + f.sessionLUNs(ctx, t, session, map[int]string{0: d}) } f.do(ctx, "-add_dgrp_dev", d, "-dev_group", dg) tg := f.targetGroup(ctx, dg, "local", t, "11") @@ -41,15 +46,21 @@ var _ = Describe("ALUA device activation", Label("basic", "alua", "fileio", "iss Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) f.aluaState(ctx, dg, tg, "transitioning") Expect(f.deviceAttr(ctx, d, "active")).To(Equal("0")) + f.reject(ctx, "-set_dev_attr", d, "-attributes", "sync=1") 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") + if mapped { + f.setDevice(ctx, d, "sync=1") + } else { + f.reject(ctx, "-set_dev_attr", d, "-attributes", "sync=1") + } f.aluaState(ctx, dg, tg, "standby") Expect(f.deviceAttr(ctx, d, "active")).To(Equal("0")) + f.reject(ctx, "-set_dev_attr", d, "-attributes", "sync=1") 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") @@ -57,13 +68,14 @@ var _ = Describe("ALUA device activation", Label("basic", "alua", "fileio", "iss 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})) + Expect(f.luns(ctx, t, "access")).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") + t, _ := f.attachFileio(ctx, d) + dg := 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") @@ -78,7 +90,8 @@ var _ = Describe("ALUA device activation", Label("basic", "alua", "fileio", "iss 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") + t, _ := f.attachFileio(ctx, d) + dg := 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") diff --git a/tests/ginkgo/suite/fileio_test.go b/tests/ginkgo/suite/fileio_test.go index 360491606..f66f3c89d 100644 --- a/tests/ginkgo/suite/fileio_test.go +++ b/tests/ginkgo/suite/fileio_test.go @@ -2,10 +2,37 @@ package suite import ( + "context" + "strconv" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) +// Drop only this fixture's copy-manager mappings. Without this, a device can +// retain an open backend after the last external LUN has been removed. +func (f *fixture) detachCopyManager(ctx context.Context, device string) { + GinkgoHelper() + for lun, d := range copyManagerLUNs(ctx) { + if d == device { + f.do(ctx, "-rem_lun", strconv.Itoa(lun), "-driver", "copy_manager", "-target", "copy_manager_tgt") + } + } + Expect(copyManagerLUNs(ctx)).NotTo(ContainElement(device)) +} + +func (f *fixture) attachFileio(ctx context.Context, device string) (string, string) { + GinkgoHelper() + session := f.name("session") + target := f.target(ctx, "target", "session_name="+session) + // scstadmin reports session LUNs only for explicit initiator groups. + f.at(ctx, target, "", "-add_group", "access") + f.at(ctx, target, "access", "-add_lun", "0", "-device", device) + f.at(ctx, target, "access", "-add_init", session) + f.sessionLUNs(ctx, target, session, map[int]string{0: device}) + return target, session +} + var _ = Describe("File-backed devices", Label("basic", "fileio"), func() { var f *fixture BeforeEach(func() { f = newFixture() }) @@ -13,6 +40,7 @@ var _ = Describe("File-backed devices", Label("basic", "fileio"), func() { It("syncs an active device and keeps its configuration across reactivation", func(ctx SpecContext) { path := f.backingFile("disk") d := f.fileDevice(ctx, "disk", path) + f.attachFileio(ctx, d) Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) f.setDevice(ctx, d, "sync=1") f.setDevice(ctx, d, "active=0") @@ -27,6 +55,7 @@ var _ = Describe("File-backed devices", Label("basic", "fileio"), func() { 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.attachFileio(ctx, d) 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")) @@ -91,23 +120,42 @@ var _ = Describe("File-backed devices", Label("basic", "fileio"), func() { }, 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") - } +var _ = It("handles sync on an inactive fileio device without losing manageability", Label("basic", "fileio", "negative", "issue-394"), func(ctx SpecContext) { f := newFixture() path := f.backingFile("disk") d := f.fileDevice(ctx, "disk", path) + f.attachFileio(ctx, d) 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()) + f.reject(ctx, "-set_dev_attr", d, "-attributes", "sync=1") Expect(f.deviceAttr(ctx, d, "active")).To(Equal("0")) Expect(f.deviceAttr(ctx, d, "filename")).To(Equal(path)) + Expect(f.deviceAttr(ctx, d, "size_mb")).To(Equal("16")) f.setDevice(ctx, d, "active=1") Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) f.setDevice(ctx, d, "sync=1") }) + +var _ = It("rejects sync after the last attachment and reopens on a new attachment", Label("basic", "fileio", "negative", "issue-394"), func(ctx SpecContext) { + f := newFixture() + path := f.backingFile("disk") + d := f.fileDevice(ctx, "disk", path) + f.detachCopyManager(ctx, d) + Expect(f.deviceAttr(ctx, d, "active")).To(Equal("1")) + f.reject(ctx, "-set_dev_attr", d, "-attributes", "sync=1") + // Reactivation alone must not invent a backend attachment. + f.setDevice(ctx, d, "active=0") + f.setDevice(ctx, d, "active=1") + f.reject(ctx, "-set_dev_attr", d, "-attributes", "sync=1") + target, session := f.attachFileio(ctx, d) + f.setDevice(ctx, d, "sync=1") + f.at(ctx, target, "access", "-rem_lun", "0") + f.sessionLUNs(ctx, target, session, map[int]string{}) + f.reject(ctx, "-set_dev_attr", d, "-attributes", "sync=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.at(ctx, target, "access", "-add_lun", "0", "-device", d) + f.sessionLUNs(ctx, target, session, map[int]string{0: d}) + f.setDevice(ctx, d, "sync=1") +})