Carry the delete capability through the indirection that hid it
`ctl weights --collect` answered "this control plane cannot ask nodes to delete files" against a real cluster, with the whole suite green. The controller is handed a dispatcherRef rather than the agent service — an indirection that exists to break a construction cycle, and one that satisfies the dispatcher interface by forwarding each method by hand. The controller reaches past that interface by type assertion to find capabilities beyond scheduling, and an assertion against dispatcherRef fails for anything nobody remembered to forward. Nothing could have caught it below the wiring. The dispatchers the tests use do not implement the capability either, so its absence there is the expected case and looks identical to this bug. The test therefore asserts on the production types themselves: that the forwarding dispatcher carries the capability, and that the service it forwards to has one to carry. Checked by deleting the method and watching it fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
774e134ed1
commit
e6bc7859dc
2 files changed
+52
No files matched your search
@@ -0,0 +1,32 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/control"
|
||||
"gitea.mynodes.xyz/ilker/llmgw/internal/control/agentsvc"
|
||||
)
|
||||
|
||||
func TestTheForwardingDispatcherCarriesEveryCapability(t *testing.T) {
|
||||
// dispatcherRef exists to break a construction cycle, and it satisfies the
|
||||
// dispatcher interface by forwarding each method by hand. That makes it a
|
||||
// place where capabilities go missing silently: the controller reaches
|
||||
// past the interface by type assertion, and an assertion against this type
|
||||
// fails for anything nobody remembered to forward.
|
||||
//
|
||||
// It happened. `ctl weights --collect` returned "this control plane cannot
|
||||
// ask nodes to delete files" against a real cluster, while every unit test
|
||||
// passed — because the test dispatchers do not implement the capability
|
||||
// either, so its absence looked exactly like the expected case.
|
||||
var ref any = &dispatcherRef{}
|
||||
if _, ok := ref.(control.WeightRemover); !ok {
|
||||
t.Error("dispatcherRef drops WeightRemover, so the control plane cannot reclaim disk")
|
||||
}
|
||||
|
||||
// And the thing it forwards to must have it, or the forwarding has nothing
|
||||
// to reach.
|
||||
var svc any = &agentsvc.Service{}
|
||||
if _, ok := svc.(control.WeightRemover); !ok {
|
||||
t.Error("the agent service does not implement WeightRemover")
|
||||
}
|
||||
}
|
||||
@@ -372,6 +372,26 @@ func (r *dispatcherRef) get() (reconciler.Dispatcher, error) {
|
||||
return r.d, nil
|
||||
}
|
||||
|
||||
// RemoveWeights forwards a capability beyond scheduling.
|
||||
//
|
||||
// It has to be spelled out here rather than inherited, because this type
|
||||
// satisfies reconciler.Dispatcher by forwarding each method by hand — so
|
||||
// anything the controller reaches by type assertion stops at this boundary
|
||||
// unless it is forwarded too. A capability added to the service and not to
|
||||
// this type is simply absent at runtime, with nothing at compile time to say
|
||||
// so.
|
||||
func (r *dispatcherRef) RemoveWeights(ctx context.Context, nodeID string, paths []string) error {
|
||||
d, err := r.get()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
rm, ok := d.(control.WeightRemover)
|
||||
if !ok {
|
||||
return errors.New("the agent service cannot remove weights")
|
||||
}
|
||||
return rm.RemoveWeights(ctx, nodeID, paths)
|
||||
}
|
||||
|
||||
func (r *dispatcherRef) StartShard(ctx context.Context, cmd reconciler.ShardCommand) error {
|
||||
d, err := r.get()
|
||||
if err != nil {
|
||||
|
||||
Reference in new issue
Block a user