fix(tap): take into account other taps on same experiment when creati…

…ng new tap

fixes #142

src/go/api/scorch/break.go

@@ -8,14 +8,12 @@ import (
 
 	"phenix/api/scorch/scorchmd"
 	"phenix/app"
-	"phenix/types"
 	"phenix/util"
 	"phenix/util/mm"
 	"phenix/util/tap"
 	"phenix/web/scorch"
 
 	"github.com/mitchellh/mapstructure"
-	"inet.af/netaddr"
 )
 
 type BreakMetadata struct {
@@ -62,7 +60,7 @@ func (this Break) breakPoint(ctx context.Context, stage Action) error {
 	}
 
 	if md.Tap != nil {
-		pairs := this.discoverUsedPairs()
+		pairs := discoverUsedPairs()
 		md.Tap.Init(tap.Experiment(exp), tap.UsedPairs(pairs))
 
 		// backwards compatibility (doesn't support external access firewall rules)
@@ -75,7 +73,7 @@ func (this Break) breakPoint(ctx context.Context, stage Action) error {
 		// (dictated by max length of Linux interface names)
 		md.Tap.Name = fmt.Sprintf("%s-tapbrk", util.RandomString(8))
 
-		if err := md.Tap.Create(mm.Headnode()); err != nil {
+		if _, err := md.Tap.Create(mm.Headnode()); err != nil {
 			return fmt.Errorf("setting up tap for break: %w", err)
 		}
 
@@ -142,25 +140,3 @@ func (this Break) breakPoint(ctx context.Context, stage Action) error {
 
 	return ctx.Err()
 }
-
-func (Break) discoverUsedPairs() []netaddr.IPPrefix {
-	var pairs []netaddr.IPPrefix
-
-	running, err := types.RunningExperiments()
-	if err != nil {
-		return nil
-	}
-
-	for _, exp := range running {
-		var status scorchmd.ScorchStatus
-		if err := exp.Status.ParseAppStatus("scorch", &status); err == nil {
-			for _, tap := range status.Taps {
-				if pair, err := netaddr.ParseIPPrefix(tap.Subnet); err == nil {
-					pairs = append(pairs, pair)
-				}
-			}
-		}
-	}
-
-	return pairs
-}

src/go/api/scorch/tap.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"phenix/api/scorch/scorchmd"
+	"phenix/app"
 	"phenix/types"
 	"phenix/util"
 	"phenix/util/mm"
@@ -40,7 +41,7 @@ func (this Tap) Start(ctx context.Context) error {
 		return fmt.Errorf("decoding tap component metadata: %w", err)
 	}
 
-	pairs := this.discoverUsedPairs()
+	pairs := discoverUsedPairs()
 	t.Init(tap.Experiment(exp), tap.UsedPairs(pairs))
 
 	// backwards compatibility (doesn't support external access firewall rules)
@@ -53,7 +54,7 @@ func (this Tap) Start(ctx context.Context) error {
 	// (dictated by max length of Linux interface names)
 	t.Name = fmt.Sprintf("%s-tapcomp", util.RandomString(7))
 
-	if err := t.Create(mm.Headnode()); err != nil {
+	if _, err := t.Create(mm.Headnode()); err != nil {
 		return fmt.Errorf("setting up tap: %w", err)
 	}
 
@@ -94,7 +95,7 @@ func (Tap) Cleanup(context.Context) error {
 	return nil
 }
 
-func (Tap) discoverUsedPairs() []netaddr.IPPrefix {
+func discoverUsedPairs() []netaddr.IPPrefix {
 	var pairs []netaddr.IPPrefix
 
 	running, err := types.RunningExperiments()
@@ -103,9 +104,18 @@ func (Tap) discoverUsedPairs() []netaddr.IPPrefix {
 	}
 
 	for _, exp := range running {
-		var status scorchmd.ScorchStatus
-		if err := exp.Status.ParseAppStatus("scorch", &status); err == nil {
-			for _, tap := range status.Taps {
+		var scorch scorchmd.ScorchStatus
+		if err := exp.Status.ParseAppStatus("scorch", &scorch); err == nil {
+			for _, tap := range scorch.Taps {
+				if pair, err := netaddr.ParseIPPrefix(tap.Subnet); err == nil {
+					pairs = append(pairs, pair)
+				}
+			}
+		}
+
+		var tap app.TapAppStatus
+		if err := exp.Status.ParseAppStatus("tap", &tap); err == nil {
+			for _, tap := range tap.Taps {
 				if pair, err := netaddr.ParseIPPrefix(tap.Subnet); err == nil {
 					pairs = append(pairs, pair)
 				}

src/go/app/tap.go

@@ -74,8 +74,6 @@ func (this *Tap) PostStart(ctx context.Context, exp *types.Experiment) error {
 
 	status := TapAppStatus{Host: host}
 
-	// TODO: prepopulate `pairs` with taps from other experiments
-
 	for _, t := range amd.Taps {
 		if slices.Contains(vlans, t.VLAN) {
 			return fmt.Errorf("tap already created for VLAN %s", t.VLAN)
@@ -86,10 +84,17 @@ func (this *Tap) PostStart(ctx context.Context, exp *types.Experiment) error {
 		// Tap name is random, yet descriptive to the fact that it's a "tapapp" tap.
 		t.Name = fmt.Sprintf("%s-tapapp", util.RandomString(8))
 
-		if err := t.Create(host); err != nil {
+		pair, err := t.Create(host)
+		if err != nil {
 			return fmt.Errorf("creating host tap for VLAN %s: %w", t.VLAN, err)
 		}
 
+		if !pair.IsZero() {
+			// Include pair just created for this tap to list of used pairs in case
+			// more than one tap is being created for this experiment.
+			pairs = append(pairs, pair)
+		}
+
 		status.Taps = append(status.Taps, t)
 		vlans = append(vlans, t.VLAN)
 	}

src/go/util/tap/tap.go

@@ -19,33 +19,33 @@ func (this *Tap) Init(opts ...Option) {
 	}
 }
 
-func (this *Tap) Create(host string) error {
+func (this *Tap) Create(host string) (netaddr.IPPrefix, error) {
 	if err := this.create(host); err != nil {
 		// attempt to clean up any progress already made
 		this.delete(host)
-		return fmt.Errorf("creating host tap for VLAN %s: %w", this.VLAN, err)
+		return netaddr.IPPrefix{}, fmt.Errorf("creating host tap for VLAN %s: %w", this.VLAN, err)
 	}
 
 	if this.External.Enabled {
 		used := append(this.o.used, netaddr.MustParseIPPrefix(this.IP))
 
 		pair, err := util.UnusedSubnet(this.o.subnet, used)
 		if err != nil {
-			return fmt.Errorf("getting unused pair of IPs for VLAN %s host tap external connectivity: %w", this.VLAN, err)
+			return netaddr.IPPrefix{}, fmt.Errorf("getting unused pair of IPs for VLAN %s host tap external connectivity: %w", this.VLAN, err)
 		}
 
-		this.o.used = append(this.o.used, pair)
-
 		if err := this.connect(host, pair); err != nil {
 			// attempt to clean up progress we already made
 			this.disconnect(host)
 			this.delete(host)
 
-			return fmt.Errorf("connecting host tap for VLAN %s for external access: %w", this.VLAN, err)
+			return netaddr.IPPrefix{}, fmt.Errorf("connecting host tap for VLAN %s for external access: %w", this.VLAN, err)
 		}
+
+		return pair, nil
 	}
 
-	return nil
+	return netaddr.IPPrefix{}, nil
 }
 
 func (this Tap) Delete(host string) error {