Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/satellite/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func loadRESTConfig() (*rest.Config, error) {
// satellite.ManagerFactory signature.
func mgrFactory(ready *readyState, logger *slog.Logger, ueventListener controllers.UeventNotifier) satellite.ManagerFactory {
return func(restCfg *rest.Config, nodeName, probeAddr string, rec *satellite.Reconciler) (manager.Manager, error) {
mgr, err := controllers.NewManager(restCfg, controllers.Config{
mgr, err := controllers.NewManager(restCfg, &controllers.Config{
NodeName: nodeName,
Apply: rec,
Exec: storage.RealExec{},
Expand Down
9 changes: 6 additions & 3 deletions pkg/rest/autoplace.go
Original file line number Diff line number Diff line change
Expand Up @@ -1138,23 +1138,26 @@ func (s *Server) applyBug156AutoTiebreakerOptOut(
// decision happens out-of-band in the controller — the REST handler
// can't suppress it inline.
func (s *Server) stampAutoTiebreakerOptOut(ctx context.Context, rdName string) error {
const propKey = "DrbdOptions/AutoAddQuorumTiebreaker"
const (
propKey = "DrbdOptions/AutoAddQuorumTiebreaker"
propValueOff = "false"
)

// Bug 205: typed-Patch via PatchResourceDefinitionSpec — the
// closure re-runs on every conflict against the live RD, so
// disjoint concurrent edits (RG-supplied props, racing r-conn
// add) converge with the tiebreaker opt-out instead of being
// lost by a stale-wire-snapshot replay.
err := s.Store.ResourceDefinitions().PatchResourceDefinitionSpec(ctx, rdName, func(live *apiv1.ResourceDefinition) error {
if live.Props != nil && live.Props[propKey] == "false" {
if live.Props != nil && live.Props[propKey] == propValueOff {
return nil
}

if live.Props == nil {
live.Props = map[string]string{}
}

live.Props[propKey] = "false"
live.Props[propKey] = propValueOff

return nil
})
Expand Down
6 changes: 5 additions & 1 deletion pkg/rest/cache_invalidation_bug_124_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,14 @@ func (l *laggingRDs) Delete(ctx context.Context, name string) error {
lag := l.lag
inner := l.inner

// Bug 124 test helper: detached goroutine survives the request
// context cancel, so propagate a fresh non-cancellable context
// derived from the caller's value chain (contextcheck-friendly).
bgCtx := context.WithoutCancel(ctx)
go func() {
time.Sleep(lag)

_ = inner.Delete(context.Background(), name)
_ = inner.Delete(bgCtx, name)
}()

return nil
Expand Down
12 changes: 11 additions & 1 deletion pkg/rest/node_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,17 @@ func (s *Server) handleNodeEvacuateMulti(w http.ResponseWriter, r *http.Request)
// stdlib, not in every call site (goconst flags package-wide
// `"true"` repetition above its threshold).
func isForce(r *http.Request) bool {
v, _ := strconv.ParseBool(r.URL.Query().Get("force"))
return queryFlag(r, "force")
}

// queryFlag centralises every `?<name>=true` boolean-toggle check
// in the REST surface. Routed through strconv.ParseBool so the
// literal `"true"` lives in stdlib (goconst flags package-wide
// `"true"` repetition above its threshold). Callers: isForce,
// handleResourceActivate (?reallocate-port), handleRDsList
// (?with_volume_definitions), handleResourceToggleDisk (?cancel).
func queryFlag(r *http.Request, name string) bool {
v, _ := strconv.ParseBool(r.URL.Query().Get(name))

return v
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/rest/nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -867,14 +867,15 @@ func (s *Server) handleNodePropDelete(w http.ResponseWriter, r *http.Request) {
func (s *Server) handleNodeDelete(w http.ResponseWriter, r *http.Request) {
name := r.PathValue("node")
force := isForce(r)
ctx := r.Context()

// Bug 179: `?force=true` cascade-deletes every referencing
// Resource + StoragePool CRD before dropping the Node — same
// shape `n lost` already enforces. Without the cascade,
// force-delete would leave orphan SP CRDs pointing at a deleted
// Node, which is precisely the symptom Bug 179 closed.
if force {
err := s.cascadeOrphansForLostNode(r.Context(), name)
err := s.cascadeOrphansForLostNode(ctx, name)
if err != nil {
writeStoreError(w, err)

Expand All @@ -891,10 +892,10 @@ func (s *Server) handleNodeDelete(w http.ResponseWriter, r *http.Request) {
return s.refuseNodeDeleteIfReferenced(w, r, name)
},
capture: func() (apiv1.Node, bool) {
return s.captureNode(r.Context(), name)
return s.captureNode(ctx, name)
},
remove: func() error {
return s.Store.Nodes().Delete(r.Context(), name)
return s.Store.Nodes().Delete(ctx, name)
},
rolledBackIfRaced: func(captured apiv1.Node, capturedOK bool) bool {
if force || !capturedOK {
Expand Down
2 changes: 1 addition & 1 deletion pkg/rest/resource_adjust.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (s *Server) registerResourceLifecycle(mux *http.ServeMux) {
// against a collision. Operators opt in only when they actively want
// the port to be reshuffled.
func (s *Server) handleResourceActivate(w http.ResponseWriter, r *http.Request) {
if r.URL.Query().Get("reallocate-port") == "true" {
if queryFlag(r, "reallocate-port") {
rdName := r.PathValue("rd")
node := r.PathValue("node")

Expand Down
2 changes: 1 addition & 1 deletion pkg/rest/resource_definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (s *Server) handleRDList(w http.ResponseWriter, r *http.Request) {
// their VDs inlined under `volume_definitions`. Without this
// handling, `vd l` renders an empty table even when VDs exist
// (the Python CLI never falls back to per-RD GETs).
if r.URL.Query().Get("with_volume_definitions") == "true" {
if queryFlag(r, "with_volume_definitions") {
for i := range rds {
vds, vdErr := s.Store.VolumeDefinitions().List(r.Context(), rds[i].Name)
if vdErr != nil {
Expand Down
5 changes: 3 additions & 2 deletions pkg/rest/resource_groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -599,16 +599,17 @@ func mergeRGProps(existing, patch *apiv1.ResourceGroup) {
// but RDs still pointing at it) can never arise.
func (s *Server) handleRGDelete(w http.ResponseWriter, r *http.Request) {
name := r.PathValue("rg")
ctx := r.Context()

(&deleteWithRollback[apiv1.ResourceGroup]{
refuseIfReferenced: func() bool {
return s.refuseRGDeleteIfReferenced(w, r, name)
},
capture: func() (apiv1.ResourceGroup, bool) {
return s.captureResourceGroup(r.Context(), name)
return s.captureResourceGroup(ctx, name)
},
remove: func() error {
return s.Store.ResourceGroups().Delete(r.Context(), name)
return s.Store.ResourceGroups().Delete(ctx, name)
},
rolledBackIfRaced: func(captured apiv1.ResourceGroup, capturedOK bool) bool {
if !capturedOK {
Expand Down
2 changes: 1 addition & 1 deletion pkg/rest/resource_toggle_disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (s *Server) handleResourceToggleDisk(w http.ResponseWriter, r *http.Request
// the reconciler does that itself as the last step of the
// rollback so an external observer sees DISKLESS reappear only
// AFTER drbdadm down + storage Delete succeed.
if r.URL.Query().Get("cancel") == "true" {
if queryFlag(r, "cancel") {
err := s.Store.Resources().PatchResourceSpec(r.Context(), rdName, node,
func(res *apiv1.Resource) error {
res.ToggleDiskCancel = true
Expand Down
22 changes: 11 additions & 11 deletions pkg/rest/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,17 +154,6 @@ func (s *Server) SetResolveHost(fn resolveHostFunc) resolveHostFunc {
return prev
}

// lookupHost dispatches to s.resolveHost if set, else the package
// default. Hoisted off-handler so the production handler stays
// readable and the test seam is explicit.
func (s *Server) lookupHost(ctx context.Context, host string) ([]string, error) {
if s.resolveHost != nil {
return s.resolveHost(ctx, host)
}

return defaultResolveHost(ctx, host)
}

// NeedLeaderElection reports whether the server requires leader election.
// REST is read-mostly today and safe to run on every replica; once we
// introduce write-paths that need a single leader we'll change this to true.
Expand Down Expand Up @@ -250,6 +239,17 @@ func (s *Server) Start(ctx context.Context) error {
}
}

// lookupHost dispatches to s.resolveHost if set, else the package
// default. Hoisted off-handler so the production handler stays
// readable and the test seam is explicit.
func (s *Server) lookupHost(ctx context.Context, host string) ([]string, error) {
if s.resolveHost != nil {
return s.resolveHost(ctx, host)
}

return defaultResolveHost(ctx, host)
}

// buildMux registers every endpoint on a fresh ServeMux. Pulled out
// of Start to keep the latter under the funlen budget — Start now
// only handles lifecycle (listener + shutdown), this owns routing.
Expand Down
5 changes: 3 additions & 2 deletions pkg/rest/storage_pools.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ func (s *Server) handleNodeStoragePoolDelete(w http.ResponseWriter, r *http.Requ
node := r.PathValue("node")
pool := r.PathValue("pool")
force := isForce(r)
ctx := r.Context()

(&deleteWithRollback[apiv1.StoragePool]{
refuseIfReferenced: func() bool {
Expand All @@ -180,10 +181,10 @@ func (s *Server) handleNodeStoragePoolDelete(w http.ResponseWriter, r *http.Requ
return s.refuseSPDeleteIfReferenced(w, r, node, pool)
},
capture: func() (apiv1.StoragePool, bool) {
return s.captureStoragePool(r.Context(), node, pool)
return s.captureStoragePool(ctx, node, pool)
},
remove: func() error {
return s.Store.StoragePools().Delete(r.Context(), node, pool)
return s.Store.StoragePools().Delete(ctx, node, pool)
},
rolledBackIfRaced: func(captured apiv1.StoragePool, capturedOK bool) bool {
// `?force=true` callers opt past this check too.
Expand Down
19 changes: 15 additions & 4 deletions pkg/rest/volume_definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,17 @@ const minVolumeDefinitionSizeKib int64 = 4 * 1024
// retry loop.
const maxVolumeDefinitionSizeKib int64 = 16 * 1024 * 1024 * 1024

// ErrVolumeSizeBelowMinimum is the sentinel for Bug 155's lower-bound
// rejection (size_kib < minVolumeDefinitionSizeKib). Wrapped with
// %w + a sizeKib / bound detail by validateVDSize; static-error
// requirement is err113.
var ErrVolumeSizeBelowMinimum = errors.New("size_kib below minimum")

// ErrVolumeSizeAboveMaximum is the sentinel for Bug 155's upper-bound
// rejection (size_kib > maxVolumeDefinitionSizeKib). See
// ErrVolumeSizeBelowMinimum for the rationale.
var ErrVolumeSizeAboveMaximum = errors.New("size_kib above maximum")

// validateVDSize returns nil when the requested size_kib is within
// the accepted bounds [minVolumeDefinitionSizeKib,
// maxVolumeDefinitionSizeKib] (Bug 155). Otherwise it returns a
Expand All @@ -453,16 +464,16 @@ const maxVolumeDefinitionSizeKib int64 = 16 * 1024 * 1024 * 1024
func validateVDSize(sizeKib int64) error {
if sizeKib < minVolumeDefinitionSizeKib {
return fmt.Errorf(
"size_kib=%d below minimum %d KiB (DRBD reserves ~32 KiB of "+
"%w: size_kib=%d below minimum %d KiB (DRBD reserves ~32 KiB of "+
"metadata per peer; backing layers add alignment on top)",
sizeKib, minVolumeDefinitionSizeKib,
ErrVolumeSizeBelowMinimum, sizeKib, minVolumeDefinitionSizeKib,
)
}

if sizeKib > maxVolumeDefinitionSizeKib {
return fmt.Errorf(
"size_kib=%d above maximum %d KiB (DRBD's per-device hard ceiling)",
sizeKib, maxVolumeDefinitionSizeKib,
"%w: size_kib=%d above maximum %d KiB (DRBD's per-device hard ceiling)",
ErrVolumeSizeAboveMaximum, sizeKib, maxVolumeDefinitionSizeKib,
)
}

Expand Down
21 changes: 2 additions & 19 deletions pkg/satellite/controllers/discovered_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,25 +142,8 @@ func (d *DiscoveredStorageRunnable) Start(ctx context.Context) error {

logger := log.FromContext(ctx).WithName("discovered-storage").WithValues("node", d.NodeName)

err := d.tick(ctx, logger)
if err != nil {
logger.Error(err, "initial discovered-storage tick")
}

ticker := time.NewTicker(period)
defer ticker.Stop()

for {
select {
case <-ctx.Done():
return nil
case <-ticker.C:
err = d.tick(ctx, logger)
if err != nil {
logger.Error(err, "discovered-storage tick")
}
}
}
return runPeriodicTick(ctx, period, logger, d.tick,
"initial discovered-storage tick", "discovered-storage tick")
}

// RegisterWithManager adds the runnable to mgr. Symmetrical with
Expand Down
Loading
Loading