Repository: scionproto/scion
Pull request #3661 (2020-02-07 14:49:54)

Refactor itopo initialization
Most of the initializers used default values. Changing to
a config struct makes the itopo package easier to refactor.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/scionproto/scion/3661)
<!-- Reviewable:end -->



Patch:
diff --git a/go/border/brconf/conf.go b/go/border/brconf/conf.go
index 4c7b671c9..eab9552c0 100644
--- a/go/border/brconf/conf.go
+++ b/go/border/brconf/conf.go
@@ -83,7 +83,7 @@ func (cfg *BRConf) loadTopo(id string) error {
 	return nil
 }
 
-// initTopo initializesthe entries related to topo in the config.
+// initTopo initializes the entries related to topo in the config.
 func (cfg *BRConf) initTopo(id string, topo topology.Topology) error {
 	cfg.Topo = topo
 	cfg.IA = cfg.Topo.IA()
diff --git a/go/border/setup.go b/go/border/setup.go
index e506aa563..4297c352a 100644
--- a/go/border/setup.go
+++ b/go/border/setup.go
@@ -69,10 +69,16 @@ func (r *Router) setup() error {
 		return err
 	}
 	// Initialize itopo.
-	itopo.Init(r.Id, proto.ServiceType_br, itopo.Callbacks{})
-	if _, _, err := itopo.SetStatic(conf.Topo); err != nil {
+	itopo.Init(
+		&itopo.Config{
+			ID:  r.Id,
+			Svc: proto.ServiceType_br,
+		},
+	)
+	if err := itopo.Update(conf.Topo); err != nil {
 		return err
 	}
+	conf.Topo = itopo.Get()
 	// Setup new context.
 	if err = r.setupCtxFromConfig(conf); err != nil {
 		return err
@@ -120,13 +126,13 @@ func (r *Router) setupCtxFromConfig(config *brconf.BRConf) error {
 	// We want to keep in sync itopo and the context that is set.
 	// We attempt to set the context with the topology that will be current
 	// after setting itopo. If setting itopo fails in the end, we rollback the context.
-	tx, err := itopo.BeginSetStatic(config.Topo.Writable())
+	tx, err := itopo.BeginUpdate(config.Topo.Writable())
 	if err != nil {
 		return err
 	}
 	// Set config to use the appropriate topology. The returned topology is
 	// not necessarily the same as config.Topo. It can be another static topology.
-	newConf, err := brconf.WithNewTopo(r.Id, topology.FromRWTopology(tx.Get()), config)
+	newConf, err := brconf.WithNewTopo(r.Id, tx.Get(), config)
 	if err != nil {
 		return err
 	}
@@ -138,7 +144,7 @@ func (r *Router) setupCtxFromConfig(config *brconf.BRConf) error {
 func (r *Router) setupCtxFromStatic(topo *topology.RWTopology) (bool, error) {
 	r.setCtxMtx.Lock()
 	defer r.setCtxMtx.Unlock()
-	tx, err := itopo.BeginSetStatic(topo)
+	tx, err := itopo.BeginUpdate(topo)
 	return r.setupCtxFromTopoUpdate(tx, err)
 }
 
@@ -151,7 +157,7 @@ func (r *Router) setupCtxFromTopoUpdate(tx itopo.Transaction, err error) (bool,
 		return false, nil
 	}
 	log.Trace("====> Setting up new context from topology update")
-	newConf, err := brconf.WithNewTopo(r.Id, topology.FromRWTopology(tx.Get()), rctx.Get().Conf)
+	newConf, err := brconf.WithNewTopo(r.Id, tx.Get(), rctx.Get().Conf)
 	if err != nil {
 		return false, err
 	}
diff --git a/go/cs/main.go b/go/cs/main.go
index 24b1cbfad..fe07e3677 100644
--- a/go/cs/main.go
+++ b/go/cs/main.go
@@ -715,13 +715,18 @@ func setup() error {
 	if err := cfg.Validate(); err != nil {
 		return common.NewBasicError("Unable to validate config", err)
 	}
-	clbks := itopo.Callbacks{UpdateStatic: handleTopoUpdate}
-	// Use CS for monolith for now
-	itopo.Init(cfg.General.ID, proto.ServiceType_cs, clbks)
 	topo, err := topology.FromJSONFile(cfg.General.Topology)
 	if err != nil {
 		return common.NewBasicError("Unable to load topology", err)
 	}
+	// Use CS for monolith for now
+	itopo.Init(
+		&itopo.Config{
+			ID:        cfg.General.ID,
+			Svc:       proto.ServiceType_cs,
+			Callbacks: itopo.Callbacks{OnUpdate: handleTopoUpdate},
+		},
+	)
 	return initTopo(topo)
 }
 
@@ -733,7 +738,7 @@ func handleTopoUpdate() {
 }
 
 func initTopo(topo topology.Topology) error {
-	if _, _, err := itopo.SetStatic(topo); err != nil {
+	if err := itopo.Update(topo); err != nil {
 		return serrors.WrapStr("Unable to set initial static topology", err)
 	}
 	infraenv.InitInfraEnvironment(cfg.General.Topology)
diff --git a/go/lib/env/env.go b/go/lib/env/env.go
index 64189d466..64fa7ce2c 100644
--- a/go/lib/env/env.go
+++ b/go/lib/env/env.go
@@ -201,7 +201,7 @@ func ReloadTopology(topologyPath string) {
 		log.Error("Unable to reload topology", "err", err)
 		return
 	}
-	if _, _, err := itopo.SetStatic(topo); err != nil {
+	if err := itopo.Update(topo); err != nil {
 		log.Error("Unable to set topology", "err", err)
 		return
 	}
diff --git a/go/lib/infra/modules/itopo/doc.go b/go/lib/infra/modules/itopo/doc.go
index b6f60afea..b8e55c40f 100644
--- a/go/lib/infra/modules/itopo/doc.go
+++ b/go/lib/infra/modules/itopo/doc.go
@@ -12,126 +12,20 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-/*
-Package itopo stores the static and dynamic topology. Client packages
-that grab a reference with Get are guaranteed to receive a stable
-snapshot of the topology. The returned value is the topology that is
-currently active.
-
-There are two types of topologies, the static and the dynamic topology.
-For more information see lib/discovery.
-
-Initialization
-
-The package must be initialized with Init. In subsequent updates through
-SetStatic or SetDynamic, the new topology is checked whether it is
-compatible with the previous version. The rules differ between services.
-
-If the dynamic topology is set, the initializing client should start
-the periodic cleaner to evict expired dynamic topologies.
-
-Updates
-
-The update of the topology is only valid if a set of constraints is
-met. The constraints differ between dynamic and static topology, and
-also between the initialized service type.
-
-In a static topology update, when the diff is empty, the static
-topology is only updated if it expires later than the current static.
-Otherwise, SetStatic succeeds and indicates that the in-memory copy
-has not been updated.
-
-A static topology update can force the dynamic topology to be dropped,
-if it does no longer meet the constraints.
-
-Constraints
-
-The topology is split into five parts. An update is valid under the
-constraints, if the constraints for each part are met.
-
-Immutable:
-This part may not differ from the initial static topology.
-
-Mutable:
-This part may differ from the initial static topology. It may also
-differ between the currently active static and dynamic topology.
-
-Semi-Mutable:
-This part may differ between static topology versions. However, it
-may not differ between the current dynamic and static topology.
-If an update to the static topology modifies this part, the dynamic
-topology is dropped.
-
-Time:
-This part is ignored when validating the constraints. It is used
-to determine if a topology shall be updated if there are no
-differences in the other parts.
-
-Ignored:
-This part is always ignored.
-
-Default Topology Split
-
-The topology file for default initialization (calling Init) is split
-into immutable, mutable, time and ignored.
-
- ISD_AS                Immutable
- Core                  Immutable
- Overlay               Immutable
- MTU                   Immutable
-
- Service Entries       Mutable
- BorderRouter Entries  Mutable
-
- Timestamp             Time
- TTL                   Time
-
- TimestampHuman        Ignored
-
-Service Topology Split
-
-The topology file for services is split into immutable, mutable,
-time and ignored.
-
- ISD_AS                Immutable
- Core                  Immutable
- Overlay               Immutable
- MTU                   Immutable
- OwnSvcType[OwnID]     Immutable // The service entry for the initialized element.
-
- Service Entries       Mutable   // Except OwnSvcType[OwnID].
- BorderRouter Entries  Mutable
-
- Timestamp             Time
- TTL                   Time
-
- TimestampHuman        Ignored
-
-Border Router Topology Split
-
-The topology file for border routers is split into immutable,
-semi-mutable, mutable, time and ignored.
-
- ISD_AS                              Immutable
- Core                                Immutable
- Overlay                             Immutable
- MTU                                 Immutable
- BorderRouters[OwnId][InternalAddrs] Immutable    // Internal address of initialized router.
- BorderRouters[OwnId][CtrlAddr]      Immutable    // Control address of initialized router.
-
- BorderRouters[OwnId][Interfaces]    Semi-Mutable // Interfaces of initialized router.
-
- Service Entries       Mutable
- BorderRouter Entries  Mutable                    // Except BorderRouters[OwnId].
-
- Timestamp             Time
- TTL                   Time
-
- TimestampHuman        Ignored
-
-Callbacks
-
-The client package can register callbacks to be notified about
-certain events.
-*/
+// Package itopo stores topology state and manages topology updates for an application.
+//
+// The package must be initialized with Init. In subsequent updates through Update, the new
+// topology is checked whether it is compatible with the previous version. The rules differ between
+// services.
+//
+// Updates
+//
+// The update of the topology is only valid if a set of constraints is met. The constraints differ
+// between the initialized service types.
+//
+// In a topology update, when the diff is empty, the topology is only updated if it
+// expires later than the current topology. Otherwise, Update succeeds and indicates that the
+// in-memory copy has not been updated.
+//
+// The client package can register callbacks to be notified about certain events.
 package itopo
diff --git a/go/lib/infra/modules/itopo/itopo.go b/go/lib/infra/modules/itopo/itopo.go
index c6403d52b..6232466f4 100644
--- a/go/lib/infra/modules/itopo/itopo.go
+++ b/go/lib/infra/modules/itopo/itopo.go
@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"net/http"
 	"sync"
-	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -36,8 +35,8 @@ var st *state
 
 // Callbacks are callbacks to respond to specific topology update events.
 type Callbacks struct {
-	// UpdateStatic is called whenever the pointer to static topology is updated.
-	UpdateStatic func()
+	// OnUpdate is called whenever the pointer to static topology is updated.
+	OnUpdate func()
 }
 
 // providerFunc wraps the Get call as a topology provider.
@@ -57,31 +56,39 @@ func (f providerFunc) Get() topology.Topology {
 	return f()
 }
 
-// Init initializes itopo with the particular validator. A topology must be
-// initialized by calling SetStatic.
-func Init(id string, svc proto.ServiceType, clbks Callbacks) {
+// Config is used to initialize the package.
+type Config struct {
+	// ID is the application identifier.
+	ID string
+	// Svc is the service type of the application. Updated are treated differently depending
+	// on it.
+	Svc proto.ServiceType
+	// Callbacks can be used to run custom code on specific events.
+	Callbacks Callbacks
+	// TopologyFactory is used to build Topology facades for the underlying topology object.
+	// If nil, topology.FromRWTopology is used.
+	TopologyFactory func(*topology.RWTopology) topology.Topology
+}
+
+// Init initializes the itopo package. A topology must be initialized by calling Update.
+func Init(cfg *Config) {
 	if st != nil {
 		panic("Must not re-initialize itopo")
 	}
-	st = newState(id, svc, clbks)
+	st = newState(cfg)
 }
 
 // Get atomically gets the pointer to the current topology.
 func Get() topology.Topology {
 	st.RLock()
 	defer st.RUnlock()
-	return topology.FromRWTopology(st.topo.Get())
+	return runFactory(st.topo.Get())
 }
 
-// SetStatic atomically sets the static topology. Whether semi-mutable fields are
-// allowed to change can be specified using semiMutAllowed. The returned
-// topology is a pointer to the currently active topology at the end of the function call.
-// It might differ from the input topology (same contents as existing static,
-// or dynamic set and still valid). The second return value indicates whether the in-memory
-// copy of the static topology has been updated.
-func SetStatic(static topology.Topology) (topology.Topology, bool, error) {
+// Update atomically sets the topology.
+func Update(static topology.Topology) error {
 	l := metrics.UpdateLabels{Type: metrics.Static}
-	topo, updated, err := st.setStatic(static.Writable())
+	_, updated, err := st.setStatic(static.Writable())
 	switch {
 	case err != nil:
 		l.Result = metrics.ErrValidate
@@ -91,13 +98,13 @@ func SetStatic(static topology.Topology) (topology.Topology, bool, error) {
 		l.Result = metrics.OkIgnored
 	}
 	incUpdateMetric(l)
-	return topology.FromRWTopology(topo), updated, err
+	return err
 }
 
-// BeginSetStatic checks whether setting the static topology is permissible. The returned
+// BeginUpdate checks whether setting the static topology is permissible. The returned
 // transaction provides a view on which topology would be active, if committed.
-func BeginSetStatic(static *topology.RWTopology) (Transaction, error) {
-	tx, err := st.beginSetStatic(static)
+func BeginUpdate(static *topology.RWTopology) (Transaction, error) {
+	tx, err := st.beginUpdate(static)
 	if err != nil {
 		incUpdateMetric(metrics.UpdateLabels{Type: metrics.Static, Result: metrics.ErrValidate})
 	}
@@ -107,16 +114,14 @@ func BeginSetStatic(static *topology.RWTopology) (Transaction, error) {
 // Transaction allows to get a view on which topology will be active without committing
 // to the topology update yet.
 type Transaction struct {
-	// candidateTopo contains the view of what the static and dynamic topologies
-	// will be when the transaction is successfully committed.
+	// candidateTopo contains the view of what the topology will be when the transaction is
+	// successfully committed.
 	candidateTopo topo
 	// staticAtTxStart stores a snapshot of the currently active static
 	// topology at transaction start.
 	staticAtTxStart *topology.RWTopology
 	// inputStatic stores the provided static topology.
 	inputStatic *topology.RWTopology
-	// inputDynamic stores the provided dynamic topology.
-	inputDynamic *topology.RWTopology
 }
 
 // Commit commits the change. An error is returned, if the static topology changed in the meantime.
@@ -146,30 +151,23 @@ func (tx *Transaction) Commit() error {
 }
 
 // Get returns the topology that will be active if the transaction is committed.
-func (tx *Transaction) Get() *topology.RWTopology {
-	return tx.candidateTopo.Get()
+func (tx *Transaction) Get() topology.Topology {
+	return runFactory(tx.candidateTopo.Get())
 }
 
 // IsUpdate indicates whether the transaction will cause an update.
 func (tx *Transaction) IsUpdate() bool {
-	if tx.inputStatic != nil {
-		return tx.candidateTopo.static == tx.inputStatic
-	}
-	return tx.candidateTopo.dynamic == tx.inputDynamic
+	return tx.candidateTopo.static == tx.inputStatic
 }
 
 // topo stores the currently active static and dynamic topologies.
 type topo struct {
-	static  *topology.RWTopology
-	dynamic *topology.RWTopology
+	static *topology.RWTopology
 }
 
 // Get returns the dynamic topology if it is set and has not expired. Otherwise,
 // the static topology is returned.
 func (t *topo) Get() *topology.RWTopology {
-	if t.dynamic != nil && t.dynamic.Active(time.Now()) {
-		return t.dynamic
-	}
 	return t.static
 }
 
@@ -178,15 +176,14 @@ type state struct {
 	sync.RWMutex
 	topo      topo
 	validator validator
-	clbks     Callbacks
+	config    *Config
 }
 
-func newState(id string, svc proto.ServiceType, clbks Callbacks) *state {
-	s := &state{
-		validator: validatorFactory(id, svc),
-		clbks:     clbks,
+func newState(cfg *Config) *state {
+	return &state{
+		validator: validatorFactory(cfg.ID, cfg.Svc),
+		config:    cfg,
 	}
-	return s
 }
 
 // setStatic atomically sets the static topology.
@@ -204,7 +201,7 @@ func (s *state) setStatic(static *topology.RWTopology) (*topology.RWTopology, bo
 	return s.topo.Get(), true, nil
 }
 
-func (s *state) beginSetStatic(static *topology.RWTopology) (Transaction, error) {
+func (s *state) beginUpdate(static *topology.RWTopology) (Transaction, error) {
 	s.Lock()
 	defer s.Unlock()
 	if err := s.validator.Validate(static, s.topo.static); err != nil {
@@ -225,7 +222,7 @@ func (s *state) beginSetStatic(static *topology.RWTopology) (Transaction, error)
 // updateStatic updates the static topology, if necessary, and calls the corresponding callbacks.
 func (s *state) updateStatic(static *topology.RWTopology) {
 	s.topo.static = static
-	call(s.clbks.UpdateStatic)
+	call(s.config.Callbacks.OnUpdate)
 	cl := metrics.CurrentLabels{Type: metrics.Static}
 	metrics.Current.Timestamp(cl).Set(metrics.Timestamp(static.Timestamp))
 	metrics.Current.Expiry(cl).Set(metrics.Expiry(static.Expiry()))
@@ -272,3 +269,10 @@ func TopologyHandler(w http.ResponseWriter, r *http.Request) {
 		fmt.Fprint(w, string(bytes)+"\n")
 	}
 }
+
+func runFactory(topo *topology.RWTopology) topology.Topology {
+	if st.config.TopologyFactory != nil {
+		return st.config.TopologyFactory(topo)
+	}
+	return topology.FromRWTopology(topo)
+}
diff --git a/go/lib/topology/mock_topology/topology.go b/go/lib/topology/mock_topology/topology.go
index 0d6d305d0..a180b3d9b 100644
--- a/go/lib/topology/mock_topology/topology.go
+++ b/go/lib/topology/mock_topology/topology.go
@@ -96,20 +96,6 @@ func (mr *MockTopologyMockRecorder) Core() *gomock.Call {
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Core", reflect.TypeOf((*MockTopology)(nil).Core))
 }
 
-// DS mocks base method
-func (m *MockTopology) DS() topology.IDAddrMap {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "DS")
-	ret0, _ := ret[0].(topology.IDAddrMap)
-	return ret0
-}
-
-// DS indicates an expected call of DS
-func (mr *MockTopologyMockRecorder) DS() *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DS", reflect.TypeOf((*MockTopology)(nil).DS))
-}
-
 // Exists mocks base method
 func (m *MockTopology) Exists(arg0 addr.HostSVC, arg1 string) bool {
 	m.ctrl.T.Helper()
diff --git a/go/sciond/main.go b/go/sciond/main.go
index 67e860e92..b25fd1721 100644
--- a/go/sciond/main.go
+++ b/go/sciond/main.go
@@ -219,12 +219,12 @@ func setup() error {
 	if err := cfg.Validate(); err != nil {
 		return common.NewBasicError("unable to validate config", err)
 	}
-	itopo.Init("", proto.ServiceType_unset, itopo.Callbacks{})
 	topo, err := topology.FromJSONFile(cfg.General.Topology)
 	if err != nil {
 		return common.NewBasicError("unable to load topology", err)
 	}
-	if _, _, err := itopo.SetStatic(topo); err != nil {
+	itopo.Init(&itopo.Config{})
+	if err := itopo.Update(topo); err != nil {
 		return common.NewBasicError("unable to set initial static topology", err)
 	}
 	infraenv.InitInfraEnvironment(cfg.General.Topology)
diff --git a/python/topology/common.py b/python/topology/common.py
index 81b3dc819..abc1203cf 100644
--- a/python/topology/common.py
+++ b/python/topology/common.py
@@ -27,7 +27,6 @@ SCION_SERVICE_NAMES = (
     "ControlService",
     "BorderRouters",
     "ColibriService",
-    "DiscoveryService",
 )
 
 BR_CONFIG_NAME = 'br.toml'


Test patch:
diff --git a/acceptance/topo_sd_reload/testdata/topology_reload.json b/acceptance/topo_sd_reload/testdata/topology_reload.json
index 0ef791989..611fa00c5 100644
--- a/acceptance/topo_sd_reload/testdata/topology_reload.json
+++ b/acceptance/topo_sd_reload/testdata/topology_reload.json
@@ -73,7 +73,6 @@
     }
   },
   "ColibriService": {},
-  "DiscoveryService": {},
   "Attributes": [
     "authoritative",
     "core",
diff --git a/go/lib/infra/modules/itopo/itopo_test.go b/go/lib/infra/modules/itopo/itopo_test.go
index ee887503f..f40a8f09f 100644
--- a/go/lib/infra/modules/itopo/itopo_test.go
+++ b/go/lib/infra/modules/itopo/itopo_test.go
@@ -38,17 +38,19 @@ func setStaticTestFunc(s *state) updateTestFunc {
 
 func TestInit(t *testing.T) {
 	Convey("Initializing itopo twice should panic", t, func() {
-		SoMsg("first", func() { Init("", 0, Callbacks{}) }, ShouldNotPanic)
-		SoMsg("second", func() { Init("", 0, Callbacks{}) }, ShouldPanic)
+		SoMsg("first", func() { Init(&Config{}) }, ShouldNotPanic)
+		SoMsg("second", func() { Init(&Config{}) }, ShouldPanic)
 	})
 }
 
-func TestStateSetStatic(t *testing.T) {
+func TestStateUpdate(t *testing.T) {
 	mctrl := gomock.NewController(&xtest.PanickingReporter{T: t})
 	defer mctrl.Finish()
 	Convey("When state is initialized with no specific element", t, func() {
 		clbks := newMockClbks(mctrl)
-		s := newState("", proto.ServiceType_unset, clbks.Clbks())
+		s := newState(&Config{
+			Callbacks: clbks.Clbks(),
+		})
 		s.topo.static = loadTopo(fn, t)
 		topo := loadTopo(fn, t)
 		Convey("Calling with modified topo should succeed", func() {
@@ -57,7 +59,7 @@ func TestStateSetStatic(t *testing.T) {
 			ifinfo := topo.IFInfoMap[1]
 			ifinfo.MTU = 42
 			topo.IFInfoMap[1] = ifinfo
-			clbks.update.EXPECT().Call().Do(wg.Done)
+			clbks.onUpdate.EXPECT().Call().Do(wg.Done)
 			_, updated, err := s.setStatic(topo)
 			SoMsg("err", err, ShouldBeNil)
 			SoMsg("updated", updated, ShouldBeTrue)
@@ -70,7 +72,12 @@ func TestStateSetStatic(t *testing.T) {
 	Convey("When itopo is initialized with a service element", t, func() {
 		clbks := newMockClbks(mctrl)
 		id := "cs1-ff00:0:311-1"
-		s := newState(id, proto.ServiceType_cs, clbks.Clbks())
+		config := &Config{
+			ID:        id,
+			Svc:       proto.ServiceType_cs,
+			Callbacks: clbks.Clbks(),
+		}
+		s := newState(config)
 		s.topo.static = loadTopo(fn, t)
 		topo := loadTopo(fn, t)
 		Convey("Modification without touching the element's entry should be allowed", func() {
@@ -84,7 +91,7 @@ func TestStateSetStatic(t *testing.T) {
 			cs := topo.CS["cs1-ff00:0:311-2"]
 			cs.UnderlayAddress = &net.UDPAddr{Port: 42}
 			topo.CS["cs1-ff00:0:311-2"] = cs
-			clbks.update.EXPECT().Call().Do(wg.Done)
+			clbks.onUpdate.EXPECT().Call().Do(wg.Done)
 			_, updated, err := s.setStatic(topo)
 			SoMsg("err", err, ShouldBeNil)
 			SoMsg("updated", updated, ShouldBeTrue)
@@ -106,7 +113,13 @@ func TestStateSetStatic(t *testing.T) {
 	Convey("When itopo is initialized with a border router", t, func() {
 		clbks := newMockClbks(mctrl)
 		id := "br1-ff00:0:311-1"
-		s := newState(id, proto.ServiceType_br, clbks.Clbks())
+		s := newState(
+			&Config{
+				ID:        id,
+				Svc:       proto.ServiceType_br,
+				Callbacks: clbks.Clbks(),
+			},
+		)
 		s.topo.static = loadTopo(fn, t)
 		topo := loadTopo(fn, t)
 		Convey("Modification without touching the br's entry should be allowed", func() {
@@ -121,7 +134,7 @@ func TestStateSetStatic(t *testing.T) {
 			cs.UnderlayAddress = &net.UDPAddr{Port: 42}
 			topo.CS["cs1-ff00:0:311-2"] = cs
 			Convey("If semi-mutation is allowed", func() {
-				clbks.update.EXPECT().Call().Do(wg.Done)
+				clbks.onUpdate.EXPECT().Call().Do(wg.Done)
 				_, updated, err := s.setStatic(topo)
 				SoMsg("err", err, ShouldBeNil)
 				SoMsg("updated", updated, ShouldBeTrue)
@@ -129,7 +142,7 @@ func TestStateSetStatic(t *testing.T) {
 				wg.WaitWithTimeout(time.Second)
 			})
 			Convey("If semi-mutation is not allowed", func() {
-				clbks.update.EXPECT().Call().Do(wg.Done)
+				clbks.onUpdate.EXPECT().Call().Do(wg.Done)
 				_, updated, err := s.setStatic(topo)
 				SoMsg("err", err, ShouldBeNil)
 				SoMsg("updated", updated, ShouldBeTrue)
@@ -151,7 +164,7 @@ func TestStateSetStatic(t *testing.T) {
 
 			var wg xtest.Waiter
 			wg.Add(2)
-			clbks.update.EXPECT().Call().Do(wg.Done)
+			clbks.onUpdate.EXPECT().Call().Do(wg.Done)
 			newTopo, updated, err := s.setStatic(topo)
 			SoMsg("err", err, ShouldBeNil)
 			SoMsg("updated", updated, ShouldBeTrue)
@@ -194,7 +207,7 @@ func testNoModified(update updateTestFunc, prevTopo *topology.RWTopology,
 			topo.TTL += 2 * time.Second
 			if updateCalled {
 				wg.Add(1)
-				clbks.update.EXPECT().Call().Do(wg.Done)
+				clbks.onUpdate.EXPECT().Call().Do(wg.Done)
 			}
 			_, updated, err := update(topo)
 			SoMsg("err", err, ShouldBeNil)
@@ -205,17 +218,17 @@ func testNoModified(update updateTestFunc, prevTopo *topology.RWTopology,
 }
 
 type mockClbks struct {
-	update *mock_xtest.MockCallback
+	onUpdate *mock_xtest.MockCallback
 }
 
 func newMockClbks(mctrl *gomock.Controller) *mockClbks {
 	return &mockClbks{
-		update: mock_xtest.NewMockCallback(mctrl),
+		onUpdate: mock_xtest.NewMockCallback(mctrl),
 	}
 }
 
 func (clbks *mockClbks) Clbks() Callbacks {
 	return Callbacks{
-		UpdateStatic: clbks.update.Call,
+		OnUpdate: clbks.onUpdate.Call,
 	}
 }
diff --git a/go/lib/topology/testdata/basic.json b/go/lib/topology/testdata/basic.json
index 3e02687c9..374bbd097 100644
--- a/go/lib/topology/testdata/basic.json
+++ b/go/lib/topology/testdata/basic.json
@@ -179,27 +179,5 @@
                 }
             }
         }
-    },
-    "DiscoveryService": {
-        "ds1-ff00:0:311-1": {
-            "Addrs": {
-                "IPv4": {
-                    "Public": {
-                        "Addr": "127.0.0.99",
-                        "L4Port": 53535
-                    }
-                }
-            }
-        },
-        "ds1-ff00:0:311-2": {
-            "Addrs": {
-                "IPv6": {
-                    "Public": {
-                        "Addr": "2001:db8:f00:b43::99",
-                        "L4Port": 53535
-                    }
-                }
-            }
-        }
     }
 }