From 14dac9a4223c17bd084b544cda0c5de18172eff8 Mon Sep 17 00:00:00 2001 From: Alex Lovell-Troy Date: Thu, 25 Sep 2025 12:37:23 -0400 Subject: [PATCH] feat: add network interface handling to metadata generation and tests Signed-off-by: Alex Lovell-Troy --- .../comprehensive_metadata_test.go | 210 ++++++++++++++++++ cmd/cloud-init-server/metadata.go | 46 +++- cmd/cloud-init-server/metadata_handlers.go | 2 +- .../network_interfaces_test.go | 106 +++++++++ internal/smdclient/FakeSMDClient.go | 22 ++ internal/smdclient/SMDclient.go | 11 + 6 files changed, 385 insertions(+), 12 deletions(-) create mode 100644 cmd/cloud-init-server/comprehensive_metadata_test.go create mode 100644 cmd/cloud-init-server/network_interfaces_test.go diff --git a/cmd/cloud-init-server/comprehensive_metadata_test.go b/cmd/cloud-init-server/comprehensive_metadata_test.go new file mode 100644 index 00000000..3d91092f --- /dev/null +++ b/cmd/cloud-init-server/comprehensive_metadata_test.go @@ -0,0 +1,210 @@ +package main + +import ( + "strings" + "testing" + + base "github.com/Cray-HPE/hms-base" + "github.com/OpenCHAMI/cloud-init/internal/memstore" + "github.com/OpenCHAMI/cloud-init/internal/smdclient" + "github.com/OpenCHAMI/cloud-init/pkg/cistore" + yaml "gopkg.in/yaml.v2" +) + +// TestCompleteMetadataWithNetworkInterfaces tests the complete metadata output including network interfaces +func TestCompleteMetadataWithNetworkInterfaces(t *testing.T) { + // Create a test store with some group data + store := memstore.NewMemStore() + + // Add cluster defaults + clusterDefaults := cistore.ClusterDefaults{ + ClusterName: "demo-cluster", + ShortName: "demo", + NidLength: 4, + CloudProvider: "openchami", + Region: "us-west-2", + } + err := store.SetClusterDefaults(clusterDefaults) + if err != nil { + t.Fatalf("Failed to set cluster defaults: %v", err) + } + + // Add a group with metadata + groupData := cistore.GroupData{ + Name: "compute", + Description: "Compute nodes", + Data: map[string]interface{}{ + "syslog_server": "192.168.1.10", + "ntp_servers": "pool.ntp.org time.nist.gov", + "environment": "production cluster", + "management_network": "10.1.0.0/16", + }, + } + err = store.AddGroupData("compute", groupData) + if err != nil { + t.Fatalf("Failed to add group data: %v", err) + } + + // Create a fake SMD client + fakeSmd := smdclient.NewFakeSMDClient("demo-cluster", 1) + + // Create test component - use the ID that matches the fake SMD client + component := cistore.OpenCHAMIComponent{ + Component: base.Component{ + ID: "x3000c0b0n1", + Type: "Node", + }, + IP: "10.20.30.1", // This should match what the fake SMD generates + MAC: "00:DE:AD:BE:EF:01", // This should match what the fake SMD generates + } + + // Generate metadata with the compute group + groups := []string{"compute"} + metadata := generateMetaData(component, groups, store, fakeSmd) + + // Marshal to YAML to see the complete output + yamlData, err := yaml.Marshal(metadata) + if err != nil { + t.Fatalf("Failed to marshal metadata to YAML: %v", err) + } + + yamlString := string(yamlData) + t.Logf("Complete metadata YAML:\n%s", yamlString) + + // Verify all the expected components are present + + // Basic metadata + if metadata.ClusterName != "demo-cluster" { + t.Errorf("Expected cluster name 'demo-cluster', got '%s'", metadata.ClusterName) + } + + // Network interfaces + interfaces := metadata.InstanceData.V1.VendorData.NetworkInterfaces + if len(interfaces) == 0 { + t.Error("Expected at least one network interface") + } else { + firstInterface := interfaces[0] + if firstInterface.MAC == "" { + t.Error("Expected MAC address to be populated") + } + if firstInterface.IP == "" { + t.Error("Expected IP address to be populated") + } + t.Logf("Network interface: MAC=%s, IP=%s, Desc=%s", + firstInterface.MAC, firstInterface.IP, firstInterface.Description) + } + + // Group metadata + groups_data := metadata.InstanceData.V1.VendorData.Groups + if len(groups_data) == 0 { + t.Error("Expected group data to be present") + } else { + computeGroup, exists := groups_data["compute"] + if !exists { + t.Error("Expected 'compute' group to be present") + } else { + // Check that our group metadata values with spaces are preserved + if syslogServer, ok := computeGroup["syslog_server"]; ok { + if syslogServer != "192.168.1.10" { + t.Errorf("Expected syslog_server '192.168.1.10', got '%v'", syslogServer) + } + } else { + t.Error("Expected 'syslog_server' to be present in compute group") + } + + if ntpServers, ok := computeGroup["ntp_servers"]; ok { + ntpStr := ntpServers.(string) + if !strings.Contains(ntpStr, "pool.ntp.org") || !strings.Contains(ntpStr, "time.nist.gov") { + t.Errorf("Expected ntp_servers to contain both servers, got '%v'", ntpServers) + } + // Verify no unexpected newlines in the value with spaces + if strings.Contains(ntpStr, "\n") { + t.Errorf("NTP servers value contains unexpected newlines: '%v'", ntpServers) + } + } else { + t.Error("Expected 'ntp_servers' to be present in compute group") + } + } + } + + // Verify in YAML output that values with spaces are handled correctly + if strings.Contains(yamlString, "pool.ntp.org\ntime.nist.gov") { + t.Error("NTP servers value was incorrectly split across lines in YAML") + } + + if strings.Contains(yamlString, "production\ncluster") { + t.Error("Environment value was incorrectly split across lines in YAML") + } + + // Verify the YAML contains the expected network interface fields + if !strings.Contains(yamlString, "network_interfaces:") { + t.Error("Expected 'network_interfaces:' to be present in YAML output") + } +} + +// TestNetworkInterfacesPreserveSpacesInValues specifically tests that spaces in metadata don't cause newlines +func TestNetworkInterfacesPreserveSpacesInValues(t *testing.T) { + store := memstore.NewMemStore() + + // Add group data with various space scenarios + groupData := cistore.GroupData{ + Name: "test-spaces", + Description: "Group to test space handling", + Data: map[string]interface{}{ + "single_space": "hello world", + "multiple_spaces": "value with multiple spaces", + "leading_space": " leading space", + "trailing_space": "trailing space ", + "description": "This is a long description with many words and spaces", + "command": "systemctl restart some-service --with-options", + "url_with_spaces": "https://example.com/path with spaces/file.txt", + }, + } + err := store.AddGroupData("test-spaces", groupData) + if err != nil { + t.Fatalf("Failed to add group data: %v", err) + } + + fakeSmd := smdclient.NewFakeSMDClient("test-cluster", 1) + component := cistore.OpenCHAMIComponent{ + Component: base.Component{ + ID: "x3000c0b0n1", + Type: "Node", + }, + IP: "10.20.30.1", + MAC: "00:DE:AD:BE:EF:01", + } + + groups := []string{"test-spaces"} + metadata := generateMetaData(component, groups, store, fakeSmd) + + yamlData, err := yaml.Marshal(metadata) + if err != nil { + t.Fatalf("Failed to marshal metadata to YAML: %v", err) + } + + yamlString := string(yamlData) + t.Logf("YAML with spaces test:\n%s", yamlString) + + // Check that none of the values with spaces were split into multiple lines + testCases := []struct { + original string + broken string + name string + }{ + {"hello world", "hello\nworld", "single space"}, + {"multiple spaces", "multiple\n", "multiple spaces"}, + {"This is a long description", "This is a\nlong", "long description"}, + {"systemctl restart some-service", "systemctl restart\nsome-service", "command with spaces"}, + {"path with spaces", "path with\nspaces", "URL with spaces"}, + } + + for _, tc := range testCases { + if strings.Contains(yamlString, tc.broken) { + t.Errorf("%s was incorrectly split across lines in YAML", tc.name) + } + if !strings.Contains(yamlString, tc.original) { + t.Errorf("Expected '%s' to be present as single value in YAML", tc.original) + } + } +} diff --git a/cmd/cloud-init-server/metadata.go b/cmd/cloud-init-server/metadata.go index a167a865..df40ee1c 100644 --- a/cmd/cloud-init-server/metadata.go +++ b/cmd/cloud-init-server/metadata.go @@ -3,6 +3,7 @@ package main import ( "fmt" + "github.com/OpenCHAMI/cloud-init/internal/smdclient" "github.com/OpenCHAMI/cloud-init/pkg/cistore" "github.com/rs/zerolog/log" ) @@ -32,21 +33,29 @@ type InstanceData struct { } type VendorData struct { - Version string `json:"version" yaml:"version"` - CloudInitBaseURL string `json:"cloud-init-base-url,omitempty" yaml:"cloud_init_base_url,omitempty"` - Rack string `json:"rack,omitempty" yaml:"rack,omitempty"` - Nid int64 `json:"nid,omitempty" yaml:"nid,omitempty"` - Role string `json:"role,omitempty" yaml:"role,omitempty"` - SubRole string `json:"sub-role,omitempty" yaml:"sub_role,omitempty"` - Cabinet string `json:"cabinet,omitempty" yaml:"cabinet,omitempty"` - Location string `json:"location,omitempty" yaml:"location,omitempty"` - ClusterName string `json:"cluster_name,omitempty" yaml:"cluster_name,omitempty" example:"demo" description:"Long name of entire cluster, used as a human-readable identifier and is used in the cluster's FQDN"` - Groups map[string]Group `json:"groups" yaml:"groups" description:"Groups known to cloud-init and their meta-data"` + Version string `json:"version" yaml:"version"` + CloudInitBaseURL string `json:"cloud-init-base-url,omitempty" yaml:"cloud_init_base_url,omitempty"` + Rack string `json:"rack,omitempty" yaml:"rack,omitempty"` + Nid int64 `json:"nid,omitempty" yaml:"nid,omitempty"` + Role string `json:"role,omitempty" yaml:"role,omitempty"` + SubRole string `json:"sub-role,omitempty" yaml:"sub_role,omitempty"` + Cabinet string `json:"cabinet,omitempty" yaml:"cabinet,omitempty"` + Location string `json:"location,omitempty" yaml:"location,omitempty"` + ClusterName string `json:"cluster_name,omitempty" yaml:"cluster_name,omitempty" example:"demo" description:"Long name of entire cluster, used as a human-readable identifier and is used in the cluster's FQDN"` + Groups map[string]Group `json:"groups" yaml:"groups" description:"Groups known to cloud-init and their meta-data"` + NetworkInterfaces []NetworkInterface `json:"network_interfaces,omitempty" yaml:"network_interfaces,omitempty" description:"All network interfaces for this node"` } type Group map[string]interface{} -func generateMetaData(component cistore.OpenCHAMIComponent, groups []string, s cistore.Store) MetaData { +type NetworkInterface struct { + MAC string `json:"mac" yaml:"mac"` + IP string `json:"ip" yaml:"ip"` + WGIP string `json:"wgip,omitempty" yaml:"wgip,omitempty"` + Description string `json:"description,omitempty" yaml:"description,omitempty"` +} + +func generateMetaData(component cistore.OpenCHAMIComponent, groups []string, s cistore.Store, smdClient smdclient.SMDClientInterface) MetaData { metadata := MetaData{} extendedInstanceData, err := s.GetInstanceInfo(component.ID) if err != nil { @@ -96,6 +105,21 @@ func generateMetaData(component cistore.OpenCHAMIComponent, groups []string, s c instanceData.V1.LocalIPv4 = component.IP instanceData.V1.VendorData.Version = "1.0" + // Populate network interfaces from SMD client + if node, found := smdClient.GetNodeInterfaces(component.ID); found { + var networkInterfaces []NetworkInterface + for _, iface := range node.Interfaces { + netIface := NetworkInterface{ + MAC: iface.MAC, + IP: iface.IP, + WGIP: iface.WGIP, + Description: iface.Desc, + } + networkInterfaces = append(networkInterfaces, netIface) + } + instanceData.V1.VendorData.NetworkInterfaces = networkInterfaces + } + // Add extended attributes instanceData.V1.InstanceID = extendedInstanceData.InstanceID instanceData.V1.InstanceType = extendedInstanceData.InstanceType diff --git a/cmd/cloud-init-server/metadata_handlers.go b/cmd/cloud-init-server/metadata_handlers.go index 496c8555..97e7633d 100644 --- a/cmd/cloud-init-server/metadata_handlers.go +++ b/cmd/cloud-init-server/metadata_handlers.go @@ -91,7 +91,7 @@ func MetaDataHandler(smd smdclient.SMDClientInterface, store cistore.Store) http MAC: bootMAC, } - metadata := generateMetaData(component, groups, store) + metadata := generateMetaData(component, groups, store, smd) w.Header().Set("Content-Type", "application/x-yaml") w.WriteHeader(http.StatusOK) diff --git a/cmd/cloud-init-server/network_interfaces_test.go b/cmd/cloud-init-server/network_interfaces_test.go new file mode 100644 index 00000000..8b933d80 --- /dev/null +++ b/cmd/cloud-init-server/network_interfaces_test.go @@ -0,0 +1,106 @@ +package main + +import ( + "strings" + "testing" + + base "github.com/Cray-HPE/hms-base" + "github.com/OpenCHAMI/cloud-init/internal/memstore" + "github.com/OpenCHAMI/cloud-init/internal/smdclient" + "github.com/OpenCHAMI/cloud-init/pkg/cistore" + yaml "gopkg.in/yaml.v2" +) + +// TestNetworkInterfacesInMetadata tests that all network interfaces are included in metadata +func TestNetworkInterfacesInMetadata(t *testing.T) { + // Create a test store + store := memstore.NewMemStore() + + // Create a fake SMD client + fakeSmd := smdclient.NewFakeSMDClient("test-cluster", 1) + + // Create test component - use the ID that the fake SMD client will generate + component := cistore.OpenCHAMIComponent{ + Component: base.Component{ + ID: "x3000c0b0n1", // This matches the first generated fake component + Type: "Node", + }, + IP: "10.20.30.41", + MAC: "aa:bb:cc:dd:ee:ff", + } + + // Generate metadata with no groups for simplicity + groups := []string{} + metadata := generateMetaData(component, groups, store, fakeSmd) + + // Marshal to YAML to see the output + yamlData, err := yaml.Marshal(metadata) + if err != nil { + t.Fatalf("Failed to marshal metadata to YAML: %v", err) + } + + yamlString := string(yamlData) + t.Logf("Generated YAML:\n%s", yamlString) + + // Check if network_interfaces is present in the YAML + if !strings.Contains(yamlString, "network_interfaces:") { + t.Error("Expected 'network_interfaces' to be present in the metadata") + } + + // Check if the network interfaces contain MAC and IP information + if !strings.Contains(yamlString, "mac:") { + t.Error("Expected 'mac:' field to be present in network interfaces") + } + + if !strings.Contains(yamlString, "ip:") { + t.Error("Expected 'ip:' field to be present in network interfaces") + } + + // Verify the network interfaces are populated correctly + interfaces := metadata.InstanceData.V1.VendorData.NetworkInterfaces + if len(interfaces) == 0 { + t.Error("Expected at least one network interface in the metadata") + } else { + // Check first interface + firstInterface := interfaces[0] + if firstInterface.MAC == "" { + t.Error("Expected MAC address to be populated") + } + if firstInterface.IP == "" { + t.Error("Expected IP address to be populated") + } + t.Logf("First interface - MAC: %s, IP: %s, Description: %s", + firstInterface.MAC, firstInterface.IP, firstInterface.Description) + } +} + +// TestNetworkInterfacesWithMultipleInterfaces tests handling of multiple interfaces +func TestNetworkInterfacesWithMultipleInterfaces(t *testing.T) { + // This test would be more comprehensive if we had a way to create + // a fake SMD client with multiple interfaces per node. + // For now, we'll test that the structure supports multiple interfaces. + + store := memstore.NewMemStore() + fakeSmd := smdclient.NewFakeSMDClient("test-cluster", 1) + + component := cistore.OpenCHAMIComponent{ + Component: base.Component{ + ID: "x3000c0b0n1", // Use the correct fake SMD client ID + Type: "Node", + }, + IP: "10.20.30.41", + MAC: "aa:bb:cc:dd:ee:ff", + } + + groups := []string{} + metadata := generateMetaData(component, groups, store, fakeSmd) + + // Verify the structure can handle multiple interfaces + interfaces := metadata.InstanceData.V1.VendorData.NetworkInterfaces + t.Logf("Found %d network interfaces", len(interfaces)) + + for i, iface := range interfaces { + t.Logf("Interface %d: MAC=%s, IP=%s, WGIP=%s, Desc=%s", + i, iface.MAC, iface.IP, iface.WGIP, iface.Description) + } +} diff --git a/internal/smdclient/FakeSMDClient.go b/internal/smdclient/FakeSMDClient.go index e3f6d6b8..476de99f 100644 --- a/internal/smdclient/FakeSMDClient.go +++ b/internal/smdclient/FakeSMDClient.go @@ -378,3 +378,25 @@ func (f *FakeSMDClient) WGIPfromID(id string) (string, error) { } return "", fmt.Errorf("node (%s) not found", id) } + +// GetNodeInterfaces returns all network interfaces for a given node ID +func (f *FakeSMDClient) GetNodeInterfaces(id string) (NodeMapping, bool) { + for _, c := range f.rosetta_mapping { + if c.ComponentID == id { + // Create a NodeMapping with the interface information + node := NodeMapping{ + Xname: c.ComponentID, + Interfaces: []NodeInterface{ + { + MAC: c.BootMAC, + IP: c.BootIPAddress, + WGIP: c.WGIPAddress, + Desc: "Boot interface", + }, + }, + } + return node, true + } + } + return NodeMapping{}, false +} diff --git a/internal/smdclient/SMDclient.go b/internal/smdclient/SMDclient.go index 505cc38b..eb3d9015 100644 --- a/internal/smdclient/SMDclient.go +++ b/internal/smdclient/SMDclient.go @@ -31,6 +31,7 @@ type SMDClientInterface interface { ClusterName() string AddWGIP(id string, wgip string) error WGIPfromID(id string) (string, error) + GetNodeInterfaces(id string) (NodeMapping, bool) } // Add client usage examples @@ -386,3 +387,13 @@ func (s *SMDClient) WGIPfromID(id string) (string, error) { } return "", errors.New("ID " + id + " not found in nodes") } + +// GetNodeInterfaces returns all network interfaces for a given node ID +func (s *SMDClient) GetNodeInterfaces(id string) (NodeMapping, bool) { + s.nodesMutex.Lock() + defer s.nodesMutex.Unlock() + if node, found := s.nodes[id]; found { + return node, true + } + return NodeMapping{}, false +}