Skip to content

Commit 77e8b78

Browse files
kvapsclaude
andcommitted
feat(delete): finalizer-driven Resource teardown via DeleteResource RPC
New satellite RPC: DeleteResource(name, storage_pool, volume_numbers) runs `drbdadm down` (best-effort) → DeleteVolume on every numbered volume through the matching Provider → rm /etc/drbd.d/<name>.res. Idempotent on missing. ResourceReconciler now installs a finalizer "blockstor.io.blockstor.io/resource" on every live Resource. When DeletionTimestamp arrives, runDelete dials the satellite, asks it to tear the resource down, and only then strips the finalizer so kube-apiserver completes the delete. Without this the satellite never sees the delete and the .res / LV / loopfile orphan. Reconcile loop split for funlen budget: runApply (gRPC dispatch) and runDelete (teardown). Boilerplate envtest stays a no-op when Dispatcher is nil. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent edf5dd8 commit 77e8b78

7 files changed

Lines changed: 802 additions & 377 deletions

File tree

internal/controller/resource_controller.go

Lines changed: 83 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package controller
1818

1919
import (
2020
"context"
21+
"slices"
2122
"time"
2223

2324
"k8s.io/apimachinery/pkg/api/errors"
@@ -30,6 +31,11 @@ import (
3031
"github.com/cozystack/blockstor/pkg/dispatcher"
3132
)
3233

34+
// resourceFinalizer guards on-disk + DRBD-side teardown when the
35+
// Resource CRD is deleted. Without it the satellite would never see
36+
// the delete, leaving an orphan .res file + LV / loopfile.
37+
const resourceFinalizer = "blockstor.io.blockstor.io/resource"
38+
3339
// ResourceReconciler dispatches Resource CRD changes to the right
3440
// satellite via the Dispatcher. It collects same-RD peers and the
3541
// full Node list (for endpoint resolution) on every reconcile —
@@ -51,8 +57,6 @@ type ResourceReconciler struct {
5157
// to the satellite that hosts it. Per-replica errors land in the
5258
// log; transport faults trigger a 10s requeue.
5359
func (r *ResourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
54-
log := logf.FromContext(ctx)
55-
5660
if r.Dispatcher == nil {
5761
// envtest scaffolding (suite_test.go) constructs the reconciler
5862
// without a Dispatcher — keep the original no-op behaviour for
@@ -71,9 +75,37 @@ func (r *ResourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
7175
return ctrl.Result{}, err
7276
}
7377

78+
// Deletion path: tell the satellite to drop the resource, then
79+
// remove the finalizer so kube-apiserver finishes the delete.
80+
if !target.DeletionTimestamp.IsZero() {
81+
return r.runDelete(ctx, &target)
82+
}
83+
84+
// Ensure finalizer is present on every live Resource so the
85+
// delete path above can run.
86+
if !slices.Contains(target.Finalizers, resourceFinalizer) {
87+
target.Finalizers = append(target.Finalizers, resourceFinalizer)
88+
89+
err = r.Update(ctx, &target)
90+
if err != nil {
91+
return ctrl.Result{}, err
92+
}
93+
94+
return ctrl.Result{Requeue: true}, nil
95+
}
96+
97+
return r.runApply(ctx, &target)
98+
}
99+
100+
// runApply is the apply branch of Reconcile. Pulled out to keep
101+
// Reconcile under the funlen budget — the body deals with the
102+
// finalizer dance, this with the actual gRPC dispatch.
103+
func (r *ResourceReconciler) runApply(ctx context.Context, target *blockstoriov1alpha1.Resource) (ctrl.Result, error) {
104+
log := logf.FromContext(ctx)
105+
74106
var resList blockstoriov1alpha1.ResourceList
75107

76-
err = r.List(ctx, &resList)
108+
err := r.List(ctx, &resList)
77109
if err != nil {
78110
return ctrl.Result{}, err
79111
}
@@ -98,7 +130,7 @@ func (r *ResourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
98130
return ctrl.Result{}, err
99131
}
100132

101-
result, err := r.Dispatcher.Apply(ctx, &target, peers, nodeList.Items, rdPtr)
133+
result, err := r.Dispatcher.Apply(ctx, target, peers, nodeList.Items, rdPtr)
102134
if err != nil {
103135
log.Error(err, "Apply RPC failed", "resource", target.Name, "node", target.Spec.NodeName)
104136

@@ -116,6 +148,53 @@ func (r *ResourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
116148
return ctrl.Result{}, nil
117149
}
118150

151+
// runDelete is the finalizer-driven teardown. We dial the satellite
152+
// to drop the resource (drbdadm down → DeleteVolume → rm .res), then
153+
// strip the finalizer so kube-apiserver completes the delete.
154+
// Failures requeue with a 10 s back-off so the resource isn't stuck
155+
// half-gone if a satellite is briefly unreachable.
156+
func (r *ResourceReconciler) runDelete(ctx context.Context, target *blockstoriov1alpha1.Resource) (ctrl.Result, error) {
157+
log := logf.FromContext(ctx)
158+
159+
if !slices.Contains(target.Finalizers, resourceFinalizer) {
160+
return ctrl.Result{}, nil
161+
}
162+
163+
rdPtr, _ := r.lookupRD(ctx, target.Spec.ResourceDefinitionName)
164+
165+
var nodeList blockstoriov1alpha1.NodeList
166+
167+
err := r.List(ctx, &nodeList)
168+
if err != nil {
169+
return ctrl.Result{}, err
170+
}
171+
172+
resp, err := r.Dispatcher.DeleteResource(ctx, target, rdPtr, nodeList.Items)
173+
if err != nil {
174+
log.Error(err, "DeleteResource RPC failed",
175+
"resource", target.Name, "node", target.Spec.NodeName)
176+
177+
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
178+
}
179+
180+
if !resp.GetOk() {
181+
log.Info("satellite rejected delete", "msg", resp.GetMessage(),
182+
"resource", target.Name, "node", target.Spec.NodeName)
183+
184+
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
185+
}
186+
187+
target.Finalizers = slices.DeleteFunc(target.Finalizers,
188+
func(s string) bool { return s == resourceFinalizer })
189+
190+
err = r.Update(ctx, target)
191+
if err != nil {
192+
return ctrl.Result{}, err
193+
}
194+
195+
return ctrl.Result{}, nil
196+
}
197+
119198
// lookupRD fetches the parent ResourceDefinition. A NotFound is
120199
// converted to (nil, nil) so the dispatcher can still push the
121200
// .res for connection setup; any other error bubbles.

pkg/dispatcher/dispatcher.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,48 @@ func buildVolumes(rd *blockstoriov1alpha1.ResourceDefinition, target *blockstori
230230
return out
231231
}
232232

233+
// DeleteResource dials the target satellite's endpoint and asks it
234+
// to drop the resource (drbdadm down → DeleteVolume → rm .res).
235+
// Returns the per-satellite result. Missing endpoint surfaces as a
236+
// nil response with an error — callers retry once the Node CRD
237+
// catches up.
238+
func (d *Dispatcher) DeleteResource(ctx context.Context, target *blockstoriov1alpha1.Resource, rd *blockstoriov1alpha1.ResourceDefinition, nodes []blockstoriov1alpha1.Node) (*satellitepb.DeleteResourceResponse, error) {
239+
endpoint := lookupEndpoint(target.Spec.NodeName, nodes)
240+
if endpoint == "" {
241+
return nil, errors.Errorf("no SatelliteEndpoint for node %q", target.Spec.NodeName)
242+
}
243+
244+
client, closer, err := d.dialer.Dial(ctx, endpoint)
245+
if err != nil {
246+
return nil, errors.Wrapf(err, "dial %s", endpoint)
247+
}
248+
defer func() { _ = closer() }()
249+
250+
pool := target.Spec.Props["StorPoolName"]
251+
if pool == "" && rd != nil {
252+
pool = rd.Spec.Props["StorPoolName"]
253+
}
254+
255+
volNumbers := make([]int32, 0)
256+
257+
if rd != nil {
258+
for _, vd := range rd.Spec.VolumeDefinitions {
259+
volNumbers = append(volNumbers, vd.VolumeNumber)
260+
}
261+
}
262+
263+
resp, err := client.DeleteResource(ctx, &satellitepb.DeleteResourceRequest{
264+
Name: target.Spec.ResourceDefinitionName,
265+
StoragePool: pool,
266+
VolumeNumbers: volNumbers,
267+
})
268+
if err != nil {
269+
return nil, errors.Wrap(err, "DeleteResource RPC")
270+
}
271+
272+
return resp, nil
273+
}
274+
233275
// CreateSnapshot dials every satellite that hosts a non-DISKLESS
234276
// replica of `rdName` and asks it to take a snapshot. Returns the
235277
// list of per-node results so the controller can surface granular

pkg/satellite/grpc_server.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,12 @@ func (g *GRPCServer) ApplyStoragePools(_ context.Context, req *satellitepb.Apply
6969
return &satellitepb.ApplyStoragePoolsResponse{Results: results}, nil
7070
}
7171

72+
// DeleteResource tears down a resource (drbdadm down → DeleteVolume
73+
// → remove .res). Per-step errors land in the response Ok=false.
74+
func (g *GRPCServer) DeleteResource(ctx context.Context, req *satellitepb.DeleteResourceRequest) (*satellitepb.DeleteResourceResponse, error) {
75+
return g.rec.DeleteResource(ctx, req)
76+
}
77+
7278
// CreateSnapshot routes through the Reconciler's existing snapshot
7379
// path. Per-snapshot errors land in the response body (Ok=false), the
7480
// gRPC error path is reserved for context cancellation / transport

0 commit comments

Comments
 (0)