diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 9e834ec1..7ceaca77 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -71,7 +71,7 @@ func main() { var controllerNamespace string var tlsOpts []func(*tls.Config) flag.BoolVar(&enableRestAPI, "enable-rest-api", false, - "Mount the LINSTOR-compatible REST API alongside the reconcilers. Default is OFF — the apiserver runs as a separate Deployment (cmd/apiserver) since Phase 11.x. Set to true only for the legacy single-binary deployment.") + "Mount the LINSTOR-compatible REST API alongside the reconcilers. Default is off: the REST API runs as the separate apiserver Deployment (cmd/apiserver). Set it only for a single-binary deployment.") flag.StringVar(&restAddr, "rest-bind-address", defaultPlainBindAddress, "The address the plain-HTTP LINSTOR-compatible REST API binds to. It has no authentication, so the default is loopback, "+ "reachable through `kubectl port-forward` only. Ignored when --enable-rest-api=false.") diff --git a/pkg/api/v1/resource_definition.go b/pkg/api/v1/resource_definition.go index 596861bc..715359f4 100644 --- a/pkg/api/v1/resource_definition.go +++ b/pkg/api/v1/resource_definition.go @@ -224,9 +224,9 @@ type ResourceDefinitionCreate struct { // bypasses the LUKS-prereq gate. LayerList []string `json:"layer_list,omitempty"` - // OverrideProps mirrors GenericPropsModify — golinstor stuffs RG - // spawn defaults into this when creating an RD via spawn, so the - // REST handler must accept it even if we don't yet diff the values. + // The props-modify fields are declared so the decoder can name them; + // a non-empty one is refused, since a create has no props to modify. + // Props for a new definition go in resource_definition.props. OverrideProps map[string]string `json:"override_props,omitempty"` DeleteProps []string `json:"delete_props,omitempty"` DeleteNamespace []string `json:"delete_namespaces,omitempty"` diff --git a/pkg/api/v1/resource_group.go b/pkg/api/v1/resource_group.go index 5b7d84de..f3250804 100644 --- a/pkg/api/v1/resource_group.go +++ b/pkg/api/v1/resource_group.go @@ -97,10 +97,12 @@ type AutoSelectFilter struct { OverrideVlmID string `json:"override_vlm_id,omitempty"` } -// ResourceGroupSpawn is the payload for POST /resource-groups/{rg}/spawn — -// the call linstor-csi uses to actually create a Resource from a group. -// Upstream LINSTOR reuses GenericPropsModify here too, so we accept the -// override_props / delete_props fields even though we don't consume them. +// ResourceGroupSpawn is the payload for POST /resource-groups/{rg}/spawn, +// behind `linstor rg spawn`: it creates a definition, its volumes and, +// unless definitions_only is set, its replicas from a group. +// The props-modify fields are declared so the decoder can name them, and a +// non-empty one is refused: a spawn creates the definition, there are no +// props on it to modify yet. type ResourceGroupSpawn struct { ResourceDefinitionName string `json:"resource_definition_name,omitempty"` ResourceDefinitionExternal string `json:"resource_definition_external_name,omitempty"` diff --git a/pkg/rest/autoplace.go b/pkg/rest/autoplace.go index 2840a0b8..0830c5ce 100644 --- a/pkg/rest/autoplace.go +++ b/pkg/rest/autoplace.go @@ -1707,7 +1707,7 @@ func (s *Server) refuseResourceCreateOnSPDeletedRace(w http.ResponseWriter, r *h writeError(w, http.StatusNotFound, "storage pool '"+pool+"' was deleted on node '"+res.NodeName+ - "' concurrently with the resource create (Bug 145): "+ + "' concurrently with the resource create: "+ "retry after re-creating the pool, or pick a different pool") return false @@ -1745,7 +1745,7 @@ func (s *Server) refuseResourceCreateOnNodeDeletedRace(w http.ResponseWriter, r writeError(w, http.StatusNotFound, "node '"+res.NodeName+"' was deleted concurrently with "+ - "the resource create (Bug 174): retry after re-creating "+ + "the resource create: retry after re-creating "+ "the node, or pick a different node") return false diff --git a/pkg/rest/create_doors_props_modify_test.go b/pkg/rest/create_doors_props_modify_test.go new file mode 100644 index 00000000..bc20ae40 --- /dev/null +++ b/pkg/rest/create_doors_props_modify_test.go @@ -0,0 +1,177 @@ +// SPDX-License-Identifier: Apache-2.0 + +/* +Copyright 2026 Cozystack contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rest + +import ( + "encoding/json" + "errors" + "io" + "net/http" + "strings" + "testing" + + apiv1 "github.com/cozystack/blockstor/pkg/api/v1" + "github.com/cozystack/blockstor/pkg/store" +) + +// The props-modify fields edit props a definition already has. On a door +// that creates the definition there is nothing to edit, and answering +// success while dropping them claimed an edit that never happened. +func propsModifyCases() map[string]func(override *map[string]string, del, ns *[]string) { + return map[string]func(override *map[string]string, del, ns *[]string){ + "override_props": func(o *map[string]string, _, _ *[]string) { *o = map[string]string{"Aux/k": "v"} }, + "delete_props": func(_ *map[string]string, d, _ *[]string) { *d = []string{"Aux/k"} }, + "delete_namespaces": func(_ *map[string]string, _, n *[]string) { *n = []string{"Aux"} }, + } +} + +func TestRDCreateRefusesPropsModifyFields(t *testing.T) { + t.Parallel() + + for field, set := range propsModifyCases() { + t.Run(field, func(t *testing.T) { + t.Parallel() + + st := store.NewInMemory() + base, stop := startServerWithStore(t, st) + defer stop() + + body := apiv1.ResourceDefinitionCreate{ + ResourceDefinition: apiv1.ResourceDefinition{Name: "rd-modify"}, + } + set(&body.OverrideProps, &body.DeleteProps, &body.DeleteNamespace) + + assertRefusedNamingField(t, base+"/v1/resource-definitions", body, field) + + _, err := st.ResourceDefinitions().Get(t.Context(), "rd-modify") + if !errors.Is(err, store.ErrNotFound) { + t.Fatalf("a refused create left the definition behind: err=%v", err) + } + }) + } +} + +func TestSpawnRefusesPropsModifyFields(t *testing.T) { + t.Parallel() + + for field, set := range propsModifyCases() { + t.Run(field, func(t *testing.T) { + t.Parallel() + + st := store.NewInMemory() + if err := st.ResourceGroups().Create(t.Context(), &apiv1.ResourceGroup{Name: "rg-modify"}); err != nil { + t.Fatalf("seed group: %v", err) + } + + base, stop := startServerWithStore(t, st) + defer stop() + + body := apiv1.ResourceGroupSpawn{ResourceDefinitionName: "rd-modify", VolumeSizes: []int64{4096}} + set(&body.OverrideProps, &body.DeleteProps, &body.DeleteNamespace) + + assertRefusedNamingField(t, base+"/v1/resource-groups/rg-modify/spawn", body, field) + + _, err := st.ResourceDefinitions().Get(t.Context(), "rd-modify") + if !errors.Is(err, store.ErrNotFound) { + t.Fatalf("a refused spawn left the definition behind: err=%v", err) + } + }) + } +} + +// `linstor rg spawn --definition-only` asks for the definition and its +// volumes and no placement. The handler placed replicas anyway. +func TestSpawnDefinitionsOnlyPlacesNothing(t *testing.T) { + t.Parallel() + + st := store.NewInMemory() + ctx := t.Context() + + if err := st.ResourceGroups().Create(ctx, &apiv1.ResourceGroup{ + Name: "rg-defs", + SelectFilter: apiv1.AutoSelectFilter{PlaceCount: 2, StoragePool: "pool"}, + }); err != nil { + t.Fatalf("seed group: %v", err) + } + + for _, node := range []string{"n1", "n2"} { + if err := st.StoragePools().Create(ctx, &apiv1.StoragePool{ + StoragePoolName: "pool", NodeName: node, ProviderKind: apiv1.StoragePoolKindLVMThin, + FreeCapacity: 1_000_000, TotalCapacity: 10_000_000, + }); err != nil { + t.Fatalf("seed pool: %v", err) + } + } + + base, stop := startServerWithStore(t, st) + defer stop() + + raw, err := json.Marshal(apiv1.ResourceGroupSpawn{ + ResourceDefinitionName: "rd-defs", + VolumeSizes: []int64{4096}, + DefinitionsOnly: true, + }) + if err != nil { + t.Fatalf("marshal: %v", err) + } + + resp := httpPost(t, base+"/v1/resource-groups/rg-defs/spawn", raw) + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated { + b, _ := io.ReadAll(resp.Body) + t.Fatalf("status %d, want 201: %s", resp.StatusCode, b) + } + + vds, err := st.VolumeDefinitions().List(ctx, "rd-defs") + if err != nil || len(vds) != 1 { + t.Fatalf("volume definitions: %+v, err=%v; want one", vds, err) + } + + replicas, err := st.Resources().ListByDefinition(ctx, "rd-defs") + if err != nil { + t.Fatalf("list replicas: %v", err) + } + + if len(replicas) != 0 { + t.Fatalf("definitions_only spawn placed %d replicas, want none", len(replicas)) + } +} + +func assertRefusedNamingField(t *testing.T, url string, body any, field string) { + t.Helper() + + raw, err := json.Marshal(body) + if err != nil { + t.Fatalf("marshal: %v", err) + } + + resp := httpPost(t, url, raw) + defer func() { _ = resp.Body.Close() }() + + got, _ := io.ReadAll(resp.Body) + + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("status %d, want 400: %s", resp.StatusCode, got) + } + + if !strings.Contains(string(got), field) { + t.Fatalf("the refusal does not name %s: %s", field, got) + } +} diff --git a/pkg/rest/flags_validation.go b/pkg/rest/flags_validation.go index f05f0ee2..6835e0ed 100644 --- a/pkg/rest/flags_validation.go +++ b/pkg/rest/flags_validation.go @@ -156,7 +156,7 @@ func init() { for _, p := range pairs { if p.local != p.exported { - panic("Bug-167 flag-allowlist desync: local " + p.name + " != apiv1 constant") + panic("flag allowlist out of sync with apiv1: local " + p.name + " != apiv1 constant") } } } diff --git a/pkg/rest/kv_store.go b/pkg/rest/kv_store.go index c3c06bdc..7a1b91aa 100644 --- a/pkg/rest/kv_store.go +++ b/pkg/rest/kv_store.go @@ -212,7 +212,7 @@ func decodeKVModifyBody(w http.ResponseWriter, r *http.Request) (apiv1.GenericPr for key := range raw { switch key { - case "override_props", "delete_props", "delete_namespaces": + case fieldOverrideProps, fieldDeleteProps, fieldDeleteNamespaces: continue default: writeError(w, http.StatusBadRequest, @@ -224,7 +224,7 @@ func decodeKVModifyBody(w http.ResponseWriter, r *http.Request) (apiv1.GenericPr } } - if rawOverride, ok := raw["override_props"]; ok { + if rawOverride, ok := raw[fieldOverrideProps]; ok { err = json.Unmarshal(rawOverride, &patch.OverrideProps) if err != nil { writeError(w, http.StatusBadRequest, "decode override_props: "+err.Error()) @@ -233,7 +233,7 @@ func decodeKVModifyBody(w http.ResponseWriter, r *http.Request) (apiv1.GenericPr } } - if rawDelete, ok := raw["delete_props"]; ok { + if rawDelete, ok := raw[fieldDeleteProps]; ok { err = json.Unmarshal(rawDelete, &patch.DeleteProps) if err != nil { writeError(w, http.StatusBadRequest, "decode delete_props: "+err.Error()) @@ -242,7 +242,7 @@ func decodeKVModifyBody(w http.ResponseWriter, r *http.Request) (apiv1.GenericPr } } - if rawNS, ok := raw["delete_namespaces"]; ok { + if rawNS, ok := raw[fieldDeleteNamespaces]; ok { err = json.Unmarshal(rawNS, &patch.DeleteNamespace) if err != nil { writeError(w, http.StatusBadRequest, "decode delete_namespaces: "+err.Error()) diff --git a/pkg/rest/operator_messages_test.go b/pkg/rest/operator_messages_test.go new file mode 100644 index 00000000..1500a640 --- /dev/null +++ b/pkg/rest/operator_messages_test.go @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: Apache-2.0 + +/* +Copyright 2026 Cozystack contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rest + +import ( + "fmt" + "go/ast" + "go/parser" + "go/token" + "io/fs" + "path/filepath" + "regexp" + "strconv" + "strings" + "testing" +) + +// internalTrackerRef matches the project's internal tracker spellings in +// any case, "Bug 174", "Bug-020", "BUG-048" and "Phase 12". They mean nothing to an operator +// reading an error, an event or a log line, so no string literal in the +// shipped code carries one; comments may. +var internalTrackerRef = regexp.MustCompile(`(?i)\bBug[- ][0-9]|\bPhase [0-9]`) + +func TestShippedStringsCarryNoInternalTrackerNumbers(t *testing.T) { + t.Parallel() + + var hits []string + + for _, root := range []string{"../../pkg", "../../internal", "../../cmd"} { + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + + if d.IsDir() || !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + + fset := token.NewFileSet() + + file, parseErr := parser.ParseFile(fset, path, nil, parser.SkipObjectResolution) + if parseErr != nil { + return fmt.Errorf("parse %s: %w", path, parseErr) + } + + ast.Inspect(file, func(n ast.Node) bool { + lit, ok := n.(*ast.BasicLit) + if !ok || lit.Kind != token.STRING { + return true + } + + value, unquoteErr := strconv.Unquote(lit.Value) + if unquoteErr != nil { + value = lit.Value + } + + if internalTrackerRef.MatchString(value) { + hits = append(hits, fset.Position(lit.Pos()).String()+": "+value) + } + + return true + }) + + return nil + }) + if err != nil { + t.Fatalf("walk %s: %v", root, err) + } + } + + for _, hit := range hits { + t.Errorf("string literal carries an internal tracker number: %s", hit) + } +} diff --git a/pkg/rest/props_modify.go b/pkg/rest/props_modify.go index 6697f4c4..f0b8c7ae 100644 --- a/pkg/rest/props_modify.go +++ b/pkg/rest/props_modify.go @@ -18,7 +18,11 @@ limitations under the License. package rest -import "github.com/cozystack/blockstor/pkg/store" +import ( + "net/http" + + "github.com/cozystack/blockstor/pkg/store" +) // applyPropsModify is the shared property-bag merge core. Every // LINSTOR object that accepts a `GenericPropsModify` envelope @@ -84,3 +88,37 @@ func deletePropNamespaces(props map[string]string, namespaces []string) { } } } + +// The wire names of the props-modify envelope's three fields. +const ( + fieldOverrideProps = "override_props" + fieldDeleteProps = "delete_props" + fieldDeleteNamespaces = "delete_namespaces" +) + +// refusePropsModifyOnCreate answers a create door that received the +// props-modify fields. Those fields edit props that already exist, and a +// create has none yet; neither golinstor nor python-linstor sends them on +// rd create or rg spawn. Honouring them would invent semantics, and dropping +// them answers success for an edit that never happened, so they are refused +// with the field named. Returns true when the request was answered. +func refusePropsModifyOnCreate(w http.ResponseWriter, door string, override map[string]string, del, namespaces []string) bool { + var field string + + switch { + case len(override) > 0: + field = fieldOverrideProps + case len(del) > 0: + field = fieldDeleteProps + case len(namespaces) > 0: + field = fieldDeleteNamespaces + default: + return false + } + + writeError(w, http.StatusBadRequest, + door+" does not take "+field+": set the properties on the definition once it exists, "+ + "with `linstor rd set-property` or a resource-definition modify") + + return true +} diff --git a/pkg/rest/rd_clone.go b/pkg/rest/rd_clone.go index c66e9214..5db7339d 100644 --- a/pkg/rest/rd_clone.go +++ b/pkg/rest/rd_clone.go @@ -1974,7 +1974,7 @@ func (s *Server) cloneSnapshotPreconditionsHold(ctx context.Context, w http.Resp "' refused: the source has no active diskful replicas to clone from", Cause: "the clone data plane snapshots the source and restores the target " + "from it; with no deployed diskful replica there is nothing to snapshot " + - "and the result would be an empty shell (Bug 114)", + "and the result would be an empty shell", Correc: "deploy the source first (`linstor rd ap " + src.Name + "`), " + "or clone before removing its replicas", }) @@ -2164,9 +2164,8 @@ func writeCloneStoreError(w http.ResponseWriter, srcName, cloneName string, err // are dropped before the Create lands. python-linstor 1.27.0 sends // these via `linstor resource-definition clone --override-prop K=V` // / `--delete-prop K`; wiring them through here keeps the operator's -// intent honoured for the empty-VD path. `req.SrcSnapName` is -// accepted-and-no-op (see rdCloneRequest docstring) — the snapshot- -// based clone data plane lands separately. +// intent honoured for the empty-VD path. A non-empty `req.SrcSnapName` +// never gets here: it is refused with 501 before either clone path runs. func (s *Server) cloneEmptyRDShell(w http.ResponseWriter, r *http.Request, src *apiv1.ResourceDefinition, req *rdCloneRequest, ) { @@ -2396,11 +2395,9 @@ func computeCloneStatus(ctx context.Context, st store.Store, srcName, targetName func writeSnapshotCloneNotImplemented(w http.ResponseWriter, srcName, cloneName, srcSnapName string) { writeCloneRefused(w, http.StatusNotImplemented, srcName, cloneName, &apiv1.APICallRc{ RetCode: apiCallRcError, - Message: "snapshot-based clone not implemented in this release (pending Phase 12)", - Cause: "the apiserver accepts `src_snap_name` on the wire for python-linstor 1.27.0 " + - "compatibility (Bug 232 + 237) but the satellite-side snapshot-clone data plane is " + - "not yet wired; silently falling back to a live-RD shell copy would discard the " + - "snapshot intent and produce a clone with the wrong contents", + Message: "cloning from a named snapshot (src_snap_name) is not supported", + Cause: "blockstor clones from the live source only; cloning from the live source " + + "instead would give a clone with different contents than the snapshot holds", Correc: "use the snapshot-then-restore workflow which IS wired today: " + "`linstor s create " + srcName + " " + srcSnapName + "` (if the snapshot " + "doesn't already exist) then " + diff --git a/pkg/rest/resource_definitions.go b/pkg/rest/resource_definitions.go index 1808644b..9b08a7b2 100644 --- a/pkg/rest/resource_definitions.go +++ b/pkg/rest/resource_definitions.go @@ -404,8 +404,8 @@ func (s *Server) refuseRDCreateOnRGDeletedRace(w http.ResponseWriter, r *http.Re writeError(w, http.StatusNotFound, "resource group '"+rd.ResourceGroupName+"' was deleted "+ - "concurrently with the resource-definition create "+ - "(Bug 174): retry after re-creating the resource group, "+ + "concurrently with the resource-definition create: "+ + "retry after re-creating the resource group, "+ "or pick a different resource group") return false @@ -428,6 +428,10 @@ func (s *Server) refuseRDCreateOnRGDeletedRace(w http.ResponseWriter, r *http.Re // 60-line cap and so each new wire-boundary refusal lives in one // canonical spot rather than as another diff inside the parent. func (s *Server) validateRDCreateBody(w http.ResponseWriter, r *http.Request, body *apiv1.ResourceDefinitionCreate, rd *apiv1.ResourceDefinition) bool { + if refusePropsModifyOnCreate(w, "resource definition create", body.OverrideProps, body.DeleteProps, body.DeleteNamespace) { + return false + } + // Bug 97: refuse whitespace-only / RFC-1123-illegal names at the // REST wire boundary, BEFORE pkg/store/k8s.Name() slugifies + // hash-prefixes the input. Without this gate `linstor rd c " "` diff --git a/pkg/rest/snapshots.go b/pkg/rest/snapshots.go index f6f48715..4aa4186e 100644 --- a/pkg/rest/snapshots.go +++ b/pkg/rest/snapshots.go @@ -571,7 +571,7 @@ func (s *Server) handleSnapshotCreate(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusConflict, "resource definition '"+rd+"' is being deleted "+ "(DELETE flag set): retry against a different RD or "+ - "wait for the tear-down to complete (Bug 180)") + "wait for the tear-down to complete") return } @@ -790,7 +790,7 @@ func (s *Server) refuseSnapshotCreateOnRDDeletedRace(w http.ResponseWriter, r *h writeError(w, http.StatusConflict, "resource definition '"+snap.ResourceName+"' "+reason+ - " concurrently with the snapshot create (Bug 180): "+ + " concurrently with the snapshot create: "+ "retry after re-creating the resource definition, or "+ "pick a different RD") @@ -1041,7 +1041,7 @@ func (s *Server) refuseSnapshotOnNonSnapshotPool(w http.ResponseWriter, r *http. "can take copy-on-write snapshots", Correc: "place the resource on a thin-provisioned snapshot-capable " + "pool (LVM_THIN / ZFS_THIN / FILE_THIN); `linstor rd clone` also " + - "relies on snapshots (Bug-020) so it is no workaround on a thick pool", + "relies on snapshots, so it is no workaround on a thick pool", }}) return false diff --git a/pkg/rest/spawn.go b/pkg/rest/spawn.go index 51c8cd8f..d80a2f1a 100644 --- a/pkg/rest/spawn.go +++ b/pkg/rest/spawn.go @@ -39,11 +39,9 @@ func (s *Server) registerSpawn(mux *http.ServeMux) { } // handleSpawn creates a ResourceDefinition + VolumeDefinitions from a -// ResourceGroup template. Replica placement (creating Resource objects on -// satellites) is a separate Phase 3 concern owned by the autoplacer; this -// handler only materialises the definition side, which is what linstor-csi -// expects from the Spawn call before it issues separate placement -// instructions. +// ResourceGroup template and places replicas by the group's select filter, +// as upstream `linstor rg spawn` does, unless the caller asks for the +// definitions only. func (s *Server) handleSpawn(w http.ResponseWriter, r *http.Request) { rgName := r.PathValue("rg") @@ -53,6 +51,10 @@ func (s *Server) handleSpawn(w http.ResponseWriter, r *http.Request) { return } + if refusePropsModifyOnCreate(w, "resource group spawn", req.OverrideProps, req.DeleteProps, req.DeleteNamespace) { + return + } + // CreateVolume hot path — the just-created RG may not yet be visible // on this apiserver replica's informer cache; retry on NotFound. // See pkg/rest/cache_retry.go. @@ -64,7 +66,9 @@ func (s *Server) handleSpawn(w http.ResponseWriter, r *http.Request) { } if req.ResourceDefinitionName == "" { - writeError(w, http.StatusBadRequest, "resource_definition_name is required (auto-naming Phase 6)") + writeError(w, http.StatusBadRequest, + "resource_definition_name is required: blockstor does not generate names, "+ + "so a spawn by external name alone is refused; pass a resource definition name") return } @@ -163,13 +167,22 @@ func (s *Server) spawnCreate(w http.ResponseWriter, r *http.Request, rgName stri return } + // definitions_only asks for the definition and its volumes without a + // placement (`linstor rg spawn --definition-only`). + if req.DefinitionsOnly { + writeJSON(w, http.StatusCreated, []apiv1.APICallRc{{ + RetCode: maskInfo, + Message: "resource definition spawned without placement (definitions_only): " + rd.Name, + }}) + + return + } + // Upstream LINSTOR's `linstor rg spawn` autoplaces according to // the RG's SelectFilter — that's the operator-visible contract: // spawn = create RD + VDs + place replicas. Without this step, // `r l` would be empty after spawn until the operator runs a - // separate `r c --auto-place=N`. The CSI flow follows the same - // contract (linstor-csi calls /spawn, then expects to find - // resources in /v1/view/resources on the next reconcile). + // separate `r c --auto-place=N`. placeErr := spawnAutoplace(r.Context(), s, rd.Name, rg, req) if placeErr != nil { log.FromContext(r.Context()).WithName("rest"). diff --git a/pkg/rest/vd_passphrase_bug_233.go b/pkg/rest/vd_passphrase_bug_233.go index 5eacd8b5..21e2a089 100644 --- a/pkg/rest/vd_passphrase_bug_233.go +++ b/pkg/rest/vd_passphrase_bug_233.go @@ -161,7 +161,7 @@ func (s *Server) handleVDPassphraseRotate(w http.ResponseWriter, r *http.Request writeJSON(w, http.StatusOK, []apiv1.APICallRc{{ RetCode: maskInfo, - Message: "VD passphrase stored; cluster-side LUKS rotation pending Phase 12", + Message: "VD passphrase stored; it is not applied to existing LUKS devices", }}) } diff --git a/pkg/satellite/agent.go b/pkg/satellite/agent.go index 7d43c55e..1f4130c1 100644 --- a/pkg/satellite/agent.go +++ b/pkg/satellite/agent.go @@ -142,7 +142,7 @@ func (a *Agent) Run(ctx context.Context) error { } if a.cfg.RESTConfig == nil || a.cfg.ManagerFactory == nil { - return errors.New("RESTConfig + ManagerFactory are required (Phase 10.6 retired the gRPC path)") + return errors.New("RESTConfig and ManagerFactory are required") } a.logger.Info("agent starting", diff --git a/pkg/satellite/controllers/physicaldevice.go b/pkg/satellite/controllers/physicaldevice.go index 06e53828..f62d7e48 100644 --- a/pkg/satellite/controllers/physicaldevice.go +++ b/pkg/satellite/controllers/physicaldevice.go @@ -528,7 +528,7 @@ func (r *PhysicalDeviceReconciler) clearStaleAttachTo(ctx context.Context, dev * return ctrl.Result{}, errors.Wrap(err, "reset Phase to Available after stale AttachTo clear") } - logger.Info("cleared stale AttachTo; target StoragePool gone (Bug 340 self-heal)", + logger.Info("cleared stale AttachTo: the target StoragePool is gone", "missing-pool", missingPool) return ctrl.Result{}, nil diff --git a/pkg/satellite/reconciler.go b/pkg/satellite/reconciler.go index 8a005f29..cd4e69fd 100644 --- a/pkg/satellite/reconciler.go +++ b/pkg/satellite/reconciler.go @@ -1266,12 +1266,12 @@ func (r *Reconciler) applyInactive(ctx context.Context, dr *intent.DesiredResour switch r.cfg.Adm.EvaluateDownVeto(ctx, dr.GetName()) { case drbd.DownVetoResync: res.Ok = false - res.Message = "deferring drbdadm down: resync in flight (Bug 350 veto)" + res.Message = "deferring drbdadm down: resync in flight" return case drbd.DownVetoInconclusive: res.Ok = false - res.Message = "deferring drbdadm down: kernel probe inconclusive, cannot confirm idle (Bug 350 fail-closed veto)" + res.Message = "deferring drbdadm down: kernel probe inconclusive, cannot confirm the resource is idle" return case drbd.DownAllowed: @@ -2427,14 +2427,14 @@ func (r *Reconciler) reconcileKernelMyNodeID(ctx context.Context, dr *intent.Des return nil } - log.FromContext(ctx).Info("Bug 360 self-heal: kernel my-node-id mismatch, recreating slot", + log.FromContext(ctx).Info("kernel node-id differs from the allocated one, recreating the DRBD slot", "resource", dr.GetName(), "kernelMyNodeID", kernelID, "desiredNodeID", desiredID) err = r.cfg.Adm.Down(ctx, dr.GetName()) if err != nil { - return errors.Wrapf(err, "Bug 360 self-heal: drbdadm down %s (kernel my-id %d != desired %d)", + return errors.Wrapf(err, "recreate DRBD slot: drbdadm down %s (kernel node-id %d, allocated %d)", dr.GetName(), kernelID, desiredID) } @@ -2533,7 +2533,7 @@ func (r *Reconciler) finishDRBDApply(ctx context.Context, dr *intent.DesiredReso return nil } - log.FromContext(ctx).Info("BUG-028: force-promote veto bypassed — every connected peer is a day0-empty sibling, proceeding to first-activation mkfs", + log.FromContext(ctx).Info("force-promote veto bypassed — every connected peer is a day0-empty sibling, proceeding to first-activation mkfs", "resource", dr.GetName()) } @@ -2834,7 +2834,7 @@ func (r *Reconciler) maybeRecoveryPromote(ctx context.Context, dr *intent.Desire return nil } - log.FromContext(ctx).Info("Bug 366 recovery-promote: re-arming auto-primary to unstick wedged initial sync", + log.FromContext(ctx).Info("re-arming auto-primary to unstick a stalled initial sync", "resource", dr.GetName()) return r.runAutoPromote(ctx, dr) @@ -2888,7 +2888,7 @@ func (r *Reconciler) maybeLateAddPromote(ctx context.Context, dr *intent.Desired return nil } - log.FromContext(ctx).Info("BUG-048 late-add-promote: mint UpToDate source (disconnect + new-current-uuid --clear-bitmap + reconnect) for a late-added volume wedged Inconsistent on every replica", + log.FromContext(ctx).Info("late-add-promote: mint UpToDate source (disconnect + new-current-uuid --clear-bitmap + reconnect) for a late-added volume wedged Inconsistent on every replica", "resource", dr.GetName()) minors, err := r.cfg.Adm.MintLateAddSource(ctx, dr.GetName()) @@ -2896,7 +2896,7 @@ func (r *Reconciler) maybeLateAddPromote(ctx context.Context, dr *intent.Desired return errors.Wrapf(err, "late-add-promote mint source %s", dr.GetName()) } - log.FromContext(ctx).Info("BUG-048 late-add-promote: minted UpToDate source", + log.FromContext(ctx).Info("late-add-promote: minted UpToDate source", "resource", dr.GetName(), "minors", minors) return nil @@ -2981,7 +2981,7 @@ func (r *Reconciler) maybeKickLateAddResync(ctx context.Context, dr *intent.Desi // the kernel device scan. vol := volumes[0] - log.FromContext(ctx).Info("BUG-048 late-add resync-kick: invalidate locally to re-pull a stalled late-added volume from its UpToDate peer", + log.FromContext(ctx).Info("late-add resync-kick: invalidate locally to re-pull a stalled late-added volume from its UpToDate peer", "resource", dr.GetName(), "volume", vol) return errors.Wrapf(r.cfg.Adm.InvalidateVolume(ctx, dr.GetName(), vol), @@ -3331,7 +3331,7 @@ func (r *Reconciler) latchFreeMkfsRetryAllowed(ctx context.Context, dr *intent.D return false } - log.FromContext(ctx).Info("BUG-028: latch-free mkfs retry — no auto-primary election but a volume provably lacks its filesystem, re-entering promote+mkfs+demote", + log.FromContext(ctx).Info("latch-free mkfs retry — no auto-primary election but a volume provably lacks its filesystem, re-entering promote+mkfs+demote", "resource", dr.GetName()) return true @@ -5239,7 +5239,7 @@ func refuseUnresolvedSkipInitialSync(dr *intent.DesiredResource, freshActivation func (r *Reconciler) gateBringUpReadiness(ctx context.Context, dr *intent.DesiredResource, diskless bool, mdMarkerPath string) error { err := refuseUnresolvedLocalNodeID(dr) if err != nil { - log.FromContext(ctx).Info("Bug 360: deferring DRBD bring-up until controller allocates local node-id", + log.FromContext(ctx).Info("deferring DRBD bring-up until the controller allocates this node's node-id", "resource", dr.GetName()) return err diff --git a/tests/e2e/cli-matrix/rg-spawn-definitions-only.sh b/tests/e2e/cli-matrix/rg-spawn-definitions-only.sh new file mode 100755 index 00000000..353f9bba --- /dev/null +++ b/tests/e2e/cli-matrix/rg-spawn-definitions-only.sh @@ -0,0 +1,64 @@ +#!/usr/bin/env bash +# +# usage: rg-spawn-definitions-only.sh WORK_DIR +# +# L6 cli-matrix cell: `linstor rg spawn-resources --definition-only` +# creates the definition and its volumes and places nothing. The spawn +# handler ignored `definitions_only` and placed replicas from the group's +# place-count anyway (cozystack/blockstor#216, issue #210). +# +# $ linstor resource-group create rg --place-count 1 -s stand +# $ linstor resource-group spawn-resources --definition-only rg rd 32M +# $ linstor resource list -r rd # MUST be empty +# +# Unit pin: pkg/rest/create_doors_props_modify_test.go. +# L7 replay: tests/operator-harness/replay/rg-spawn-definitions-only.yaml. + +set -euo pipefail + +WORK_DIR=${1:?work_dir required} +export KUBECONFIG="$WORK_DIR/kubeconfig" + +SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=lib.sh +source "$SCRIPT_DIR/lib.sh" + +require_workers 1 + +linstor_cli_setup + +RG=cli-matrix-defs-only-rg +RD=cli-matrix-defs-only-rd +POOL=${POOL:-stand} + +cleanup() { + "${LCTL[@]}" resource-definition delete "$RD" >/dev/null 2>&1 || true + "${LCTL[@]}" resource-group delete "$RG" >/dev/null 2>&1 || true + assert_no_orphans "$RD" + linstor_cli_teardown +} +trap cleanup EXIT + +echo ">> rg create --place-count 1 + spawn-resources --definition-only $RD 32M" +"${LCTL[@]}" resource-group create "$RG" --place-count 1 --storage-pool="$POOL" >/dev/null +"${LCTL[@]}" resource-group spawn-resources --definition-only "$RG" "$RD" 32M >/dev/null + +if ! wait_vd_size "$RD" 0 32768 60; then + echo "FAIL: the spawned definition has no 32 MiB volume" >&2 + "${LCTL[@]}" volume-definition list --resource-definitions "$RD" 2>&1 | tail -20 >&2 + exit 1 +fi + +echo ">> no replica may appear over 15s" +deadline=$(( $(date +%s) + 15 )) +while (( $(date +%s) < deadline )); do + count=$(linstor_replica_count "$RD") + if (( count > 0 )); then + echo "FAIL: --definition-only spawn placed ${count} replica(s)" >&2 + "${LCTL[@]}" resource list --resources "$RD" 2>&1 | tail -20 >&2 + exit 1 + fi + sleep 3 +done + +echo ">> rg-spawn-definitions-only OK" diff --git a/tests/operator-harness/replay/rg-spawn-definitions-only.yaml b/tests/operator-harness/replay/rg-spawn-definitions-only.yaml new file mode 100644 index 00000000..576b822e --- /dev/null +++ b/tests/operator-harness/replay/rg-spawn-definitions-only.yaml @@ -0,0 +1,61 @@ +name: rg-spawn-definitions-only +description: | + `linstor rg spawn-resources --definition-only ` asks for + the definition and its volumes and no placement. The spawn handler + ignored `definitions_only` and placed replicas from the group's + place-count anyway. Fixed in cozystack/blockstor#216 (#210). + + Operator sequence: + rg create --place-count 1 -> spawn-resources --definition-only 32M + + CLI-side contract: + - the definition and its 32 MiB volume exist (vd_size_kib) + - no replica is placed, and none appears over a hold window + (replica_count_max with max 0) + + Unit pin: pkg/rest/create_doors_props_modify_test.go + (TestSpawnDefinitionsOnlyPlacesNothing). + L6 cell: tests/e2e/cli-matrix/rg-spawn-definitions-only.sh. + +prerequisites: + min_nodes: 1 + storage_pool: stand + +vars: + rg: rg-spawn-defs-only + sp: stand + +steps: + - name: create-rg + cmd: ["resource-group", "create", "{{rg}}", "--place-count", "1", "--storage-pool", "{{sp}}"] + expect_exit: 0 + - name: spawn-definitions-only + cmd: ["resource-group", "spawn-resources", "--definition-only", "{{rg}}", "{{rd}}", "32M"] + expect_exit: 0 + await: + kind: vd_size_kib + rd: "{{rd}}" + vol: 0 + expected_kib: 32768 + timeout_s: 60 + - name: no-replica-placed + cmd: ["resource", "list", "--resources", "{{rd}}"] + expect_exit: 0 + await: + kind: replica_count_max + rd: "{{rd}}" + max: 0 + hold_s: 15 + +teardown: + - cmd: ["resource-definition", "delete", "{{rd}}"] + expect_exit: 0 + await: + kind: rd_absent + rd: "{{rd}}" + timeout_s: 60 + - cmd: ["resource-group", "delete", "{{rg}}"] + expect_exit: 0 + +invariants: + - no_orphans