diff --git a/core/http/middleware/failover.go b/core/http/middleware/failover.go index 01ab807f3..e2df6285f 100644 --- a/core/http/middleware/failover.go +++ b/core/http/middleware/failover.go @@ -49,13 +49,15 @@ func (re *RequestExtractor) resolveFailover(c echo.Context, requested string, ch } for { cfg, err := re.loadFailoverTarget(st.attempt.Target()) - if err == nil && cfg.IsDisabled() { + // A target that became a chain after its chain was saved has no + // backend of its own; like a disabled one it is skipped, not tripped. + if err == nil && (cfg.IsDisabled() || cfg.IsFailover()) { // Disabled on purpose, not broken: move on without a trip. if st.attempt.Skip() { continue } c.Set(ContextKeyFailoverAttempt, nil) - return nil, fmt.Errorf("failover chain %q: target %q is disabled", chain.Name, cfg.Name) + return nil, fmt.Errorf("failover chain %q: target %q is disabled or is itself a chain", chain.Name, cfg.Name) } if err == nil { failover.PrepareTarget(cfg) // cfg is a copy diff --git a/core/services/failover/manager.go b/core/services/failover/manager.go index 8a9ea7e0b..878c1c867 100644 --- a/core/services/failover/manager.go +++ b/core/services/failover/manager.go @@ -198,6 +198,14 @@ func (m *Manager) syncLocked() { m.setTargetLocked(ts, StateMissing, ReasonMissing, "target config not found") continue } + if tc.IsFailover() { + // Nested chains are rejected when a chain is saved, but a + // target edited into a chain later slips past that check. + // Serving its chain config would load a model with no + // backend, so treat it as unusable. + m.setTargetLocked(ts, StateMissing, ReasonMissing, "target is itself a failover chain") + continue + } ts.kind = KindOf(tc) ts.serving = tc.Name if t.Warm && ts.kind == KindLocal { diff --git a/core/services/failover/manager_test.go b/core/services/failover/manager_test.go index b74e32920..69326920c 100644 --- a/core/services/failover/manager_test.go +++ b/core/services/failover/manager_test.go @@ -260,6 +260,23 @@ var _ = Describe("Manager", func() { Expect(m.WarmTargets()).To(Equal([]string{"b"})) }) + It("never plans a target that has since become a chain itself", func() { + // Validation rejects a nested chain when the outer chain is saved, + // but not when one of its targets is later edited into a chain. + src.Put(chainCfg("inner", nil, t("a"))) + src.Put(chainCfg("chain", nil, t("a"), t("inner"))) + m.Sync() + m.ReportFailure("a", errBoom) + + st, ok := m.ChainStatus("chain") + Expect(ok).To(BeTrue()) + Expect(st.Targets[1].State).To(Equal(StateMissing)) + att, err := m.Plan("chain") + if err == nil { + Expect(att.Target()).NotTo(Equal("inner")) + } + }) + It("closes a subscription on cancel", func() { events, cancel := m.Subscribe(1) cancel()