diff --git a/cmd/webhook/run.go b/cmd/webhook/run.go index 0696d38..b9aa09e 100644 --- a/cmd/webhook/run.go +++ b/cmd/webhook/run.go @@ -75,6 +75,12 @@ func run(ctx context.Context, cfg *rest.Config, options *config.Options) error { return err } + if err := webhookServer.RegisterMutators( + ippool.NewMutator(), + ); err != nil { + return err + } + if err := webhookServer.Start(); err != nil { return err } diff --git a/pkg/agent/ippool/ippool.go b/pkg/agent/ippool/ippool.go index ef7a227..67efe27 100644 --- a/pkg/agent/ippool/ippool.go +++ b/pkg/agent/ippool/ippool.go @@ -17,7 +17,7 @@ func (c *Controller) Update(ipPool *networkv1.IPPool) error { return nil } allocated := ipPool.Status.IPv4.Allocated - filterExcluded(allocated) + filterExcludedAndReserved(allocated) return c.updatePoolCacheAndLeaseStore(allocated, ipPool.Spec.IPv4Config) } @@ -62,9 +62,9 @@ func (c *Controller) updatePoolCacheAndLeaseStore(latest map[string]string, ipv4 return nil } -func filterExcluded(allocated map[string]string) { +func filterExcludedAndReserved(allocated map[string]string) { for ip, mac := range allocated { - if mac == util.ExcludedMark { + if mac == util.ExcludedMark || mac == util.ReservedMark { delete(allocated, ip) } } diff --git a/pkg/controller/ippool/controller.go b/pkg/controller/ippool/controller.go index d1dc9a5..fb231b0 100644 --- a/pkg/controller/ippool/controller.go +++ b/pkg/controller/ippool/controller.go @@ -217,6 +217,12 @@ func (h *Handler) OnChange(key string, ipPool *networkv1.IPPool) (*networkv1.IPP if allocated == nil { allocated = make(map[string]string) } + if util.IsIPInBetweenOf(ipPool.Spec.IPv4Config.ServerIP, ipPool.Spec.IPv4Config.Pool.Start, ipPool.Spec.IPv4Config.Pool.End) { + allocated[ipPool.Spec.IPv4Config.ServerIP] = util.ReservedMark + } + if util.IsIPInBetweenOf(ipPool.Spec.IPv4Config.Router, ipPool.Spec.IPv4Config.Pool.Start, ipPool.Spec.IPv4Config.Pool.End) { + allocated[ipPool.Spec.IPv4Config.Router] = util.ReservedMark + } for _, eIP := range ipPool.Spec.IPv4Config.Pool.Exclude { allocated[eIP] = util.ExcludedMark } @@ -364,6 +370,18 @@ func (h *Handler) BuildCache(ipPool *networkv1.IPPool, status networkv1.IPPoolSt return status, err } + // Revoke server IP address in IPAM + if err := h.ipAllocator.RevokeIP(ipPool.Spec.NetworkName, ipPool.Spec.IPv4Config.ServerIP); err != nil { + return status, err + } + logrus.Debugf("(ippool.BuildCache) server ip %s was revoked in ipam %s", ipPool.Spec.IPv4Config.ServerIP, ipPool.Spec.NetworkName) + + // Revoke router IP address in IPAM + if err := h.ipAllocator.RevokeIP(ipPool.Spec.NetworkName, ipPool.Spec.IPv4Config.Router); err != nil { + return status, err + } + logrus.Debugf("(ippool.BuildCache) router ip %s was revoked in ipam %s", ipPool.Spec.IPv4Config.Router, ipPool.Spec.NetworkName) + // Revoke excluded IP addresses in IPAM for _, eIP := range ipPool.Spec.IPv4Config.Pool.Exclude { if err := h.ipAllocator.RevokeIP(ipPool.Spec.NetworkName, eIP); err != nil { diff --git a/pkg/controller/ippool/controller_test.go b/pkg/controller/ippool/controller_test.go index aed77c9..4dff06b 100644 --- a/pkg/controller/ippool/controller_test.go +++ b/pkg/controller/ippool/controller_test.go @@ -31,10 +31,13 @@ const ( testPodName = testNADNamespace + "-" + testNADName + "-agent" testUID = "3a955369-9eaa-43db-94f3-9153289d7dc2" testClusterNetwork = "provider" - testServerIP = "192.168.0.2" + testServerIP1 = "192.168.0.2" + testServerIP2 = "192.168.0.110" testNetworkName = testNADNamespace + "/" + testNADName testNetworkNameLong = testNADNamespace + "/" + testNADNameLong testCIDR = "192.168.0.0/24" + testRouter1 = "192.168.0.1" + testRouter2 = "192.168.0.120" testStartIP = "192.168.0.101" testEndIP = "192.168.0.200" testServiceAccountName = "vdca" @@ -125,14 +128,14 @@ func TestHandler_OnChange(t *testing.T) { IPSubnet(testNetworkName, testCIDR, testStartIP, testEndIP). Build() givenIPPool := newTestIPPoolBuilder(). - ServerIP(testServerIP). + ServerIP(testServerIP1). CIDR(testCIDR). PoolRange(testStartIP, testEndIP). NetworkName(testNetworkName). CacheReadyCondition(corev1.ConditionTrue, "", "").Build() expectedIPPool := newTestIPPoolBuilder(). - ServerIP(testServerIP). + ServerIP(testServerIP1). CIDR(testCIDR). PoolRange(testStartIP, testEndIP). NetworkName(testNetworkName). @@ -268,7 +271,7 @@ func TestHandler_OnChange(t *testing.T) { func TestHandler_DeployAgent(t *testing.T) { t.Run("ippool created", func(t *testing.T) { givenIPPool := newTestIPPoolBuilder(). - ServerIP(testServerIP). + ServerIP(testServerIP1). CIDR(testCIDR). NetworkName(testNetworkName).Build() givenNAD := newTestNetworkAttachmentDefinitionBuilder(). @@ -278,7 +281,7 @@ func TestHandler_DeployAgent(t *testing.T) { AgentPodRef(testPodNamespace, testPodName, testImage, "").Build() expectedPod, _ := prepareAgentPod( NewIPPoolBuilder(testIPPoolNamespace, testIPPoolName). - ServerIP(testServerIP). + ServerIP(testServerIP1). CIDR(testCIDR). NetworkName(testNetworkName).Build(), false, @@ -367,7 +370,7 @@ func TestHandler_DeployAgent(t *testing.T) { t.Run("agent pod already exists", func(t *testing.T) { givenIPPool := newTestIPPoolBuilder(). - ServerIP(testServerIP). + ServerIP(testServerIP1). CIDR(testCIDR). NetworkName(testNetworkName). AgentPodRef(testPodNamespace, testPodName, testImage, "").Build() @@ -375,7 +378,7 @@ func TestHandler_DeployAgent(t *testing.T) { Label(clusterNetworkLabelKey, testClusterNetwork).Build() givenPod, _ := prepareAgentPod( NewIPPoolBuilder(testIPPoolNamespace, testIPPoolName). - ServerIP(testServerIP). + ServerIP(testServerIP1). CIDR(testCIDR). NetworkName(testNetworkName).Build(), false, @@ -392,7 +395,7 @@ func TestHandler_DeployAgent(t *testing.T) { AgentPodRef(testPodNamespace, testPodName, testImage, "").Build() expectedPod, _ := prepareAgentPod( NewIPPoolBuilder(testIPPoolNamespace, testIPPoolName). - ServerIP(testServerIP). + ServerIP(testServerIP1). CIDR(testCIDR). NetworkName(testNetworkName).Build(), false, @@ -442,7 +445,7 @@ func TestHandler_DeployAgent(t *testing.T) { t.Run("very long name ippool created", func(t *testing.T) { givenIPPool := NewIPPoolBuilder(testIPPoolNamespace, testIPPoolNameLong). - ServerIP(testServerIP). + ServerIP(testServerIP1). CIDR(testCIDR). NetworkName(testNetworkNameLong).Build() givenNAD := NewNetworkAttachmentDefinitionBuilder(testNADNamespace, testNADNameLong). @@ -452,7 +455,7 @@ func TestHandler_DeployAgent(t *testing.T) { AgentPodRef(testPodNamespace, testPodNameLong, testImage, "").Build() expectedPod, _ := prepareAgentPod( NewIPPoolBuilder(testIPPoolNamespace, testIPPoolNameLong). - ServerIP(testServerIP). + ServerIP(testServerIP1). CIDR(testCIDR). NetworkName(testNetworkNameLong).Build(), false, @@ -500,7 +503,7 @@ func TestHandler_DeployAgent(t *testing.T) { t.Run("agent pod upgrade (from main to dev)", func(t *testing.T) { givenIPPool := newTestIPPoolBuilder(). - ServerIP(testServerIP). + ServerIP(testServerIP1). CIDR(testCIDR). NetworkName(testNetworkName). AgentPodRef(testPodNamespace, testPodName, testImage, "").Build() @@ -615,7 +618,7 @@ func TestHandler_DeployAgent(t *testing.T) { t.Run("existing agent pod uid mismatch", func(t *testing.T) { givenIPPool := newTestIPPoolBuilder(). - ServerIP(testServerIP). + ServerIP(testServerIP1). CIDR(testCIDR). NetworkName(testNetworkName). AgentPodRef(testPodNamespace, testPodName, testImage, testUID).Build() @@ -623,7 +626,7 @@ func TestHandler_DeployAgent(t *testing.T) { Label(clusterNetworkLabelKey, testClusterNetwork).Build() givenPod, _ := prepareAgentPod( NewIPPoolBuilder(testIPPoolNamespace, testIPPoolName). - ServerIP(testServerIP). + ServerIP(testServerIP1). CIDR(testCIDR). NetworkName(testNetworkName).Build(), false, diff --git a/pkg/util/common.go b/pkg/util/common.go index 22ce0b5..9c3db89 100644 --- a/pkg/util/common.go +++ b/pkg/util/common.go @@ -8,6 +8,7 @@ import ( const ( ExcludedMark = "EXCLUDED" + ReservedMark = "RESERVED" AgentSuffixName = "agent" ) diff --git a/pkg/util/network.go b/pkg/util/network.go index 8115fee..69614fe 100644 --- a/pkg/util/network.go +++ b/pkg/util/network.go @@ -18,7 +18,7 @@ type PoolInfo struct { RouterIPAddr netip.Addr } -func loadCIDR(cidr string) (ipNet *net.IPNet, networkIPAddr netip.Addr, broadcastIPAddr netip.Addr, err error) { +func LoadCIDR(cidr string) (ipNet *net.IPNet, networkIPAddr netip.Addr, broadcastIPAddr netip.Addr, err error) { _, ipNet, err = net.ParseCIDR(cidr) if err != nil { return @@ -45,7 +45,7 @@ func loadCIDR(cidr string) (ipNet *net.IPNet, networkIPAddr netip.Addr, broadcas } func LoadPool(ipPool *networkv1.IPPool) (pi PoolInfo, err error) { - pi.IPNet, pi.NetworkIPAddr, pi.BroadcastIPAddr, err = loadCIDR(ipPool.Spec.IPv4Config.CIDR) + pi.IPNet, pi.NetworkIPAddr, pi.BroadcastIPAddr, err = LoadCIDR(ipPool.Spec.IPv4Config.CIDR) if err != nil { return } @@ -91,3 +91,29 @@ func LoadAllocated(allocated map[string]string) (ipAddrList []netip.Addr) { } return } + +func IsIPAddrInList(ipAddr netip.Addr, ipAddrList []netip.Addr) bool { + for i := range ipAddrList { + if ipAddr == ipAddrList[i] { + return true + } + } + return false +} + +func IsIPInBetweenOf(ip, ip1, ip2 string) bool { + ipAddr, err := netip.ParseAddr(ip) + if err != nil { + return false + } + ip1Addr, err := netip.ParseAddr(ip1) + if err != nil { + return false + } + ip2Addr, err := netip.ParseAddr(ip2) + if err != nil { + return false + } + + return ipAddr.Compare(ip1Addr) >= 0 && ipAddr.Compare(ip2Addr) <= 0 +} diff --git a/pkg/webhook/ippool/mutator.go b/pkg/webhook/ippool/mutator.go new file mode 100644 index 0000000..da6eaee --- /dev/null +++ b/pkg/webhook/ippool/mutator.go @@ -0,0 +1,187 @@ +package ippool + +import ( + "fmt" + "net/netip" + "reflect" + + "github.com/harvester/webhook/pkg/server/admission" + "github.com/sirupsen/logrus" + admissionregv1 "k8s.io/api/admissionregistration/v1" + "k8s.io/apimachinery/pkg/runtime" + + networkv1 "github.com/harvester/vm-dhcp-controller/pkg/apis/network.harvesterhci.io/v1alpha1" + "github.com/harvester/vm-dhcp-controller/pkg/util" + "github.com/harvester/vm-dhcp-controller/pkg/webhook" +) + +const ( + Start EndpointType = "start" + End EndpointType = "end" +) + +type EndpointType string + +type Mutator struct { + admission.DefaultMutator +} + +func NewMutator() *Mutator { + return &Mutator{} +} + +func (m *Mutator) Create(_ *admission.Request, newObj runtime.Object) (admission.Patch, error) { + ipPool := newObj.(*networkv1.IPPool) + + serverIP, err := ensureServerIP( + ipPool.Spec.IPv4Config.ServerIP, + ipPool.Spec.IPv4Config.CIDR, + ipPool.Spec.IPv4Config.Router, + ipPool.Spec.IPv4Config.Pool.Exclude, + ) + if err != nil { + return nil, fmt.Errorf(webhook.CreateErr, "IPPool", ipPool.Namespace, ipPool.Name, err) + } + + pool, err := ensurePoolRange( + ipPool.Spec.IPv4Config.Pool, + ipPool.Spec.IPv4Config.CIDR, + ) + if err != nil { + return nil, fmt.Errorf(webhook.CreateErr, "IPPool", ipPool.Namespace, ipPool.Name, err) + } + + var patch admission.Patch + if pool != nil { + patch = append(patch, admission.Patch{ + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/pool", + Value: *pool, + }, + }...) + } + if serverIP != nil { + patch = append(patch, admission.Patch{ + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/serverIP", + Value: *serverIP, + }, + }...) + } + + return patch, nil +} + +func (m *Mutator) Resource() admission.Resource { + return admission.Resource{ + Names: []string{"ippools"}, + Scope: admissionregv1.NamespacedScope, + APIGroup: networkv1.SchemeGroupVersion.Group, + APIVersion: networkv1.SchemeGroupVersion.Version, + ObjectType: &networkv1.IPPool{}, + OperationTypes: []admissionregv1.OperationType{ + admissionregv1.Create, + }, + } +} + +func ensureServerIP(server string, cidr, router string, excludes []string) (*string, error) { + var maskedIPAddrList []netip.Addr + + ipNet, networkIPAddr, broadcastIPAddr, err := util.LoadCIDR(cidr) + if err != nil { + return nil, err + } + + routerIPAddr, err := netip.ParseAddr(router) + if err == nil { + maskedIPAddrList = append(maskedIPAddrList, routerIPAddr) + } + + serverIPAddr, err := netip.ParseAddr(server) + if err != nil { + serverIPAddr = netip.Addr{} + } + + for _, exclude := range excludes { + var excludeIPAddr netip.Addr + excludeIPAddr, err = netip.ParseAddr(exclude) + if err != nil { + return nil, err + } + maskedIPAddrList = append(maskedIPAddrList, excludeIPAddr) + } + + if !serverIPAddr.IsValid() { + for serverIPAddr = networkIPAddr.Next(); ipNet.Contains(serverIPAddr.AsSlice()); serverIPAddr = serverIPAddr.Next() { + if util.IsIPAddrInList(serverIPAddr, maskedIPAddrList) { + continue + } + + if serverIPAddr.As4() == broadcastIPAddr.As4() { + break + } + + serverIPStr := serverIPAddr.String() + logrus.Infof("auto assign serverIP=%s", serverIPStr) + + return &serverIPStr, nil + } + + return nil, fmt.Errorf("fail to assign ip for dhcp server") + } + + return nil, nil +} + +func ensurePoolRange(pool networkv1.Pool, cidr string) (*networkv1.Pool, error) { + startIPAddr, err := netip.ParseAddr(pool.Start) + if err != nil { + startIPAddr = netip.Addr{} + } + + endIPAddr, err := netip.ParseAddr(pool.End) + if err != nil { + endIPAddr = netip.Addr{} + } + + ipNet, networkIPAddr, broadcastIPAddr, err := util.LoadCIDR(cidr) + if err != nil { + return nil, err + } + + newPool := pool + + if !startIPAddr.IsValid() { + startIPAddr = networkIPAddr.Next() + + if !ipNet.Contains(startIPAddr.AsSlice()) { + logrus.Warningf("start ip is out of subnet") + } + + newPool.Start = startIPAddr.String() + } + + if !endIPAddr.IsValid() { + endIPAddr = broadcastIPAddr.Prev() + + if !ipNet.Contains(endIPAddr.AsSlice()) { + logrus.Warningf("end ip is out of subnet") + } + + newPool.End = endIPAddr.String() + } + + if startIPAddr.Compare(endIPAddr) > 0 { + return nil, fmt.Errorf("invalid pool range") + } + + if !reflect.DeepEqual(newPool, pool) { + logrus.Infof("auto assign startIP=%s, endIP=%s", startIPAddr.String(), endIPAddr.String()) + return &newPool, nil + } + + return nil, nil +} diff --git a/pkg/webhook/ippool/mutator_test.go b/pkg/webhook/ippool/mutator_test.go new file mode 100644 index 0000000..33d3804 --- /dev/null +++ b/pkg/webhook/ippool/mutator_test.go @@ -0,0 +1,254 @@ +package ippool + +import ( + "fmt" + "testing" + + networkv1 "github.com/harvester/vm-dhcp-controller/pkg/apis/network.harvesterhci.io/v1alpha1" + "github.com/harvester/webhook/pkg/server/admission" + "github.com/stretchr/testify/assert" +) + +func TestMutator_Create(t *testing.T) { + type input struct { + name string + ipPool *networkv1.IPPool + } + type output struct { + patch admission.Patch + err error + } + testCases := []struct { + given input + expected output + }{ + { + given: input{ + name: "no router ippool with server, start, and end ips undefined", + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24").Build(), + }, + expected: output{ + patch: admission.Patch{ + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/pool", + Value: networkv1.Pool{ + Start: "192.168.0.1", + End: "192.168.0.254", + }, + }, + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/serverIP", + Value: "192.168.0.1", + }, + }, + }, + }, + { + given: input{ + name: "ippool with server, start, and end ips undefined", + ipPool: newTestIPPoolBuilder(). + CIDR("172.19.64.128/29"). + Router("172.19.64.129").Build(), + }, + expected: output{ + patch: admission.Patch{ + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/pool", + Value: networkv1.Pool{ + Start: "172.19.64.129", + End: "172.19.64.134", + }, + }, + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/serverIP", + Value: "172.19.64.130", + }, + }, + }, + }, + { + given: input{ + name: "ippool with server, start, and end ips all defined", + ipPool: newTestIPPoolBuilder(). + CIDR("172.19.64.128/29"). + ServerIP("172.19.64.130"). + Router("172.19.64.129"). + PoolRange("172.19.64.131", "172.19.64.133").Build(), + }, + expected: output{}, + }, + { + given: input{ + name: "/30 ippool with router (zero allocatable ip left effectively)", + ipPool: newTestIPPoolBuilder(). + CIDR("172.19.64.128/30"). + Router("172.19.64.129").Build(), + }, + expected: output{ + patch: admission.Patch{ + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/pool", + Value: networkv1.Pool{ + Start: "172.19.64.129", + End: "172.19.64.130", + }, + }, + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/serverIP", + Value: "172.19.64.130", + }, + }, + }, + }, + { + given: input{ + name: "/30 ippool without router (one allocatable ip left effectively)", + ipPool: newTestIPPoolBuilder(). + CIDR("172.19.64.128/30").Build(), + }, + expected: output{ + patch: admission.Patch{ + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/pool", + Value: networkv1.Pool{ + Start: "172.19.64.129", + End: "172.19.64.130", + }, + }, + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/serverIP", + Value: "172.19.64.129", + }, + }, + }, + }, + { + given: input{ + name: "/30 ippool without router but with start ip defined", + ipPool: newTestIPPoolBuilder(). + CIDR("172.19.64.128/30"). + PoolRange("172.19.64.130", "").Build(), + }, + expected: output{ + patch: admission.Patch{ + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/pool", + Value: networkv1.Pool{ + Start: "172.19.64.130", + End: "172.19.64.130", + }, + }, + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/serverIP", + Value: "172.19.64.129", + }, + }, + }, + }, + { + given: input{ + name: "no router ippool with excluded ips", + ipPool: newTestIPPoolBuilder(). + CIDR("172.19.64.128/29"). + Exclude("172.19.64.129", "172.19.64.131").Build(), + }, + expected: output{ + patch: admission.Patch{ + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/pool", + Value: networkv1.Pool{ + Start: "172.19.64.129", + End: "172.19.64.134", + Exclude: []string{ + "172.19.64.129", + "172.19.64.131", + }, + }, + }, + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/serverIP", + Value: "172.19.64.130", + }, + }, + }, + }, + { + given: input{ + name: "the only available ip left is the broadcast ip", + ipPool: newTestIPPoolBuilder(). + CIDR("172.19.64.128/29"). + Router("172.19.64.130"). + Exclude("172.19.64.129", "172.19.64.131", "172.19.64.132", "172.19.64.133", "172.19.64.134").Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because fail to assign ip for dhcp server", testIPPoolNamespace, testIPPoolName), + }, + }, + { + given: input{ + name: "/32 ippool", + ipPool: newTestIPPoolBuilder(). + CIDR("172.19.64.128/32").Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because fail to assign ip for dhcp server", testIPPoolNamespace, testIPPoolName), + }, + }, + { + given: input{ + name: "/31 ippool", + ipPool: newTestIPPoolBuilder(). + CIDR("172.19.64.128/31").Build(), + }, + expected: output{ + err: fmt.Errorf("could not create IPPool %s/%s because fail to assign ip for dhcp server", testIPPoolNamespace, testIPPoolName), + }, + }, + { + given: input{ + name: "server ip and router are in the middle of pool range", + ipPool: newTestIPPoolBuilder(). + CIDR("192.168.0.0/24"). + ServerIP("192.168.0.50"). + Router("192.168.0.100").Build(), + }, + expected: output{ + patch: admission.Patch{ + { + Op: admission.PatchOpReplace, + Path: "/spec/ipv4Config/pool", + Value: networkv1.Pool{ + Start: "192.168.0.1", + End: "192.168.0.254", + }, + }, + }, + }, + }, + } + + for _, tc := range testCases { + mutator := NewMutator() + + patch, err := mutator.Create(&admission.Request{}, tc.given.ipPool) + if tc.expected.err != nil { + assert.Equal(t, tc.expected.err.Error(), err.Error(), tc.given.name) + } else { + assert.Nil(t, err, tc.given.name) + } + assert.Equal(t, tc.expected.patch, patch, tc.given.name) + } +}