Skip to content

Commit 9d33037

Browse files
committed
fix(workloadmonitor): keep a bucket not ready until its claim is provisioned
A bucket WorkloadMonitor has no pods and minReplicas 0, so it stayed operational, and the Bucket app stayed Ready, while its BucketClaim was never provisioned. A tenant whose nearest S3 ancestor runs no SeaweedFS gets a claim for a bucket class that does not exist, and the dashboard showed a ready bucket that was never created. Assisted-by: LLM Signed-off-by: Yan Bondarenko <[email protected]>
1 parent 94ada11 commit 9d33037

3 files changed

Lines changed: 168 additions & 3 deletions

File tree

‎api/v1alpha1/workloadmonitor_types.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ type WorkloadMonitorSpec struct {
3939
// that is not operational because a DataVolume it selects is not ready.
4040
const WorkloadMonitorReasonDataVolumeNotReady = "DataVolumeNotReady"
4141

42+
// WorkloadMonitorReasonBucketClaimNotReady is the status Reason of a monitor
43+
// that is not operational because a BucketClaim it selects is not provisioned.
44+
const WorkloadMonitorReasonBucketClaimNotReady = "BucketClaimNotReady"
45+
4246
// WorkloadMonitorStatus defines the observed state of WorkloadMonitor
4347
type WorkloadMonitorStatus struct {
4448
// Operational indicates if the workload meets all operational requirements

‎internal/controller/workloadmonitor_controller.go‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,25 @@ func (r *WorkloadMonitorReconciler) isBucketClaimReady(bc *cosiv1alpha1.BucketCl
374374
return bc.Status.BucketReady
375375
}
376376

377+
// bucketClaimsMessage names every BucketClaim that has not been provisioned,
378+
// with its bucket class, and is empty when there is none.
379+
func (r *WorkloadMonitorReconciler) bucketClaimsMessage(claims []cosiv1alpha1.BucketClaim) string {
380+
var pending []string
381+
for i := range claims {
382+
bc := &claims[i]
383+
if r.isBucketClaimReady(bc) {
384+
continue
385+
}
386+
entry := fmt.Sprintf("BucketClaim %s is not provisioned", bc.Name)
387+
if bc.Spec.BucketClassName != "" {
388+
entry = fmt.Sprintf("BucketClaim %s of bucket class %s is not provisioned", bc.Name, bc.Spec.BucketClassName)
389+
}
390+
pending = append(pending, entry)
391+
}
392+
sort.Strings(pending)
393+
return strings.Join(pending, "; ")
394+
}
395+
377396
// isServiceReady checks if the service has an external IP bound
378397
func (r *WorkloadMonitorReconciler) isServiceReady(svc *corev1.Service) bool {
379398
return len(svc.Status.LoadBalancer.Ingress) > 0
@@ -1157,6 +1176,8 @@ func (r *WorkloadMonitorReconciler) Reconcile(ctx context.Context, req ctrl.Requ
11571176
}
11581177
}
11591178

1179+
bucketsMessage := r.bucketClaimsMessage(bucketClaimList.Items)
1180+
11601181
// Update WorkloadMonitor status based on observed pods
11611182
monitor.Status.ObservedReplicas = observedReplicas
11621183
monitor.Status.AvailableReplicas = availableReplicas
@@ -1178,15 +1199,23 @@ func (r *WorkloadMonitorReconciler) Reconcile(ctx context.Context, req ctrl.Requ
11781199
if fresh.Spec.MinReplicas != nil && availableReplicas < *fresh.Spec.MinReplicas {
11791200
fresh.Status.Operational = pointer.Bool(false)
11801201
}
1181-
// Only the DataVolume check writes Message and Reason, so the stored ones
1182-
// are the last DataVolume verdict: kept while no reader is installed yet, so a
1183-
// controller restart does not report a stuck disk as ready.
1202+
// A stored DataVolume verdict is the last one: kept while no reader is
1203+
// installed yet, so a controller restart does not report a stuck disk as
1204+
// ready. BucketClaims are listed on every pass, so their verdict is
1205+
// recomputed and only fills fields no DataVolume verdict holds.
11841206
if dataVolumesRead {
11851207
fresh.Status.Message = dataVolumesMessage
11861208
fresh.Status.Reason = ""
11871209
if dataVolumesMessage != "" {
11881210
fresh.Status.Reason = cozyv1alpha1.WorkloadMonitorReasonDataVolumeNotReady
11891211
}
1212+
} else if fresh.Status.Reason == cozyv1alpha1.WorkloadMonitorReasonBucketClaimNotReady {
1213+
fresh.Status.Message = ""
1214+
fresh.Status.Reason = ""
1215+
}
1216+
if fresh.Status.Message == "" && bucketsMessage != "" {
1217+
fresh.Status.Message = bucketsMessage
1218+
fresh.Status.Reason = cozyv1alpha1.WorkloadMonitorReasonBucketClaimNotReady
11901219
}
11911220
if fresh.Status.Message != "" {
11921221
fresh.Status.Operational = pointer.Bool(false)

‎internal/controller/workloadmonitor_controller_test.go‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2975,3 +2975,135 @@ func TestReconcileBucketApp_LegacySizeWinsOverOwnerlessNewWorkload(t *testing.T)
29752975
t.Errorf("size = %s, want the legacy 689Gi over the stale 10Gi", got.String())
29762976
}
29772977
}
2978+
2979+
func newBucketClaim(name string, labels map[string]string, class string, ready bool) *cosiv1alpha1.BucketClaim {
2980+
return &cosiv1alpha1.BucketClaim{
2981+
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default", Labels: labels},
2982+
Spec: cosiv1alpha1.BucketClaimSpec{
2983+
BucketClassName: class,
2984+
Protocols: []cosiv1alpha1.Protocol{cosiv1alpha1.ProtocolS3},
2985+
},
2986+
Status: cosiv1alpha1.BucketClaimStatus{BucketReady: ready},
2987+
}
2988+
}
2989+
2990+
func reconcileBucketMonitor(t *testing.T, c client.Client, s *runtime.Scheme, withReader bool) *cozyv1alpha1.WorkloadMonitor {
2991+
t.Helper()
2992+
reconciler := &WorkloadMonitorReconciler{Client: c, Scheme: s}
2993+
if withReader {
2994+
reconciler.DataVolumeReader = c
2995+
}
2996+
req := reconcile.Request{NamespacedName: types.NamespacedName{Name: "my-bucket", Namespace: "default"}}
2997+
if _, err := reconciler.Reconcile(context.TODO(), req); err != nil {
2998+
t.Fatalf("Reconcile returned error: %v", err)
2999+
}
3000+
updated := &cozyv1alpha1.WorkloadMonitor{}
3001+
if err := c.Get(context.TODO(), req.NamespacedName, updated); err != nil {
3002+
t.Fatalf("Failed to get updated WorkloadMonitor: %v", err)
3003+
}
3004+
if updated.Status.Operational == nil {
3005+
t.Fatal("Expected Operational to be set, got nil")
3006+
}
3007+
return updated
3008+
}
3009+
3010+
func newBucketMonitorClient(s *runtime.Scheme, objs ...client.Object) client.Client {
3011+
monitor := &cozyv1alpha1.WorkloadMonitor{
3012+
ObjectMeta: metav1.ObjectMeta{Name: "my-bucket", Namespace: "default"},
3013+
Spec: cozyv1alpha1.WorkloadMonitorSpec{
3014+
Kind: "bucket",
3015+
Type: "s3",
3016+
Selector: map[string]string{"app.kubernetes.io/instance": "my-bucket"},
3017+
Replicas: ptr.To[int32](0),
3018+
MinReplicas: ptr.To[int32](0),
3019+
},
3020+
}
3021+
return fake.NewClientBuilder().
3022+
WithScheme(s).
3023+
WithObjects(append([]client.Object{monitor}, objs...)...).
3024+
WithStatusSubresource(monitor).
3025+
Build()
3026+
}
3027+
3028+
func TestReconcile_BucketClaimDecidesOperational(t *testing.T) {
3029+
selected := map[string]string{"app.kubernetes.io/instance": "my-bucket"}
3030+
cases := []struct {
3031+
name string
3032+
claim *cosiv1alpha1.BucketClaim
3033+
operational bool
3034+
message string
3035+
}{
3036+
{"provisioned", newBucketClaim("my-bucket", selected, "tenant-root", true), true, ""},
3037+
{"not provisioned", newBucketClaim("my-bucket", selected, "tenant-root", false), false, "BucketClaim my-bucket of bucket class tenant-root is not provisioned"},
3038+
{"not provisioned without a class", newBucketClaim("my-bucket", selected, "", false), false, "BucketClaim my-bucket is not provisioned"},
3039+
{"outside the selector", newBucketClaim("other", map[string]string{"app.kubernetes.io/instance": "other"}, "tenant-root", false), true, ""},
3040+
}
3041+
for _, tc := range cases {
3042+
t.Run(tc.name, func(t *testing.T) {
3043+
s := newTestScheme()
3044+
got := reconcileBucketMonitor(t, newBucketMonitorClient(s, tc.claim), s, true)
3045+
if *got.Status.Operational != tc.operational {
3046+
t.Errorf("Operational = %v, want %v", *got.Status.Operational, tc.operational)
3047+
}
3048+
if got.Status.Message != tc.message {
3049+
t.Errorf("Message = %q, want %q", got.Status.Message, tc.message)
3050+
}
3051+
wantReason := ""
3052+
if tc.message != "" {
3053+
wantReason = cozyv1alpha1.WorkloadMonitorReasonBucketClaimNotReady
3054+
}
3055+
if got.Status.Reason != wantReason {
3056+
t.Errorf("Reason = %q, want %q", got.Status.Reason, wantReason)
3057+
}
3058+
})
3059+
}
3060+
}
3061+
3062+
func TestReconcile_ProvisionedBucketClaimClearsTheVerdict(t *testing.T) {
3063+
for _, withReader := range []bool{true, false} {
3064+
t.Run(fmt.Sprintf("dataVolumeReader=%v", withReader), func(t *testing.T) {
3065+
s := newTestScheme()
3066+
claim := newBucketClaim("my-bucket", map[string]string{"app.kubernetes.io/instance": "my-bucket"}, "tenant-root", false)
3067+
c := newBucketMonitorClient(s, claim)
3068+
3069+
got := reconcileBucketMonitor(t, c, s, withReader)
3070+
if *got.Status.Operational || got.Status.Reason != cozyv1alpha1.WorkloadMonitorReasonBucketClaimNotReady {
3071+
t.Fatalf("before provisioning: Operational = %v, Reason = %q", *got.Status.Operational, got.Status.Reason)
3072+
}
3073+
3074+
stored := &cosiv1alpha1.BucketClaim{}
3075+
if err := c.Get(context.TODO(), types.NamespacedName{Name: "my-bucket", Namespace: "default"}, stored); err != nil {
3076+
t.Fatalf("Failed to get BucketClaim: %v", err)
3077+
}
3078+
stored.Status.BucketReady = true
3079+
if err := c.Update(context.TODO(), stored); err != nil {
3080+
t.Fatalf("Failed to update BucketClaim: %v", err)
3081+
}
3082+
3083+
got = reconcileBucketMonitor(t, c, s, withReader)
3084+
if !*got.Status.Operational || got.Status.Message != "" || got.Status.Reason != "" {
3085+
t.Errorf("after provisioning: Operational = %v, Message = %q, Reason = %q", *got.Status.Operational, got.Status.Message, got.Status.Reason)
3086+
}
3087+
})
3088+
}
3089+
}
3090+
3091+
func TestReconcile_DataVolumeVerdictOutranksBucketClaim(t *testing.T) {
3092+
s := newTestScheme()
3093+
selected := map[string]string{"app.kubernetes.io/instance": "my-bucket"}
3094+
phase := "ImportInProgress"
3095+
c := newBucketMonitorClient(s,
3096+
newBucketClaim("my-bucket", selected, "tenant-root", false),
3097+
newDataVolume("my-bucket", selected, &phase),
3098+
)
3099+
got := reconcileBucketMonitor(t, c, s, true)
3100+
if *got.Status.Operational {
3101+
t.Error("Operational = true, want false")
3102+
}
3103+
if got.Status.Reason != cozyv1alpha1.WorkloadMonitorReasonDataVolumeNotReady {
3104+
t.Errorf("Reason = %q, want %q", got.Status.Reason, cozyv1alpha1.WorkloadMonitorReasonDataVolumeNotReady)
3105+
}
3106+
if got.Status.Message != "DataVolume my-bucket is ImportInProgress" {
3107+
t.Errorf("Message = %q", got.Status.Message)
3108+
}
3109+
}

0 commit comments

Comments
 (0)