dhcp: release & inform (#1251)

* add dhcp release support * add tests * fix duplicate log fields * fix
BeryJu · Nov 26, 2024 · 294cdd7 · 294cdd7
1 parent fab94cf
commit 294cdd7
Show file tree

Hide file tree

Showing 10 changed files with 129 additions and 11 deletions.
diff --git a/hack/pcap_to_bytes/main.go b/hack/pcap_to_bytes/main.go
@@ -22,7 +22,12 @@ func main() {
 	}
 	packetSource := gopacket.NewPacketSource(p, p.LinkType())
 	for packet := range packetSource.Packets() {
-		dhcpLayer := packet.Layers()[3]
+		fmt.Printf("Checking packet %s\n", packet.String())
+		layers := packet.Layers()
+		if len(layers) < 3 {
+			continue
+		}
+		dhcpLayer := layers[3]
 		stringRep := fmt.Sprintf("%+v", dhcpLayer.LayerContents())
 		fmt.Println(strings.ReplaceAll(stringRep, " ", ", "))
 	}

diff --git a/pkg/roles/dhcp/dhcp_handler4.go b/pkg/roles/dhcp/dhcp_handler4.go
@@ -194,8 +194,10 @@ func (h *handler4) HandleRequest(r *Request4) *dhcpv4.DHCPv4 {
 		handler = h.role.HandleDHCPRequest4
 	case dhcpv4.MessageTypeDecline:
 		handler = h.role.HandleDHCPDecline4
+	case dhcpv4.MessageTypeRelease:
+		handler = h.role.HandleDHCPRelease4
 	default:
-		r.log.Info("Unsupported message type", zap.String("msg", mt.String()))
+		r.log.Info("Unsupported message type", zap.String("dhcpMsg", mt.String()))
 		return nil
 	}
 

diff --git a/pkg/roles/dhcp/dhcp_handler4_decline.go b/pkg/roles/dhcp/dhcp_handler4_decline.go
@@ -14,7 +14,7 @@ func (r *Role) HandleDHCPDecline4(req *Request4) *dhcpv4.DHCPv4 {
 			return nil
 		}
 		req.log.Debug("found scope for new lease", zap.String("scope", scope.Name))
-		match = scope.createLeaseFor(req)
+		match = scope.leaseFor(req)
 		if match == nil {
 			return nil
 		}
@@ -27,7 +27,7 @@ func (r *Role) HandleDHCPDecline4(req *Request4) *dhcpv4.DHCPv4 {
 	// since there's no further requests to confirm this lease, save it directly with the TTL of the scope
 	err := match.Put(req.Context, match.scope.TTL)
 	if err != nil {
-		r.log.Warn("failed to put lease", zap.Error(err))
+		req.log.Warn("failed to put lease", zap.Error(err))
 	}
 
 	dhcpRequests.WithLabelValues(req.MessageType().String(), match.scope.Name).Inc()

diff --git a/pkg/roles/dhcp/dhcp_handler4_discover.go b/pkg/roles/dhcp/dhcp_handler4_discover.go
@@ -14,19 +14,19 @@ func (r *Role) HandleDHCPDiscover4(req *Request4) *dhcpv4.DHCPv4 {
 			return nil
 		}
 		req.log.Debug("found scope for new lease", zap.String("scope", scope.Name))
-		match = scope.createLeaseFor(req)
+		match = scope.leaseFor(req)
 		if match == nil {
 			return nil
 		}
 		err := match.Put(req.Context, int64(r.cfg.LeaseNegotiateTimeout))
 		if err != nil {
-			r.log.Warn("failed to update lease", zap.Error(err))
+			req.log.Warn("failed to update lease", zap.Error(err))
 		}
 	} else {
 		go func() {
 			err := match.Put(req.Context, match.scope.TTL)
 			if err != nil {
-				r.log.Warn("failed to update lease", zap.Error(err))
+				req.log.Warn("failed to update lease", zap.Error(err))
 			}
 		}()
 	}

diff --git a/pkg/roles/dhcp/dhcp_handler4_release.go b/pkg/roles/dhcp/dhcp_handler4_release.go
@@ -0,0 +1,21 @@
+package dhcp
+
+import (
+	"github.com/insomniacslk/dhcp/dhcpv4"
+	"go.uber.org/zap"
+)
+
+func (r *Role) HandleDHCPRelease4(req *Request4) *dhcpv4.DHCPv4 {
+	match := r.FindLease(req)
+	if match == nil || match.IsReservation() {
+		return nil
+	}
+	err := match.Delete(req.Context)
+	if err != nil {
+		req.log.Warn("failed to delete lease", zap.Error(err))
+	} else {
+		req.log.Info("deleted lease from release")
+	}
+	dhcpRequests.WithLabelValues(req.MessageType().String(), match.scope.Name).Inc()
+	return nil
+}
diff --git a/pkg/roles/dhcp/dhcp_handler4_release_test.go b/pkg/roles/dhcp/dhcp_handler4_release_test.go
@@ -0,0 +1,77 @@
+package dhcp_test
+
+import (
+	"testing"
+
+	"beryju.io/gravity/pkg/instance"
+	"beryju.io/gravity/pkg/roles/dhcp"
+	"beryju.io/gravity/pkg/roles/dhcp/types"
+	"beryju.io/gravity/pkg/tests"
+	"github.com/insomniacslk/dhcp/dhcpv4"
+	"github.com/stretchr/testify/assert"
+)
+
+var (
+	DHCPReleasePayload = []byte{0x1, 0x1, 0x6, 0x0, 0x27, 0xda, 0x41, 0x36, 0x0, 0x0, 0x0, 0x0, 0xa, 0x78, 0x14, 0x90, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x50, 0x56, 0xaf, 0x5c, 0x20, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x63, 0x82, 0x53, 0x63, 0x35, 0x1, 0x7, 0x36, 0x4, 0xa, 0x78, 0x14, 0xfa, 0xc, 0xa, 0x6c, 0x69, 0x6e, 0x75, 0x78, 0x2d, 0x74, 0x65, 0x73, 0x74, 0xff, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}
+)
+
+func TestDHCPRelease(t *testing.T) {
+	defer tests.Setup(t)()
+	rootInst := instance.New()
+	ctx := tests.Context()
+	inst := rootInst.ForRole("dhcp", ctx)
+	role := dhcp.New(inst)
+
+	tests.PanicIfError(inst.KV().Put(
+		ctx,
+		inst.KV().Key(
+			types.KeyRole,
+			types.KeyScopes,
+			"test",
+		).String(),
+		tests.MustJSON(dhcp.Scope{
+			SubnetCIDR: "10.120.20.0/24",
+			Default:    true,
+			TTL:        86400,
+			IPAM: map[string]string{
+				"type":        "internal",
+				"range_start": "10.120.20.100",
+				"range_end":   "10.120.20.250",
+			},
+		}),
+	))
+	tests.PanicIfError(inst.KV().Put(
+		ctx,
+		inst.KV().Key(
+			types.KeyRole,
+			types.KeyLeases,
+			"00:50:56:af:5c:20",
+		).String(),
+		tests.MustJSON(dhcp.Lease{
+			ScopeKey:   "test",
+			Identifier: "00:50:56:af:5c:20",
+			Address:    "10.120.20.144",
+			Hostname:   "linux-test.home.arpa",
+		}),
+	))
+	tests.PanicIfError(role.Start(ctx, []byte(tests.MustJSON(dhcp.RoleConfig{
+		Port: 1067,
+	}))))
+	defer role.Stop()
+
+	req, err := dhcpv4.FromBytes(DHCPReleasePayload)
+	assert.NoError(t, err)
+	req4 := role.NewRequest4(req)
+	res := role.Handler4(req4)
+	assert.Nil(t, res)
+
+	tests.AssertEtcd(
+		t,
+		inst.KV(),
+		inst.KV().Key(
+			types.KeyRole,
+			types.KeyLeases,
+			"00:50:56:af:5c:20",
+		),
+	)
+}
diff --git a/pkg/roles/dhcp/dhcp_handler4_request.go b/pkg/roles/dhcp/dhcp_handler4_request.go
@@ -14,15 +14,15 @@ func (r *Role) HandleDHCPRequest4(req *Request4) *dhcpv4.DHCPv4 {
 			return nil
 		}
 		req.log.Debug("found scope for new lease", zap.String("scope", scope.Name))
-		match = scope.createLeaseFor(req)
+		match = scope.leaseFor(req)
 		if match == nil {
 			return nil
 		}
 	}
 
 	err := match.Put(req.Context, match.scope.TTL)
 	if err != nil {
-		r.log.Warn("failed to put dhcp lease", zap.Error(err))
+		req.log.Warn("failed to put dhcp lease", zap.Error(err))
 	}
 
 	dhcpRequests.WithLabelValues(req.MessageType().String(), match.scope.Name).Inc()

