Skip to content

Commit dc47ecb

Browse files
committed
feat(activate): gate cluster activation on failure-domain readiness
reconcileActivate fired POST /activate as soon as Spec.Action==Activate and the cluster UUID resolved, with no check on node count, domain count, or balance. If the backend correctly refused an under-provisioned FD cluster (fewer than npcs+2 distinct domains, or unequal per-domain host counts -- see simplyblock_core's fd_activation_domain_count_violation), the CR got permanently stuck: failActivate sets ActionStatus.State=Failed, and nothing in Reconcile/reconcileActivate ever resets ActionStatus away from Failed back to Running, so the next reconcile falls straight through to the GET-polling tail and loops forever without ever retrying the POST. Added a readiness check at the very top of reconcileActivate, before any ActionStatus mutation: for FD-enabled clusters, aggregate each host's failure domain across every StorageNodeSet belonging to the cluster (clusterFailureDomainHosts) and validate domain count/balance (fdActivationDomainCountViolation, mirroring the Python-side check exactly). On failure, just requeue -- ActionStatus is left completely untouched, so there's nothing to get stuck once enough domains show up. No-op for FD-disabled clusters, confirmed via the existing activate tests passing unchanged.
1 parent bb77241 commit dc47ecb

2 files changed

Lines changed: 269 additions & 0 deletions

File tree

operator/internal/controller/simplyblockstoragecluster_controller.go

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,13 +521,103 @@ func (r *StorageClusterReconciler) ensureFinalizer(
521521
return true, r.Update(ctx, clusterCR)
522522
}
523523

524+
// clusterFailureDomainHosts aggregates each host's (by management IP)
525+
// failure domain across every StorageNodeSet belonging to clusterName in
526+
// namespace. Hosts with no failure-domain assignment reported to status
527+
// yet are skipped, not treated as domain 0.
528+
func clusterFailureDomainHosts(
529+
ctx context.Context, c client.Client, namespace, clusterName string,
530+
) (map[string]int32, error) {
531+
var snList simplyblockv1alpha1.StorageNodeSetList
532+
if err := c.List(ctx, &snList, client.InNamespace(namespace)); err != nil {
533+
return nil, err
534+
}
535+
hostDomains := map[string]int32{}
536+
for _, sn := range snList.Items {
537+
if sn.Spec.ClusterName != clusterName {
538+
continue
539+
}
540+
for _, ns := range sn.Status.Nodes {
541+
if ns.FailureDomain == nil || ns.MgmtIp == "" {
542+
continue
543+
}
544+
hostDomains[ns.MgmtIp] = *ns.FailureDomain
545+
}
546+
}
547+
return hostDomains, nil
548+
}
549+
550+
// fdActivationDomainCountViolation validates the number of distinct failure
551+
// domains and per-domain host balance for fresh activation, mirroring
552+
// simplyblock_core's fd_activation_domain_count_violation (Python side) so
553+
// the two stay in lockstep.
554+
//
555+
// A 2-FD layout can never absorb a second independent failure once one
556+
// domain is fully down, so fresh activation requires npcs+2 distinct
557+
// domains (3 for npcs=1, 4 for npcs=2) with an EQUAL host count in each --
558+
// below npcs+1 domains even the initial static role-placement rotation is
559+
// structurally wrong; at exactly npcs+1 it is correct but has zero spare
560+
// capacity for a later single add/remove. Returns a human-readable reason
561+
// when the cluster isn't ready yet, "" when it is.
562+
func fdActivationDomainCountViolation(npcs int, hostDomains map[string]int32) string {
563+
if len(hostDomains) == 0 {
564+
return "no storage nodes with a failure-domain assignment reported yet"
565+
}
566+
counts := map[int32]int{}
567+
for _, fd := range hostDomains {
568+
counts[fd]++
569+
}
570+
minDomains := npcs + 2
571+
if len(counts) < minDomains {
572+
return fmt.Sprintf(
573+
"failure domains are enabled with npcs=%d, which requires at least "+
574+
"%d distinct failure domains (2 domains is not supported at any "+
575+
"npcs level); currently have %d",
576+
npcs, minDomains, len(counts))
577+
}
578+
first := -1
579+
for _, c := range counts {
580+
if first == -1 {
581+
first = c
582+
continue
583+
}
584+
if c != first {
585+
return fmt.Sprintf(
586+
"failure domains must hold an equal number of hosts at "+
587+
"activation; current split: %v", counts)
588+
}
589+
}
590+
return ""
591+
}
592+
524593
func (r *StorageClusterReconciler) reconcileActivate(
525594
ctx context.Context,
526595
clusterCR *simplyblockv1alpha1.StorageCluster,
527596
) (ctrl.Result, error) {
528597

529598
log := logf.FromContext(ctx)
530599

600+
// Failure-domain readiness gate: checked BEFORE any ActionStatus
601+
// transition below, and status is left untouched on failure. A failed
602+
// activate call gets permanently stuck (nothing in this reconciler ever
603+
// resets ActionStatus away from Failed back to Running), so a doomed
604+
// activate attempt must never be allowed to fire in the first place.
605+
// No-op for FD-disabled clusters -- falls straight through unchanged.
606+
if ptr.BoolFromOrFalse(clusterCR.Spec.EnableFailureDomains) {
607+
hostDomains, err := clusterFailureDomainHosts(ctx, r.Client, clusterCR.Namespace, clusterCR.Name)
608+
if err != nil {
609+
log.Error(err, "Failed to list StorageNodeSets for failure-domain readiness check",
610+
"cluster", clusterCR.Name)
611+
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
612+
}
613+
npcs := stripeParityChunks(clusterCR.Spec.StripeSpec)
614+
if reason := fdActivationDomainCountViolation(npcs, hostDomains); reason != "" {
615+
log.Info("Cluster not ready for activation yet, waiting on failure-domain readiness",
616+
"cluster", clusterCR.Name, "reason", reason)
617+
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
618+
}
619+
}
620+
531621
if clusterCR.Status.ActionStatus != nil &&
532622
clusterCR.Status.ActionStatus.Action == utils.ClusterActionActivate &&
533623
clusterCR.Status.ActionStatus.State == utils.ActionStateSuccess &&

operator/internal/controller/simplyblockstoragecluster_controller_unit_test.go

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"testing"
1111
"time"
1212

13+
"github.com/simplyblock/atlas/ptr"
1314
simplyblockv1alpha1 "github.com/simplyblock/simplyblock-operator/api/v1alpha1"
1415
"github.com/simplyblock/simplyblock-operator/internal/utils"
1516
"github.com/simplyblock/simplyblock-operator/internal/webapi"
@@ -146,6 +147,184 @@ func TestReconcileActivateInitializesObservedGeneration(t *testing.T) {
146147
}
147148
}
148149

150+
func TestFdActivationDomainCountViolation(t *testing.T) {
151+
hosts := func(domainCounts ...int32) map[string]int32 {
152+
m := map[string]int32{}
153+
for i, fd := range domainCounts {
154+
m[fmt.Sprintf("10.0.0.%d", i)] = fd
155+
}
156+
return m
157+
}
158+
159+
cases := []struct {
160+
name string
161+
npcs int
162+
hosts map[string]int32
163+
wantErr bool
164+
}{
165+
{"empty", 1, map[string]int32{}, true},
166+
{"npcs1 two domains violates", 1, hosts(0, 0, 1, 1), true},
167+
{"npcs1 three domains ok", 1, hosts(0, 1, 2), false},
168+
{"npcs1 three domains unequal violates", 1, hosts(0, 1, 1, 2), true},
169+
{"npcs2 two domains violates", 2, hosts(0, 0, 1, 1), true},
170+
{"npcs2 three domains violates", 2, hosts(0, 1, 2), true},
171+
{"npcs2 four domains ok", 2, hosts(0, 1, 2, 3), false},
172+
{"npcs2 four domains unequal violates", 2, hosts(0, 0, 1, 2, 3), true},
173+
}
174+
for _, tc := range cases {
175+
t.Run(tc.name, func(t *testing.T) {
176+
reason := fdActivationDomainCountViolation(tc.npcs, tc.hosts)
177+
if tc.wantErr && reason == "" {
178+
t.Fatalf("expected a violation reason, got none")
179+
}
180+
if !tc.wantErr && reason != "" {
181+
t.Fatalf("expected no violation, got: %s", reason)
182+
}
183+
})
184+
}
185+
}
186+
187+
func TestClusterFailureDomainHosts(t *testing.T) {
188+
fd := func(v int32) *int32 { return &v }
189+
190+
nsA := &simplyblockv1alpha1.StorageNodeSet{
191+
ObjectMeta: metav1.ObjectMeta{Name: "set-a", Namespace: "default"},
192+
Spec: simplyblockv1alpha1.StorageNodeSetSpec{ClusterName: "cluster-x"},
193+
Status: simplyblockv1alpha1.StorageNodeSetStatus{
194+
Nodes: []simplyblockv1alpha1.NodeStatus{
195+
{Hostname: "w0", MgmtIp: "10.0.0.1", FailureDomain: fd(0)},
196+
{Hostname: "w1", MgmtIp: "10.0.0.2", FailureDomain: fd(1)},
197+
{Hostname: "w2", MgmtIp: "10.0.0.3"}, // no domain yet, must be skipped
198+
},
199+
},
200+
}
201+
nsB := &simplyblockv1alpha1.StorageNodeSet{
202+
ObjectMeta: metav1.ObjectMeta{Name: "set-b", Namespace: "default"},
203+
Spec: simplyblockv1alpha1.StorageNodeSetSpec{ClusterName: "cluster-x"},
204+
Status: simplyblockv1alpha1.StorageNodeSetStatus{
205+
Nodes: []simplyblockv1alpha1.NodeStatus{
206+
{Hostname: "w3", MgmtIp: "10.0.0.4", FailureDomain: fd(2)},
207+
},
208+
},
209+
}
210+
nsOther := &simplyblockv1alpha1.StorageNodeSet{
211+
ObjectMeta: metav1.ObjectMeta{Name: "set-other", Namespace: "default"},
212+
Spec: simplyblockv1alpha1.StorageNodeSetSpec{ClusterName: "cluster-y"},
213+
Status: simplyblockv1alpha1.StorageNodeSetStatus{
214+
Nodes: []simplyblockv1alpha1.NodeStatus{
215+
{Hostname: "y0", MgmtIp: "10.0.0.9", FailureDomain: fd(0)},
216+
},
217+
},
218+
}
219+
220+
r := newClusterStateTestReconciler(t, nsA, nsB, nsOther)
221+
222+
got, err := clusterFailureDomainHosts(context.Background(), r.Client, "default", "cluster-x")
223+
if err != nil {
224+
t.Fatalf("clusterFailureDomainHosts returned error: %v", err)
225+
}
226+
want := map[string]int32{"10.0.0.1": 0, "10.0.0.2": 1, "10.0.0.4": 2}
227+
if len(got) != len(want) {
228+
t.Fatalf("expected %v, got %v", want, got)
229+
}
230+
for ip, fdVal := range want {
231+
if got[ip] != fdVal {
232+
t.Fatalf("expected %s -> domain %d, got %v", ip, fdVal, got)
233+
}
234+
}
235+
}
236+
237+
func TestReconcileActivateWaitsForFailureDomainReadiness(t *testing.T) {
238+
fd := func(v int32) *int32 { return &v }
239+
240+
cluster := &simplyblockv1alpha1.StorageCluster{
241+
ObjectMeta: metav1.ObjectMeta{
242+
Name: "cluster-fd-wait",
243+
Namespace: "default",
244+
},
245+
Spec: simplyblockv1alpha1.StorageClusterSpec{
246+
Action: utils.ClusterActionActivate,
247+
EnableFailureDomains: ptr.To(true),
248+
StripeSpec: &simplyblockv1alpha1.StripeSpec{
249+
ParityChunks: ptr.To(int32(2)),
250+
},
251+
},
252+
}
253+
// Only 2 distinct domains for npcs=2 -- must NOT be allowed through
254+
// (requires npcs+2 = 4).
255+
nodeSet := &simplyblockv1alpha1.StorageNodeSet{
256+
ObjectMeta: metav1.ObjectMeta{Name: "set-fd-wait", Namespace: "default"},
257+
Spec: simplyblockv1alpha1.StorageNodeSetSpec{ClusterName: "cluster-fd-wait"},
258+
Status: simplyblockv1alpha1.StorageNodeSetStatus{
259+
Nodes: []simplyblockv1alpha1.NodeStatus{
260+
{Hostname: "w0", MgmtIp: "10.0.0.1", FailureDomain: fd(0)},
261+
{Hostname: "w1", MgmtIp: "10.0.0.2", FailureDomain: fd(1)},
262+
},
263+
},
264+
}
265+
266+
r := newClusterStateTestReconciler(t, cluster, nodeSet)
267+
268+
res, err := r.reconcileActivate(context.Background(), cluster)
269+
if err != nil {
270+
t.Fatalf("reconcileActivate returned error: %v", err)
271+
}
272+
if res.RequeueAfter == 0 {
273+
t.Fatalf("expected a requeue while waiting on failure-domain readiness")
274+
}
275+
if cluster.Status.ActionStatus != nil {
276+
t.Fatalf("expected ActionStatus to stay untouched while not ready, got %#v", cluster.Status.ActionStatus)
277+
}
278+
}
279+
280+
func TestReconcileActivateProceedsOnceFailureDomainsAreReady(t *testing.T) {
281+
fd := func(v int32) *int32 { return &v }
282+
283+
cluster := &simplyblockv1alpha1.StorageCluster{
284+
ObjectMeta: metav1.ObjectMeta{
285+
Name: "cluster-fd-ready",
286+
Namespace: "default",
287+
},
288+
Spec: simplyblockv1alpha1.StorageClusterSpec{
289+
Action: utils.ClusterActionActivate,
290+
EnableFailureDomains: ptr.To(true),
291+
StripeSpec: &simplyblockv1alpha1.StripeSpec{
292+
ParityChunks: ptr.To(int32(2)),
293+
},
294+
},
295+
}
296+
// 4 distinct, equally-sized domains for npcs=2 -- satisfies npcs+2 = 4,
297+
// so the gate must let this through to the normal init-action path.
298+
nodeSet := &simplyblockv1alpha1.StorageNodeSet{
299+
ObjectMeta: metav1.ObjectMeta{Name: "set-fd-ready", Namespace: "default"},
300+
Spec: simplyblockv1alpha1.StorageNodeSetSpec{ClusterName: "cluster-fd-ready"},
301+
Status: simplyblockv1alpha1.StorageNodeSetStatus{
302+
Nodes: []simplyblockv1alpha1.NodeStatus{
303+
{Hostname: "w0", MgmtIp: "10.0.0.1", FailureDomain: fd(0)},
304+
{Hostname: "w1", MgmtIp: "10.0.0.2", FailureDomain: fd(1)},
305+
{Hostname: "w2", MgmtIp: "10.0.0.3", FailureDomain: fd(2)},
306+
{Hostname: "w3", MgmtIp: "10.0.0.4", FailureDomain: fd(3)},
307+
},
308+
},
309+
}
310+
311+
r := newClusterStateTestReconciler(t, cluster, nodeSet)
312+
313+
res, err := r.reconcileActivate(context.Background(), cluster)
314+
if err != nil {
315+
t.Fatalf("reconcileActivate returned error: %v", err)
316+
}
317+
if cluster.Status.ActionStatus == nil {
318+
t.Fatalf("expected ActionStatus to be initialized once failure-domain readiness is satisfied")
319+
}
320+
if cluster.Status.ActionStatus.State != utils.ActionStateRunning {
321+
t.Fatalf("expected Running state, got %#v", cluster.Status.ActionStatus)
322+
}
323+
if res.Requeue != true {
324+
t.Fatalf("expected immediate requeue for the init-action step, got %+v", res)
325+
}
326+
}
327+
149328
func TestReconcileExpandTransitions(t *testing.T) {
150329
t.Run("initializes running status for expand with observed generation", func(t *testing.T) {
151330
cluster := &simplyblockv1alpha1.StorageCluster{

0 commit comments

Comments
 (0)