From cc79cc7ba1ac2866e4036a68f912221e91c26235 Mon Sep 17 00:00:00 2001 From: Damyan Yordanov Date: Tue, 1 Oct 2024 14:54:14 +0200 Subject: [PATCH 1/5] Update the Readme to match the new strategy --- README.md | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index f216d21..1243785 100644 --- a/README.md +++ b/README.md @@ -72,13 +72,29 @@ As for in-band, a kubernetes namespace shall be passed as a parameter. Further, The Metal plugin acts as a connection link between DHCP and the IronCore metal stack. It creates an `EndPoint` object for each machine with leased IP address. Those endpoints are then consumed by the metal operator, who then creates the corresponding `Machine` objects. ### Configuration -Path to an inventory yaml shall be passed as a string. It represents a list of machines as follows: +Path to an inventory yaml shall be passed as a string. Currently, there are two different ways to provide an inventory list: either by specifying a MAC address filter or by providing the inventory list explicitly. If both a static list and a filter are specified in the `inventory.yaml`, the static list gets a precedence, so the filter will be ignored. + +Providing an explicit static inventory list in `inventory.yaml` goes as follows: ```yaml -- name: server-01 - macAddress: 00:1A:2B:3C:4D:5E -- name: server-02 - macAddress: 00:1A:2B:3C:4D:5F +hosts: + - name: server-01 + macAddress: 00:1A:2B:3C:4D:5E + - name: server-02 + macAddress: 00:1A:2B:3C:4D:5F ``` + +Providing a MAC address prefix filter list creates `Endpoint`s with a predefined prefix name. When the MAC address of an inventory does not match the prefix, the inventory will not be onboarded, so for now no "onboarding by default" occurs. Obviously a full MAC address is a valid prefix filter. +To get inventories with certain MACs onboarded, the following `inventory.yaml` shall be specified: +```yaml +namePrefix: server- # optional prefix, default: "compute-" +filter: + macPrefix: + - 00:1A:2B:3C:4D:5E + - 00:1A:2B:3C:4D:5F + - 00:AA:BB +``` +The inventories above will get auto-generated names like `server-aybz`. + ### Notes - supports both IPv4 and IPv6 - IPv6 relays are supported, IPv4 are not From 9a31ce1fa7974ea3f5ad66877ad94d654c34a589 Mon Sep 17 00:00:00 2001 From: Damyan Yordanov Date: Mon, 30 Sep 2024 17:14:56 +0200 Subject: [PATCH 2/5] Implement new onboarding strategy --- internal/api/config.go | 10 ++ plugins/metal/plugin.go | 176 ++++++++++++++++++++++++++------ plugins/metal/plugin_test.go | 191 ++++++++++++++++++++++++++++++++--- plugins/metal/suite_test.go | 64 ++++++++---- 4 files changed, 376 insertions(+), 65 deletions(-) diff --git a/internal/api/config.go b/internal/api/config.go index 1bda670..b05e9e0 100644 --- a/internal/api/config.go +++ b/internal/api/config.go @@ -7,3 +7,13 @@ type Inventory struct { Name string `yaml:"name"` MacAddress string `yaml:"macAddress"` } + +type Filter struct { + MacPrefix []string `yaml:"macPrefix"` +} + +type Config struct { + NamePrefix string `yaml:"namePrefix"` + Inventories []Inventory `yaml:"hosts"` + Filter Filter `yaml:"filter"` +} diff --git a/plugins/metal/plugin.go b/plugins/metal/plugin.go index f19e0ef..5ea2796 100644 --- a/plugins/metal/plugin.go +++ b/plugins/metal/plugin.go @@ -11,6 +11,8 @@ import ( "os" "strings" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/coredhcp/coredhcp/handler" @@ -38,7 +40,23 @@ var Plugin = plugins.Plugin{ // map MAC address to inventory name var inventoryMap map[string]string -// args[0] = path to configuration file +// default inventory name prefix +const defaultNamePrefix = "compute-" + +type OnBoardingStrategy int + +const ( + OnboardingFromStaticList = 1 + OnboardingFromMACPrefixFilter = 2 +) + +// flag for onboarding strategy: +// OnboardingFromStaticList: the name comes from a static inventory list and shall be taken "as is" +// +// OnboardingFromMACPrefixFilter: the name is only a prefix (default or custom), k8s will autogenerate it +var strategy OnBoardingStrategy + +// args[0] = path to inventory file func parseArgs(args ...string) (string, error) { if len(args) != 1 { return "", fmt.Errorf("exactly one argument must be passed to the metal plugin, got %d", len(args)) @@ -52,6 +70,9 @@ func setup6(args ...string) (handler.Handler6, error) { if err != nil { return nil, err } + if inventoryMap == nil { + return nil, nil + } return handler6, nil } @@ -62,22 +83,40 @@ func loadConfig(args ...string) (map[string]string, error) { return nil, fmt.Errorf("invalid configuration: %v", err) } - log.Infof("Reading metal config file %s", path) + log.Debugf("Reading metal config file %s", path) configData, err := os.ReadFile(path) if err != nil { return nil, fmt.Errorf("failed to read config file: %v", err) } - var config []api.Inventory + var config api.Config if err = yaml.Unmarshal(configData, &config); err != nil { return nil, fmt.Errorf("failed to parse config file: %v", err) } inventories := make(map[string]string) - for _, i := range config { - if i.MacAddress != "" && i.Name != "" { - inventories[strings.ToLower(i.MacAddress)] = i.Name + // static inventory list has precedence, always + if len(config.Inventories) > 0 { + strategy = OnboardingFromStaticList + log.Debug("Using static list onboarding") + for _, i := range config.Inventories { + if i.MacAddress != "" && i.Name != "" { + inventories[strings.ToLower(i.MacAddress)] = i.Name + } + } + } else if len(config.Filter.MacPrefix) > 0 { + strategy = OnboardingFromMACPrefixFilter + namePrefix := defaultNamePrefix + if config.NamePrefix != "" { + namePrefix = config.NamePrefix } + log.Debugf("Using MAC address prefix filter onboarding with name prefix '%s'", namePrefix) + for _, i := range config.Filter.MacPrefix { + inventories[strings.ToLower(i)] = namePrefix + } + } else { + log.Infof("No inventories loaded") + return nil, nil } log.Infof("Loaded metal config with %d inventories", len(inventories)) @@ -90,6 +129,9 @@ func setup4(args ...string) (handler.Handler4, error) { if err != nil { return nil, err } + if inventoryMap == nil { + return nil, nil + } return handler4, nil } @@ -109,7 +151,7 @@ func handler6(req, resp dhcpv6.DHCPv6) (dhcpv6.DHCPv6, bool) { return nil, true } - if err := applyEndpointForMACAddress(mac, ipamv1alpha1.CIPv6SubnetType); err != nil { + if err := ApplyEndpointForMACAddress(mac, ipamv1alpha1.CIPv6SubnetType); err != nil { log.Errorf("Could not apply endpoint for mac %s: %s", mac.String(), err) return resp, false } @@ -123,7 +165,7 @@ func handler4(req, resp *dhcpv4.DHCPv4) (*dhcpv4.DHCPv4, bool) { mac := req.ClientHWAddr - if err := applyEndpointForMACAddress(mac, ipamv1alpha1.CIPv4SubnetType); err != nil { + if err := ApplyEndpointForMACAddress(mac, ipamv1alpha1.CIPv4SubnetType); err != nil { log.Errorf("Could not apply peer address: %s", err) return resp, false } @@ -132,27 +174,26 @@ func handler4(req, resp *dhcpv4.DHCPv4) (*dhcpv4.DHCPv4, bool) { return resp, false } -func applyEndpointForMACAddress(mac net.HardwareAddr, subnetFamily ipamv1alpha1.SubnetAddressType) error { - inventoryName, ok := inventoryMap[strings.ToLower(mac.String())] - if !ok { - // done here, return no error, next plugin - log.Printf("Unknown inventory MAC address: %s", mac.String()) +func ApplyEndpointForMACAddress(mac net.HardwareAddr, subnetFamily ipamv1alpha1.SubnetAddressType) error { + inventoryName := GetInventoryEntryMatchingMACAddress(mac) + if inventoryName == "" { + log.Print("Unknown inventory, not processing") return nil } - ip, err := GetIPForMACAddress(mac, subnetFamily) + ip, err := GetIPAMIPAddressForMACAddress(mac, subnetFamily) if err != nil { - return fmt.Errorf("could not get IP for MAC address %s: %s", mac.String(), err) + return fmt.Errorf("could not get IPAM IP for MAC address %s: %s", mac.String(), err) } if ip != nil { if err := ApplyEndpointForInventory(inventoryName, mac, ip); err != nil { return fmt.Errorf("could not apply endpoint for inventory: %s", err) } else { - log.Infof("Successfully applied endpoint for inventory %s[%s]", inventoryName, mac.String()) + log.Infof("Successfully applied endpoint for inventory %s (%s)", inventoryName, mac.String()) } } else { - log.Infof("Could not find IP for MAC address %s", mac.String()) + log.Infof("Could not find IPAM IP for MAC address %s", mac.String()) } return nil @@ -167,29 +208,104 @@ func ApplyEndpointForInventory(name string, mac net.HardwareAddr, ip *netip.Addr ctx, cancel := context.WithCancel(context.Background()) defer cancel() - endpoint := &metalv1alpha1.Endpoint{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - Spec: metalv1alpha1.EndpointSpec{ - MACAddress: mac.String(), - IP: metalv1alpha1.MustParseIP(ip.String()), - }, + cl := kubernetes.GetClient() + if cl == nil { + return fmt.Errorf("kubernetes client not initialized") + } + if strategy == OnboardingFromStaticList { + // we do know the real name, so CreateOrPatch is fine + endpoint := &metalv1alpha1.Endpoint{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: metalv1alpha1.EndpointSpec{ + MACAddress: mac.String(), + IP: metalv1alpha1.MustParseIP(ip.String()), + }, + } + if _, err := controllerutil.CreateOrPatch(ctx, cl, endpoint, nil); err != nil { + return fmt.Errorf("failed to apply endpoint: %v", err) + } + } else if strategy == OnboardingFromMACPrefixFilter { + // the (generated) name is unknown, so go for filtering + if existingEndpoint, _ := GetEndpointForMACAddress(mac); existingEndpoint != nil { + if existingEndpoint.Spec.IP.String() != ip.String() { + log.Debugf("Endpoint exists with different IP address, updating IP address %s to %s", + existingEndpoint.Spec.IP.String(), ip.String()) + epPatch := client.MergeFrom(existingEndpoint.DeepCopy()) + existingEndpoint.Spec.IP = metalv1alpha1.MustParseIP(ip.String()) + if err := cl.Patch(ctx, existingEndpoint, epPatch); err != nil { + return fmt.Errorf("failed to patch endpoint: %v", err) + } + } + log.Debugf("Endpoint %s (%s) exists, nothing to do", mac.String(), ip.String()) + } else { + log.Debugf("Endpoint %s (%s) does not exist, creating", mac.String(), ip.String()) + endpoint := &metalv1alpha1.Endpoint{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: name, + }, + Spec: metalv1alpha1.EndpointSpec{ + MACAddress: mac.String(), + IP: metalv1alpha1.MustParseIP(ip.String()), + }, + } + if err := cl.Create(ctx, endpoint); err != nil { + return fmt.Errorf("failed to create endpoint: %v", err) + } + } + } else { + return fmt.Errorf("unknown OnboardingStrategy %d", strategy) } + return nil +} + +func GetEndpointForMACAddress(mac net.HardwareAddr) (*metalv1alpha1.Endpoint, error) { cl := kubernetes.GetClient() if cl == nil { - return fmt.Errorf("kubernetes client not initialized") + return nil, fmt.Errorf("kubernetes client not initialized") } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() - if _, err := controllerutil.CreateOrPatch(ctx, cl, endpoint, nil); err != nil { - return fmt.Errorf("failed to apply endpoint: %v", err) + epList := &metalv1alpha1.EndpointList{} + if err := cl.List(ctx, epList); err != nil { + return nil, fmt.Errorf("failed to list Endpoints: %v", err) } - return nil + for _, ep := range epList.Items { + if ep.Spec.MACAddress == mac.String() { + return &ep, nil + } + } + return nil, nil +} + +func GetInventoryEntryMatchingMACAddress(mac net.HardwareAddr) string { + if strategy == OnboardingFromStaticList { + inventoryName, ok := inventoryMap[strings.ToLower(mac.String())] + if !ok { + log.Debugf("Unknown inventory MAC address: %s", mac.String()) + } else { + return inventoryName + } + } else if strategy == OnboardingFromMACPrefixFilter { + for i, _ := range inventoryMap { + if strings.HasPrefix(strings.ToLower(mac.String()), strings.ToLower(i)) { + return inventoryMap[i] + } + } + // we don't onboard by default yet, might change in the future + log.Debugf("Inventory MAC address %s does not match any inventory MAC prefix", mac.String()) + } else { + log.Debugf("Unknown Onboarding strategy %d", strategy) + } + + return "" } -func GetIPForMACAddress(mac net.HardwareAddr, subnetFamily ipamv1alpha1.SubnetAddressType) (*netip.Addr, error) { +func GetIPAMIPAddressForMACAddress(mac net.HardwareAddr, subnetFamily ipamv1alpha1.SubnetAddressType) (*netip.Addr, error) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() diff --git a/plugins/metal/plugin_test.go b/plugins/metal/plugin_test.go index bbbe65d..bd9011b 100644 --- a/plugins/metal/plugin_test.go +++ b/plugins/metal/plugin_test.go @@ -8,11 +8,12 @@ import ( "os" "strings" + "github.com/ironcore-dev/fedhcp/internal/api" + "gopkg.in/yaml.v2" "github.com/insomniacslk/dhcp/dhcpv4" "github.com/insomniacslk/dhcp/dhcpv6" - "github.com/ironcore-dev/fedhcp/internal/api" ipamv1alpha1 "github.com/ironcore-dev/ipam/api/ipam/v1alpha1" metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" "github.com/mdlayher/netx/eui64" @@ -117,12 +118,14 @@ var _ = Describe("Endpoint", func() { Expect(err).To(HaveOccurred()) }) - It("Should return empty inventory list if the config file is malformed", func() { - configFile := "config.yaml" - data := []map[string]string{ - { - "foo": "compute-1", - "bar": "aa:bb:cc:dd:ee:ff", + It("Should return an empty inventory for an empty list", func() { + configFile := inventoryConfigFile + data := api.Config{ + Inventories: []api.Inventory{ + {}, + }, + Filter: api.Filter{ + MacPrefix: []string{}, }, } configData, err := yaml.Marshal(data) @@ -140,12 +143,73 @@ var _ = Describe("Endpoint", func() { Expect(i).To(BeEmpty()) }) - It("Should return a valid inventory list for a valid config", func() { - configFile := "config.yaml" - data := []api.Inventory{ - { - Name: "compute-1", - MacAddress: "aa:bb:cc:dd:ee:ff", + It("Should return a valid inventory list with default name prefix for non-empty MAC address filter", func() { + configFile := inventoryConfigFile + data := api.Config{ + Filter: api.Filter{ + MacPrefix: []string{ + "aa:bb:cc:dd:ee:ff", + }, + }, + } + configData, err := yaml.Marshal(data) + Expect(err).NotTo(HaveOccurred()) + + file, err := os.CreateTemp(GinkgoT().TempDir(), configFile) + Expect(err).NotTo(HaveOccurred()) + defer func() { + _ = file.Close() + }() + Expect(os.WriteFile(file.Name(), configData, 0644)).To(Succeed()) + + i, err := loadConfig(file.Name()) + Expect(err).NotTo(HaveOccurred()) + Expect(i).To(HaveKey("aa:bb:cc:dd:ee:ff")) + pref := i["aa:bb:cc:dd:ee:ff"] + Expect(pref).To(HavePrefix(defaultNamePrefix)) + }) + + It("Should return an inventory list with custom name prefix for non-empty MAC address filter and set prefix", func() { + configFile := inventoryConfigFile + data := api.Config{ + NamePrefix: "server-", + Filter: api.Filter{ + MacPrefix: []string{ + "aa:bb:cc:dd:ee:ff", + }, + }, + } + configData, err := yaml.Marshal(data) + Expect(err).NotTo(HaveOccurred()) + + file, err := os.CreateTemp(GinkgoT().TempDir(), configFile) + Expect(err).NotTo(HaveOccurred()) + defer func() { + _ = file.Close() + }() + Expect(os.WriteFile(file.Name(), configData, 0644)).To(Succeed()) + + i, err := loadConfig(file.Name()) + Expect(err).NotTo(HaveOccurred()) + Expect(i).To(HaveKey("aa:bb:cc:dd:ee:ff")) + pref := i["aa:bb:cc:dd:ee:ff"] + Expect(pref).To(HavePrefix("server-")) + }) + + It("Should return a valid inventory list for a non-empty inventory section, precedence over MAC filter", func() { + configFile := inventoryConfigFile + data := api.Config{ + NamePrefix: "server-", + Inventories: []api.Inventory{ + { + Name: "compute-1", + MacAddress: "aa:bb:cc:dd:ee:ff", + }, + }, + Filter: api.Filter{ + MacPrefix: []string{ + "aa:bb:cc:dd:ee:ff", + }, }, } configData, err := yaml.Marshal(data) @@ -188,10 +252,62 @@ var _ = Describe("Endpoint", func() { DeferCleanup(k8sClient.Delete, endpoint) }) + It("Should create an endpoint for IPv6 DHCP request from a known MAC prefix with IP address", func(ctx SpecContext) { + mac, _ := net.ParseMAC(machineWithIPAddressMACAddress) + ip := net.ParseIP(linkLocalIPV6Prefix) + linkLocalIPV6Addr, _ := eui64.ParseMAC(ip, mac) + + req, _ := dhcpv6.NewMessage() + req.MessageType = dhcpv6.MessageTypeRequest + relayedRequest, _ := dhcpv6.EncapsulateRelay(req, dhcpv6.MessageTypeRelayForward, net.IPv6loopback, linkLocalIPV6Addr) + + configFile := inventoryConfigFile + data := api.Config{ + NamePrefix: "foobar-", + Inventories: []api.Inventory{}, + Filter: api.Filter{ + MacPrefix: []string{machineWithIPAddressMACAddressPrefFilter}, + }, + } + configData, err := yaml.Marshal(data) + Expect(err).NotTo(HaveOccurred()) + + file, err := os.CreateTemp(GinkgoT().TempDir(), configFile) + Expect(err).NotTo(HaveOccurred()) + defer func() { + _ = file.Close() + }() + Expect(os.WriteFile(file.Name(), configData, 0644)).To(Succeed()) + + inventoryMap, err = loadConfig(file.Name()) + Expect(err).NotTo(HaveOccurred()) + + stub, _ := dhcpv6.NewMessage() + stub.MessageType = dhcpv6.MessageTypeReply + _, _ = handler6(relayedRequest, stub) + + epList := &metalv1alpha1.EndpointList{} + Eventually(ObjectList(epList)).Should(SatisfyAll( + HaveField("Items", HaveLen(1)), + HaveField("Items", ContainElement(SatisfyAll( + HaveField("ObjectMeta.Name", HavePrefix("foobar-")), + HaveField("Spec.MACAddress", machineWithIPAddressMACAddress), + HaveField("Spec.IP", metalv1alpha1.MustParseIP(linkLocalIPV6Addr.String())), + ))), + )) + + endpoint := &metalv1alpha1.Endpoint{ + ObjectMeta: metav1.ObjectMeta{ + Name: epList.Items[0].Name, + }, + } + DeferCleanup(k8sClient.Delete, endpoint) + }) + It("Should not return an IP address for a known machine without IP address", func(ctx SpecContext) { mac, _ := net.ParseMAC(machineWithoutIPAddressMACAddress) - ip, err := GetIPForMACAddress(mac, ipamv1alpha1.CIPv6SubnetType) + ip, err := GetIPAMIPAddressForMACAddress(mac, ipamv1alpha1.CIPv6SubnetType) Eventually(err).Should(BeNil()) Eventually(ip).Should(BeNil()) }) @@ -273,6 +389,53 @@ var _ = Describe("Endpoint", func() { DeferCleanup(k8sClient.Delete, endpoint) }) + It("Should create an endpoint for IPv4 DHCP request from a known MAC prefix with IP address", func(ctx SpecContext) { + mac, _ := net.ParseMAC(machineWithIPAddressMACAddress) + + req, _ := dhcpv4.NewDiscovery(mac) + stub, _ := dhcpv4.NewReplyFromRequest(req) + + configFile := inventoryConfigFile + data := api.Config{ + NamePrefix: "", + Inventories: []api.Inventory{}, + Filter: api.Filter{ + MacPrefix: []string{machineWithIPAddressMACAddressPrefFilter}, + }, + } + configData, err := yaml.Marshal(data) + Expect(err).NotTo(HaveOccurred()) + + file, err := os.CreateTemp(GinkgoT().TempDir(), configFile) + Expect(err).NotTo(HaveOccurred()) + defer func() { + _ = file.Close() + }() + Expect(os.WriteFile(file.Name(), configData, 0644)).To(Succeed()) + + inventoryMap, err = loadConfig(file.Name()) + Expect(err).NotTo(HaveOccurred()) + + _, _ = handler4(req, stub) + + epList := &metalv1alpha1.EndpointList{} + Eventually(ObjectList(epList)).Should(SatisfyAll( + HaveField("Items", HaveLen(1)), + HaveField("Items", ContainElement(SatisfyAll( + HaveField("ObjectMeta.Name", HavePrefix(defaultNamePrefix)), + HaveField("Spec.MACAddress", machineWithIPAddressMACAddress), + HaveField("Spec.IP", metalv1alpha1.MustParseIP(privateIPV4Address)), + ))), + )) + + endpoint := &metalv1alpha1.Endpoint{ + ObjectMeta: metav1.ObjectMeta{ + Name: epList.Items[0].Name, + }, + } + DeferCleanup(k8sClient.Delete, endpoint) + }) + It("Should not create an endpoint for IPv4 DHCP request from a known machine without IP address", func(ctx SpecContext) { mac, _ := net.ParseMAC(machineWithoutIPAddressMACAddress) diff --git a/plugins/metal/suite_test.go b/plugins/metal/suite_test.go index 0cabf93..6bd0558 100644 --- a/plugins/metal/suite_test.go +++ b/plugins/metal/suite_test.go @@ -5,11 +5,14 @@ package metal import ( "fmt" + "os" "path/filepath" "runtime" "testing" "time" + "gopkg.in/yaml.v2" + "github.com/ironcore-dev/controller-utils/modutils" "github.com/ironcore-dev/fedhcp/internal/api" "github.com/ironcore-dev/fedhcp/internal/kubernetes" @@ -32,16 +35,18 @@ import ( ) const ( - pollingInterval = 50 * time.Millisecond - eventuallyTimeout = 3 * time.Second - consistentlyDuration = 1 * time.Second - machineWithIPAddressName = "machine-with-ip-address" - machineWithoutIPAddressName = "machine-without-ip-address" - machineWithIPAddressMACAddress = "11:22:33:44:55:66" - machineWithoutIPAddressMACAddress = "47:11:47:11:47:11" - unknownMachineMACAddress = "11:11:11:11:11:11" - linkLocalIPV6Prefix = "fe80::" - privateIPV4Address = "192.168.47.11" + pollingInterval = 50 * time.Millisecond + eventuallyTimeout = 3 * time.Second + consistentlyDuration = 1 * time.Second + machineWithIPAddressName = "machine-with-ip-address" + machineWithoutIPAddressName = "machine-without-ip-address" + machineWithIPAddressMACAddress = "11:22:33:44:55:66" + machineWithoutIPAddressMACAddress = "47:11:47:11:47:11" + unknownMachineMACAddress = "11:11:11:11:11:11" + machineWithIPAddressMACAddressPrefFilter = "11:22:33" + linkLocalIPV6Prefix = "fe80::" + privateIPV4Address = "192.168.47.11" + inventoryConfigFile = "config.yaml" ) var ( @@ -116,20 +121,37 @@ func SetupTest() *corev1.Namespace { Expect(k8sClient.Create(ctx, ns)).To(Succeed(), "failed to create test namespace") DeferCleanup(k8sClient.Delete, ns) - config := []api.Inventory{ - { - Name: machineWithIPAddressName, - MacAddress: machineWithIPAddressMACAddress, + configFile := inventoryConfigFile + data := api.Config{ + NamePrefix: "server-", + Inventories: []api.Inventory{ + { + Name: machineWithIPAddressName, + MacAddress: machineWithIPAddressMACAddress, + }, + { + Name: machineWithoutIPAddressName, + MacAddress: machineWithoutIPAddressMACAddress, + }, }, - { - Name: machineWithoutIPAddressName, - MacAddress: machineWithoutIPAddressMACAddress, + Filter: api.Filter{ + MacPrefix: []string{machineWithIPAddressMACAddressPrefFilter}, }, } - inventoryMap = make(map[string]string) - for _, i := range config { - inventoryMap[i.MacAddress] = i.Name - } + configData, err := yaml.Marshal(data) + Expect(err).NotTo(HaveOccurred()) + + file, err := os.CreateTemp(GinkgoT().TempDir(), configFile) + Expect(err).NotTo(HaveOccurred()) + defer func() { + _ = file.Close() + }() + Expect(os.WriteFile(file.Name(), configData, 0644)).To(Succeed()) + + inventoryMap, err = loadConfig(file.Name()) + Expect(err).NotTo(HaveOccurred()) + Expect(inventoryMap).To(HaveKeyWithValue(machineWithIPAddressMACAddress, machineWithIPAddressName)) + Expect(inventoryMap).To(HaveKeyWithValue(machineWithoutIPAddressMACAddress, machineWithoutIPAddressName)) }) return ns From 3e0e9d1930f2361879370799114f18184057698f Mon Sep 17 00:00:00 2001 From: Damyan Yordanov Date: Fri, 11 Oct 2024 18:01:29 +0200 Subject: [PATCH 3/5] Fix linting issue --- plugins/metal/plugin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/metal/plugin.go b/plugins/metal/plugin.go index 5ea2796..55d8312 100644 --- a/plugins/metal/plugin.go +++ b/plugins/metal/plugin.go @@ -291,7 +291,7 @@ func GetInventoryEntryMatchingMACAddress(mac net.HardwareAddr) string { return inventoryName } } else if strategy == OnboardingFromMACPrefixFilter { - for i, _ := range inventoryMap { + for i := range inventoryMap { if strings.HasPrefix(strings.ToLower(mac.String()), strings.ToLower(i)) { return inventoryMap[i] } From 1eaa706b5f7f5aad131a8f5b5fa15bc45210c31b Mon Sep 17 00:00:00 2001 From: Andreas Fritzler Date: Thu, 24 Oct 2024 11:21:06 +0200 Subject: [PATCH 4/5] Refactor config loading --- plugins/metal/plugin.go | 77 ++++++++++++++++++++---------------- plugins/metal/plugin_test.go | 16 ++++---- plugins/metal/suite_test.go | 7 ++-- 3 files changed, 54 insertions(+), 46 deletions(-) diff --git a/plugins/metal/plugin.go b/plugins/metal/plugin.go index 55d8312..41f0bc8 100644 --- a/plugins/metal/plugin.go +++ b/plugins/metal/plugin.go @@ -38,24 +38,23 @@ var Plugin = plugins.Plugin{ } // map MAC address to inventory name -var inventoryMap map[string]string +var inventory *Inventory + +type Inventory struct { + Entries map[string]string + Strategy OnBoardingStrategy +} // default inventory name prefix const defaultNamePrefix = "compute-" -type OnBoardingStrategy int +type OnBoardingStrategy string const ( - OnboardingFromStaticList = 1 - OnboardingFromMACPrefixFilter = 2 + OnBoardingStrategyStatic OnBoardingStrategy = "Static" + OnboardingStrategyDynamic OnBoardingStrategy = "Dynamic" ) -// flag for onboarding strategy: -// OnboardingFromStaticList: the name comes from a static inventory list and shall be taken "as is" -// -// OnboardingFromMACPrefixFilter: the name is only a prefix (default or custom), k8s will autogenerate it -var strategy OnBoardingStrategy - // args[0] = path to inventory file func parseArgs(args ...string) (string, error) { if len(args) != 1 { @@ -66,18 +65,18 @@ func parseArgs(args ...string) (string, error) { func setup6(args ...string) (handler.Handler6, error) { var err error - inventoryMap, err = loadConfig(args...) + inventory, err = loadConfig(args...) if err != nil { return nil, err } - if inventoryMap == nil { + if inventory == nil || len(inventory.Entries) == 0 { return nil, nil } return handler6, nil } -func loadConfig(args ...string) (map[string]string, error) { +func loadConfig(args ...string) (*Inventory, error) { path, err := parseArgs(args...) if err != nil { return nil, fmt.Errorf("invalid configuration: %v", err) @@ -94,42 +93,45 @@ func loadConfig(args ...string) (map[string]string, error) { return nil, fmt.Errorf("failed to parse config file: %v", err) } - inventories := make(map[string]string) + inv := &Inventory{} + entries := make(map[string]string) // static inventory list has precedence, always if len(config.Inventories) > 0 { - strategy = OnboardingFromStaticList + inv.Strategy = OnBoardingStrategyStatic log.Debug("Using static list onboarding") for _, i := range config.Inventories { if i.MacAddress != "" && i.Name != "" { - inventories[strings.ToLower(i.MacAddress)] = i.Name + entries[strings.ToLower(i.MacAddress)] = i.Name } } } else if len(config.Filter.MacPrefix) > 0 { - strategy = OnboardingFromMACPrefixFilter + inv.Strategy = OnboardingStrategyDynamic namePrefix := defaultNamePrefix if config.NamePrefix != "" { namePrefix = config.NamePrefix } log.Debugf("Using MAC address prefix filter onboarding with name prefix '%s'", namePrefix) for _, i := range config.Filter.MacPrefix { - inventories[strings.ToLower(i)] = namePrefix + entries[strings.ToLower(i)] = namePrefix } } else { log.Infof("No inventories loaded") return nil, nil } - log.Infof("Loaded metal config with %d inventories", len(inventories)) - return inventories, nil + inv.Entries = entries + + log.Infof("Loaded metal config with %d inventories", len(entries)) + return inv, nil } func setup4(args ...string) (handler.Handler4, error) { var err error - inventoryMap, err = loadConfig(args...) + inventory, err = loadConfig(args...) if err != nil { return nil, err } - if inventoryMap == nil { + if inventory == nil || len(inventory.Entries) == 0 { return nil, nil } @@ -212,7 +214,9 @@ func ApplyEndpointForInventory(name string, mac net.HardwareAddr, ip *netip.Addr if cl == nil { return fmt.Errorf("kubernetes client not initialized") } - if strategy == OnboardingFromStaticList { + + switch inventory.Strategy { + case OnBoardingStrategyStatic: // we do know the real name, so CreateOrPatch is fine endpoint := &metalv1alpha1.Endpoint{ ObjectMeta: metav1.ObjectMeta{ @@ -226,15 +230,17 @@ func ApplyEndpointForInventory(name string, mac net.HardwareAddr, ip *netip.Addr if _, err := controllerutil.CreateOrPatch(ctx, cl, endpoint, nil); err != nil { return fmt.Errorf("failed to apply endpoint: %v", err) } - } else if strategy == OnboardingFromMACPrefixFilter { + case OnboardingStrategyDynamic: // the (generated) name is unknown, so go for filtering if existingEndpoint, _ := GetEndpointForMACAddress(mac); existingEndpoint != nil { if existingEndpoint.Spec.IP.String() != ip.String() { log.Debugf("Endpoint exists with different IP address, updating IP address %s to %s", existingEndpoint.Spec.IP.String(), ip.String()) - epPatch := client.MergeFrom(existingEndpoint.DeepCopy()) + + existingEndpointBase := existingEndpoint.DeepCopy() existingEndpoint.Spec.IP = metalv1alpha1.MustParseIP(ip.String()) - if err := cl.Patch(ctx, existingEndpoint, epPatch); err != nil { + + if err := cl.Patch(ctx, existingEndpoint, client.MergeFrom(existingEndpointBase)); err != nil { return fmt.Errorf("failed to patch endpoint: %v", err) } } @@ -254,8 +260,8 @@ func ApplyEndpointForInventory(name string, mac net.HardwareAddr, ip *netip.Addr return fmt.Errorf("failed to create endpoint: %v", err) } } - } else { - return fmt.Errorf("unknown OnboardingStrategy %d", strategy) + default: + return fmt.Errorf("unknown OnboardingStrategy %s", inventory.Strategy) } return nil @@ -283,23 +289,24 @@ func GetEndpointForMACAddress(mac net.HardwareAddr) (*metalv1alpha1.Endpoint, er } func GetInventoryEntryMatchingMACAddress(mac net.HardwareAddr) string { - if strategy == OnboardingFromStaticList { - inventoryName, ok := inventoryMap[strings.ToLower(mac.String())] + switch inventory.Strategy { + case OnBoardingStrategyStatic: + inventoryName, ok := inventory.Entries[strings.ToLower(mac.String())] if !ok { log.Debugf("Unknown inventory MAC address: %s", mac.String()) } else { return inventoryName } - } else if strategy == OnboardingFromMACPrefixFilter { - for i := range inventoryMap { + case OnboardingStrategyDynamic: + for i := range inventory.Entries { if strings.HasPrefix(strings.ToLower(mac.String()), strings.ToLower(i)) { - return inventoryMap[i] + return inventory.Entries[i] } } // we don't onboard by default yet, might change in the future log.Debugf("Inventory MAC address %s does not match any inventory MAC prefix", mac.String()) - } else { - log.Debugf("Unknown Onboarding strategy %d", strategy) + default: + log.Debugf("Unknown Onboarding strategy %s", inventory.Strategy) } return "" diff --git a/plugins/metal/plugin_test.go b/plugins/metal/plugin_test.go index bd9011b..e5fcc51 100644 --- a/plugins/metal/plugin_test.go +++ b/plugins/metal/plugin_test.go @@ -140,7 +140,7 @@ var _ = Describe("Endpoint", func() { i, err := loadConfig(file.Name()) Expect(err).NotTo(HaveOccurred()) - Expect(i).To(BeEmpty()) + Expect(i.Entries).To(BeEmpty()) }) It("Should return a valid inventory list with default name prefix for non-empty MAC address filter", func() { @@ -164,8 +164,8 @@ var _ = Describe("Endpoint", func() { i, err := loadConfig(file.Name()) Expect(err).NotTo(HaveOccurred()) - Expect(i).To(HaveKey("aa:bb:cc:dd:ee:ff")) - pref := i["aa:bb:cc:dd:ee:ff"] + Expect(i.Entries).To(HaveKey("aa:bb:cc:dd:ee:ff")) + pref := i.Entries["aa:bb:cc:dd:ee:ff"] Expect(pref).To(HavePrefix(defaultNamePrefix)) }) @@ -191,8 +191,8 @@ var _ = Describe("Endpoint", func() { i, err := loadConfig(file.Name()) Expect(err).NotTo(HaveOccurred()) - Expect(i).To(HaveKey("aa:bb:cc:dd:ee:ff")) - pref := i["aa:bb:cc:dd:ee:ff"] + Expect(i.Entries).To(HaveKey("aa:bb:cc:dd:ee:ff")) + pref := i.Entries["aa:bb:cc:dd:ee:ff"] Expect(pref).To(HavePrefix("server-")) }) @@ -224,7 +224,7 @@ var _ = Describe("Endpoint", func() { i, err := loadConfig(file.Name()) Expect(err).NotTo(HaveOccurred()) - Expect(i).To(HaveKeyWithValue("aa:bb:cc:dd:ee:ff", "compute-1")) + Expect(i.Entries).To(HaveKeyWithValue("aa:bb:cc:dd:ee:ff", "compute-1")) }) It("Should create an endpoint for IPv6 DHCP request from a known machine with IP address", func(ctx SpecContext) { @@ -279,7 +279,7 @@ var _ = Describe("Endpoint", func() { }() Expect(os.WriteFile(file.Name(), configData, 0644)).To(Succeed()) - inventoryMap, err = loadConfig(file.Name()) + inventory, err = loadConfig(file.Name()) Expect(err).NotTo(HaveOccurred()) stub, _ := dhcpv6.NewMessage() @@ -413,7 +413,7 @@ var _ = Describe("Endpoint", func() { }() Expect(os.WriteFile(file.Name(), configData, 0644)).To(Succeed()) - inventoryMap, err = loadConfig(file.Name()) + inventory, err = loadConfig(file.Name()) Expect(err).NotTo(HaveOccurred()) _, _ = handler4(req, stub) diff --git a/plugins/metal/suite_test.go b/plugins/metal/suite_test.go index 6bd0558..29ba743 100644 --- a/plugins/metal/suite_test.go +++ b/plugins/metal/suite_test.go @@ -148,10 +148,11 @@ func SetupTest() *corev1.Namespace { }() Expect(os.WriteFile(file.Name(), configData, 0644)).To(Succeed()) - inventoryMap, err = loadConfig(file.Name()) + inventory, err = loadConfig(file.Name()) Expect(err).NotTo(HaveOccurred()) - Expect(inventoryMap).To(HaveKeyWithValue(machineWithIPAddressMACAddress, machineWithIPAddressName)) - Expect(inventoryMap).To(HaveKeyWithValue(machineWithoutIPAddressMACAddress, machineWithoutIPAddressName)) + Expect(inventory.Entries).To(HaveKeyWithValue(machineWithIPAddressMACAddress, machineWithIPAddressName)) + Expect(inventory.Entries).To(HaveKeyWithValue(machineWithoutIPAddressMACAddress, machineWithoutIPAddressName)) + Expect(inventory.Strategy).To(Equal(OnBoardingStrategyStatic)) }) return ns From f3a73d4ff696fc90d78b737d8acf0ddc1d8de939 Mon Sep 17 00:00:00 2001 From: Damyan Yordanov Date: Fri, 25 Oct 2024 16:52:36 +0200 Subject: [PATCH 5/5] Use switch when determining the onboarding strategy --- plugins/metal/plugin.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/plugins/metal/plugin.go b/plugins/metal/plugin.go index 41f0bc8..cb7cc5a 100644 --- a/plugins/metal/plugin.go +++ b/plugins/metal/plugin.go @@ -95,8 +95,9 @@ func loadConfig(args ...string) (*Inventory, error) { inv := &Inventory{} entries := make(map[string]string) + switch { // static inventory list has precedence, always - if len(config.Inventories) > 0 { + case len(config.Inventories) > 0: inv.Strategy = OnBoardingStrategyStatic log.Debug("Using static list onboarding") for _, i := range config.Inventories { @@ -104,7 +105,7 @@ func loadConfig(args ...string) (*Inventory, error) { entries[strings.ToLower(i.MacAddress)] = i.Name } } - } else if len(config.Filter.MacPrefix) > 0 { + case len(config.Filter.MacPrefix) > 0: inv.Strategy = OnboardingStrategyDynamic namePrefix := defaultNamePrefix if config.NamePrefix != "" { @@ -114,7 +115,7 @@ func loadConfig(args ...string) (*Inventory, error) { for _, i := range config.Filter.MacPrefix { entries[strings.ToLower(i)] = namePrefix } - } else { + default: log.Infof("No inventories loaded") return nil, nil }