diff --git a/pkg/roles/dhcp/leases.go b/pkg/roles/dhcp/leases.go
@@ -123,6 +123,19 @@ func (l *Lease) IsReservation() bool {
 	return l.Expiry == -1
 }
 
+func (l *Lease) Delete(ctx context.Context) error {
+	leaseKey := l.inst.KV().Key(
+		types.KeyRole,
+		types.KeyLeases,
+		l.Identifier,
+	)
+	_, err := l.inst.KV().Delete(
+		ctx,
+		leaseKey.String(),
+	)
+	return err
+}
+
 func (l *Lease) Put(ctx context.Context, expiry int64, opts ...clientv3.OpOption) error {
 	if expiry > 0 && !l.IsReservation() {
 		l.Expiry = time.Now().Add(time.Duration(expiry) * time.Second).Unix()

diff --git a/pkg/roles/dhcp/scopes.go b/pkg/roles/dhcp/scopes.go
@@ -152,7 +152,7 @@ func (s *Scope) match(peer net.IP) int {
 	return -1
 }
 
-func (s *Scope) createLeaseFor(req *Request4) *Lease {
+func (s *Scope) leaseFor(req *Request4) *Lease {
 	ident := s.role.DeviceIdentifier(req.DHCPv4)
 	lease := s.role.NewLease(ident)
 	lease.Hostname = req.HostName()

diff --git a/pkg/roles/dns/handler_forward_ip.go b/pkg/roles/dns/handler_forward_ip.go
@@ -157,7 +157,7 @@ func (ipf *IPForwarderHandler) Handle(w *utils.FakeDNSWriter, r *utils.DNSReques
 	fs := sentry.TransactionFromContext(r.Context()).StartChild("gravity.dns.handler.forward_ip.lookup")
 	resolver := ipf.pickResolver()
 	fs.SetTag("resolver", resolver)
-	ipf.log.Debug("sending message to resolve", zap.String("resolver", resolver), zap.String("msg", r.Msg.String()))
+	ipf.log.Debug("sending message to resolve", zap.String("resolver", resolver), zap.String("dnsMsg", r.Msg.String()))
 	m, rtt, err := ipf.c.ExchangeContext(r.Context(), r.Msg, resolver)
 	ipf.log.Debug("dns rtt", zap.Duration("rtt", rtt))
 	fs.Finish()