diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 7a62064..1d4a601 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -14,7 +14,7 @@ jobs: core: strategy: matrix: - go-version: [1.20.x] + go-version: [1.24.x] platform: [ubuntu-latest] name: Build runs-on: ${{ matrix.platform }} @@ -74,7 +74,7 @@ jobs: - name: Generate coverage report run: make coverage - name: Upload coverage report - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: Test Coverage Report path: .coverage/coverage.html diff --git a/Makefile b/Makefile index 00a4950..ff7a050 100644 --- a/Makefile +++ b/Makefile @@ -22,8 +22,8 @@ rights: @sudo chmod o+rw /run/ovn/ovnsb_db.sock || true linter: - @golint - @echo "PASS: golint" + @golangci-lint run ./... + @echo "PASS: golangci-lint" test: covdir linter rights @go test $(VERBOSE) -coverprofile=.coverage/coverage.out @@ -50,7 +50,7 @@ clean: dep: @echo "Making dependencies check ..." - @go install golang.org/x/lint/golint@latest + @go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest @go install github.com/kyoh86/richgo@latest @go install github.com/caddyserver/xcaddy/cmd/xcaddy@latest @go install github.com/greenpau/versioned/cmd/versioned@latest diff --git a/app_cluster.go b/app_cluster.go index 76b25e3..0aaf423 100644 --- a/app_cluster.go +++ b/app_cluster.go @@ -102,7 +102,7 @@ func (cli *OvnClient) GetAppClusteringInfo(db string) (ClusterState, error) { } if strings.HasPrefix(line, "Name:") { parserOn = true - s := strings.TrimLeft(line, "Name: ") + s := strings.TrimPrefix(line, "Name: ") s = strings.Join(strings.Fields(s), " ") server.Database = s continue @@ -111,7 +111,7 @@ func (cli *OvnClient) GetAppClusteringInfo(db string) (ClusterState, error) { continue } if strings.HasPrefix(line, "Cluster ID:") { - s := strings.TrimLeft(line, "Cluster ID:") + s := strings.TrimPrefix(line, "Cluster ID:") s = strings.Join(strings.Fields(strings.TrimSpace(s)), " ") arr := strings.Split(s, " ") if len(arr) != 2 { @@ -124,7 +124,7 @@ func (cli *OvnClient) GetAppClusteringInfo(db string) (ClusterState, error) { } continue } else if strings.HasPrefix(line, "Server ID:") { - s := strings.TrimLeft(line, "Server ID:") + s := strings.TrimPrefix(line, "Server ID:") s = strings.Join(strings.Fields(strings.TrimSpace(s)), " ") arr := strings.Split(s, " ") if len(arr) != 2 { @@ -137,12 +137,12 @@ func (cli *OvnClient) GetAppClusteringInfo(db string) (ClusterState, error) { } continue } else if strings.HasPrefix(line, "Address:") { - s := strings.TrimLeft(line, "Address:") + s := strings.TrimPrefix(line, "Address:") s = strings.Join(strings.Fields(s), " ") server.Address = s continue } else if strings.HasPrefix(line, "Status:") { - s := strings.TrimLeft(line, "Status:") + s := strings.TrimPrefix(line, "Status:") s = strings.Join(strings.Fields(s), " ") switch s { case "cluster member": @@ -152,7 +152,7 @@ func (cli *OvnClient) GetAppClusteringInfo(db string) (ClusterState, error) { } continue } else if strings.HasPrefix(line, "Role:") { - s := strings.TrimLeft(line, "Role:") + s := strings.TrimPrefix(line, "Role:") s = strings.Join(strings.Fields(s), " ") switch s { case "leader": @@ -166,13 +166,13 @@ func (cli *OvnClient) GetAppClusteringInfo(db string) (ClusterState, error) { } continue } else if strings.HasPrefix(line, "Term:") { - s := strings.TrimLeft(line, "Term:") + s := strings.TrimPrefix(line, "Term:") s = strings.Join(strings.Fields(s), " ") if term, err := strconv.ParseUint(s, 10, 64); err == nil { server.Term = term } } else if strings.HasPrefix(line, "Leader:") { - s := strings.TrimLeft(line, "Leader:") + s := strings.TrimPrefix(line, "Leader:") s = strings.Join(strings.Fields(s), " ") if s == "self" { server.IsLeaderSelf = 1 @@ -180,7 +180,7 @@ func (cli *OvnClient) GetAppClusteringInfo(db string) (ClusterState, error) { server.IsLeaderSelf = 0 } } else if strings.HasPrefix(line, "Vote:") { - s := strings.TrimLeft(line, "Vote:") + s := strings.TrimPrefix(line, "Vote:") s = strings.Join(strings.Fields(s), " ") if s == "self" { server.IsVotedSelf = 1 @@ -188,7 +188,7 @@ func (cli *OvnClient) GetAppClusteringInfo(db string) (ClusterState, error) { server.IsVotedSelf = 0 } } else if strings.HasPrefix(line, "Log:") { - s := strings.TrimLeft(line, "Log:") + s := strings.TrimPrefix(line, "Log:") s = strings.Join(strings.Fields(s), " ") s = strings.TrimLeft(s, "[") s = strings.TrimRight(s, "]") @@ -205,19 +205,19 @@ func (cli *OvnClient) GetAppClusteringInfo(db string) (ClusterState, error) { server.Log.High = i } } else if strings.HasPrefix(line, "Entries not yet committed:") { - s := strings.TrimLeft(line, "Entries not yet committed:") + s := strings.TrimPrefix(line, "Entries not yet committed:") s = strings.Join(strings.Fields(s), " ") if i, err := strconv.ParseUint(s, 10, 64); err == nil { server.NotCommittedEntries = i } } else if strings.HasPrefix(line, "Entries not yet applied:") { - s := strings.TrimLeft(line, "Entries not yet applied:") + s := strings.TrimPrefix(line, "Entries not yet applied:") s = strings.Join(strings.Fields(s), " ") if i, err := strconv.ParseUint(s, 10, 64); err == nil { server.NotAppliedEntries = i } } else if strings.HasPrefix(line, "Connections:") { - s := strings.TrimLeft(line, "Connections:") + s := strings.TrimPrefix(line, "Connections:") s = strings.Join(strings.Fields(s), " ") conns := strings.Split(strings.Join(strings.Fields(s), " "), " ") for _, entry := range conns { @@ -270,45 +270,41 @@ func (cli *OvnClient) GetAppClusteringInfo(db string) (ClusterState, error) { var peerMatchIndex uint64 for _, e := range arr { if strings.HasPrefix(e, "tcp:") || strings.HasPrefix(e, "ssl:") { - if isSelf != true { + if !isSelf { peerAddress = strings.TrimRight(e, ")") } } else if strings.HasPrefix(e, "next_index=") { - nextIndex := strings.TrimLeft(e, "next_index=") + nextIndex := strings.TrimPrefix(e, "next_index=") if i, err := strconv.ParseUint(nextIndex, 10, 64); err == nil { - if isSelf == true { + if isSelf { server.NextIndex = i } else { peerNextIndex = i } } } else if strings.HasPrefix(e, "match_index=") { - matchIndex := strings.TrimLeft(e, "match_index=") + matchIndex := strings.TrimPrefix(e, "match_index=") if i, err := strconv.ParseUint(matchIndex, 10, 64); err == nil { - if isSelf == true { + if isSelf { server.MatchIndex = i } else { peerMatchIndex = i } } - } else { - // do nothing } - } - if isSelf != true { - if _, exists := server.Peers[peerID]; !exists { - peer := ClusterPeer{} - peer.ID = peerID - server.Peers[peerID] = &peer + if !isSelf { + if _, exists := server.Peers[peerID]; !exists { + peer := ClusterPeer{} + peer.ID = peerID + server.Peers[peerID] = &peer + } + peer := server.Peers[peerID] + peer.NextIndex = peerNextIndex + peer.MatchIndex = peerMatchIndex + peer.Address = peerAddress } - peer := server.Peers[peerID] - peer.NextIndex = peerNextIndex - peer.MatchIndex = peerMatchIndex - peer.Address = peerAddress + continue } - continue - } else { - // do nothing } } //spew.Dump(server) diff --git a/client.go b/client.go index 0221ece..e36db16 100644 --- a/client.go +++ b/client.go @@ -19,6 +19,7 @@ package ovsdb import ( "encoding/json" "fmt" + //"github.com/davecgh/go-spew/spew" "io" //"math/rand" @@ -58,7 +59,7 @@ func NewClient(s string, t int) (Client, error) { cli.errQueue = make(chan error, 1) go ovsdbMessenger(cli.Endpoint, cli.Timeout, cli.txQueue, cli.rxQueue, cli.errQueue) err := <-cli.errQueue - return cli, err + return cli, err //nolint:govet } // Close TODO @@ -82,7 +83,7 @@ func (cli *Client) query(method string, param interface{}) (*Response, error) { Params: param, } for { - if cli.closed == false { + if !cli.closed { cli.txQueue <- req select { case err := <-cli.errQueue: @@ -114,7 +115,6 @@ func (cli *Client) query(method string, param interface{}) (*Response, error) { retryAttempts-- } } - return nil, fmt.Errorf("%s", errMsgs) } func (cli *Client) getColumns(db, table string) (map[string]string, error) { @@ -266,20 +266,18 @@ func ovsdbMessenger(s string, t int, rxQueue <-chan Request, txQueue chan<- Resp errQueue <- nil cli := newClientCodec(conn) for { - select { - case reqMsg := <-rxQueue: - var req rpc.Request - if reqMsg.Method == "shutdown" { - cli.Close() - errQueue <- nil - return - } - req.ServiceMethod = reqMsg.Method - req.Seq = counter - if err := cli.WriteRequest(&req, reqMsg.Params); err != nil { - errQueue <- err - return - } + reqMsg := <-rxQueue + var req rpc.Request + if reqMsg.Method == "shutdown" { + cli.Close() + errQueue <- nil + return + } + req.ServiceMethod = reqMsg.Method + req.Seq = counter + if err := cli.WriteRequest(&req, reqMsg.Params); err != nil { + errQueue <- err + return } if err := cli.ReadResponseHeader(&resp); err != nil { errQueue <- err @@ -319,5 +317,4 @@ func ovsdbMessenger(s string, t int, rxQueue <-chan Request, txQueue chan<- Resp } txQueue <- respMsg } - return } diff --git a/database.go b/database.go index f48f3d4..56b82e2 100644 --- a/database.go +++ b/database.go @@ -43,7 +43,7 @@ type OvsDatabase struct { Schema struct { Version string } - connected bool + connected bool //nolint:unused } // Databases - DOCS-TBD diff --git a/database_test.go b/database_test.go index 42dc37f..83ec555 100644 --- a/database_test.go +++ b/database_test.go @@ -32,8 +32,15 @@ func TestListDatabasesMethod(t *testing.T) { t.Fatalf("FAIL: expected to find a single database, but found: %d", len(databases)) } dbName := "Open_vSwitch" - if databases[0] != dbName { - t.Fatalf("FAIL: expected to find '%s' database, but found: %s", dbName, databases[0]) + found := false + for _, db := range databases { + if db == dbName { + found = true + break + } + } + if !found { + t.Fatalf("FAIL: expected to find '%s' database in %v", dbName, databases) } t.Logf("PASS: 'list_dbs' method completed successfully") } diff --git a/echo.go b/echo.go index 4f15c17..0b95746 100644 --- a/echo.go +++ b/echo.go @@ -24,7 +24,7 @@ func (c *Client) Echo(s string) error { method := "echo" js, err := encodeString(s) if err != nil { - fmt.Errorf("'%s' method failed: %v", method, err) + _ = fmt.Errorf("'%s' method failed: %v", method, err) } response, err := c.query(method, js) if err != nil { diff --git a/encoder.go b/encoder.go index 3e24ab7..d2e38fc 100644 --- a/encoder.go +++ b/encoder.go @@ -33,6 +33,7 @@ var methods = map[string]method{ "get_schema": {Name: "get_schema"}, "transact": {Name: "transact"}, "list-commands": {Name: "list-commands"}, + "version": {Name: "version"}, "coverage/show": {Name: "coverage/show"}, "memory/show": {Name: "memory/show"}, "cluster/status": {Name: "cluster/status"}, @@ -56,7 +57,7 @@ func newOvsdbEncoder(w io.Writer) *ovsdbEncoder { // An encodeState encodes JSON into a bytes.Buffer. type encodeState struct { bytes.Buffer // accumulated output - scratch [64]byte + scratch [64]byte //nolint:unused } var encodeStatePool sync.Pool @@ -111,6 +112,7 @@ func (enc *ovsdbEncoder) Encode(v interface{}) error { e.WriteString(s) // e.WriteString("\"Open_vSwitch\",{\"op\":\"select\",\"table\":\"Open_vSwitch\",\"where\":[]}") case "list-commands": + case "version": case "coverage/show": case "memory/show": case "dpif/show": diff --git a/ovn_chassis.go b/ovn_chassis.go index 4d7ecca..711b1fc 100644 --- a/ovn_chassis.go +++ b/ovn_chassis.go @@ -28,9 +28,10 @@ type OvnChassis struct { UUID string Proto string } - Up int - Ports []string - Switches []string + NbCfg int64 // Configuration sequence number from Chassis_Private table (0 if not present) + NbCfgTimestamp int64 // Timestamp from Chassis_Private table (0 if not present) + Ports []string + Switches []string } // GetChassis returns a list of OVN chassis. @@ -134,6 +135,92 @@ func (cli *OvnClient) GetChassis() ([]*OvnChassis, error) { break } } + + query = "SELECT chassis, name, nb_cfg, nb_cfg_timestamp FROM Chassis_Private" + result, err = cli.Database.Southbound.Client.Transact(cli.Database.Southbound.Name, query) + if err != nil { + return chassis, nil + } + + // Create maps for chassis nb_cfg and nb_cfg_timestamp + chassisNbCfgMap := make(map[string]int64) + chassisTimestampMap := make(map[string]int64) + if len(result.Rows) > 0 { + for _, row := range result.Rows { + var chassisUUID string + var chassisName string + var nbCfg int64 + var nbCfgTimestamp int64 + + // Get chassis UUID (reference to Chassis table) + if r, dt, err := row.GetColumnValue("chassis", result.Columns); err == nil && dt == "string" { + chassisUUID = r.(string) + } + + // Get chassis name + if r, dt, err := row.GetColumnValue("name", result.Columns); err == nil && dt == "string" { + chassisName = r.(string) + } + + // Get the nb_cfg + if r, dt, err := row.GetColumnValue("nb_cfg", result.Columns); err == nil { + switch dt { + case "int64": + nbCfg = r.(int64) + case "integer": + // GetColumnValue returns "integer" for float64 values after converting to int64 + nbCfg = r.(int64) + case "float64": + nbCfg = int64(r.(float64)) + case "int": + nbCfg = int64(r.(int)) + } + } + + // Get the nb_cfg_timestamp + if r, dt, err := row.GetColumnValue("nb_cfg_timestamp", result.Columns); err == nil { + switch dt { + case "int64": + nbCfgTimestamp = r.(int64) + case "integer": + // GetColumnValue returns "integer" for float64 values after converting to int64 + nbCfgTimestamp = r.(int64) + case "float64": + nbCfgTimestamp = int64(r.(float64)) + case "int": + nbCfgTimestamp = int64(r.(int)) + } + } + + // Store values by both UUID and name + if chassisUUID != "" { + chassisNbCfgMap[chassisUUID] = nbCfg + chassisTimestampMap[chassisUUID] = nbCfgTimestamp + } + if chassisName != "" { + chassisNbCfgMap[chassisName] = nbCfg + chassisTimestampMap[chassisName] = nbCfgTimestamp + } + } + } + + // Set the NbCfg and NbCfgTimestamp fields for each chassis + // Will be 0 if chassis has no entry in Chassis_Private + for _, c := range chassis { + if nbCfg, exists := chassisNbCfgMap[c.UUID]; exists { + c.NbCfg = nbCfg + } else if nbCfg, exists := chassisNbCfgMap[c.Name]; exists { + c.NbCfg = nbCfg + } + + if timestamp, exists := chassisTimestampMap[c.UUID]; exists { + c.NbCfgTimestamp = timestamp + } else if timestamp, exists := chassisTimestampMap[c.Name]; exists { + c.NbCfgTimestamp = timestamp + } + // If no entry found, NbCfg and NbCfgTimestamp remain 0 (default) + } + return chassis, nil } diff --git a/process.go b/process.go index 31d6cb5..c1de880 100644 --- a/process.go +++ b/process.go @@ -17,7 +17,7 @@ package ovsdb import ( "bufio" "fmt" - "io/ioutil" + "io/ioutil" //nolint:staticcheck "os" "os/user" "strconv" diff --git a/route_filter.go b/route_filter.go index 0552430..11bcaea 100644 --- a/route_filter.go +++ b/route_filter.go @@ -130,7 +130,7 @@ func (rf *RouteFilter) Add(network string) error { } } } - if networkFound == true { + if networkFound { entry := rf.Entries[networkIndex] entry.Exclusions = append(entry.Exclusions, ipNet) } else { diff --git a/schema.go b/schema.go index 49bebae..a726e94 100644 --- a/schema.go +++ b/schema.go @@ -53,7 +53,7 @@ func (c *Client) GetSchema(s string) (Schema, error) { method := "get_schema" js, err := encodeString(s) if err != nil { - fmt.Errorf("'%s' method failed: %v", method, err) + _ = fmt.Errorf("'%s' method failed: %v", method, err) } response, err := c.query(method, js) if err != nil { diff --git a/system.go b/system.go index 79c0791..323aff6 100644 --- a/system.go +++ b/system.go @@ -18,6 +18,8 @@ import ( "bufio" "fmt" "os" + "regexp" + "strings" ) // OvsDataFile stores information about the files related to OVS @@ -46,7 +48,7 @@ type OvsDaemon struct { // GetSystemID TODO func (cli *OvnClient) GetSystemID() error { - systemID, err := getSystemID(cli.Database.Vswitch.File.SystemID.Path) + systemID, err := getSystemID(cli.Database.Vswitch.Client, cli.Database.Vswitch.Name, cli.Database.Vswitch.File.SystemID.Path) if err != nil { return err } @@ -56,7 +58,7 @@ func (cli *OvnClient) GetSystemID() error { // GetSystemID TODO func (cli *OvsClient) GetSystemID() error { - systemID, err := getSystemID(cli.Database.Vswitch.File.SystemID.Path) + systemID, err := getSystemID(cli.Database.Vswitch.Client, cli.Database.Vswitch.Name, cli.Database.Vswitch.File.SystemID.Path) if err != nil { return err } @@ -64,11 +66,40 @@ func (cli *OvsClient) GetSystemID() error { return nil } -func getSystemID(filepath string) (string, error) { +func getSystemID(client *Client, dbName string, filepath string) (string, error) { var systemID string + var dbErr error + + // First, try to query database if client is provided + if client != nil && dbName != "" { + query := fmt.Sprintf("SELECT external_ids FROM %s", dbName) + result, err := client.Transact(dbName, query) + if err == nil && len(result.Rows) > 0 { + col := "external_ids" + rowData, dataType, err := result.Rows[0].GetColumnValue(col, result.Columns) + if err == nil && dataType == "map[string]string" { + externalIDs := rowData.(map[string]string) + if dbSystemID, exists := externalIDs["system-id"]; exists && dbSystemID != "" { + systemID = dbSystemID + if len(systemID) > 253 { + return systemID, fmt.Errorf("system-id is greater than what the exporter currently allows: %d vs 253", len(systemID)) + } + return systemID, nil + } + } + } else if err != nil { + dbErr = err + } + } + + // Fallback to reading from file file, err := os.Open(filepath) if err != nil { - return systemID, err + // If we also had a database error, return both + if dbErr != nil { + return "", fmt.Errorf("failed to get system-id from database (%s) and file (%s)", dbErr, err) + } + return "", err } defer file.Close() scanner := bufio.NewScanner(file) @@ -77,7 +108,10 @@ func getSystemID(filepath string) (string, error) { break } if err := scanner.Err(); err != nil { - return systemID, err + if dbErr != nil { + return "", fmt.Errorf("failed to get system-id from database (%s) and file (%s)", dbErr, err) + } + return "", err } // vswitch.ovsschema does not limit system IDs to a particular length and a common // ID to use is UUID (36 bytes). However, some tools use FQDNs for system-ids which @@ -89,6 +123,99 @@ func getSystemID(filepath string) (string, error) { return systemID, nil } +func getVersionViaAppctl(sock string, timeout int) (string, error) { + cmd := "version" + app, err := NewClient(sock, timeout) + if err != nil { + return "", fmt.Errorf("failed to connect to socket %s: %s", sock, err) + } + r, err := app.query(cmd, nil) + app.Close() + if err != nil { + return "", fmt.Errorf("the '%s' command failed: %s", cmd, err) + } + response := r.String() + if response == "" { + return "", fmt.Errorf("the '%s' command returned no data", cmd) + } + return response, nil +} + +func parseOvsVersion(versionStr string) string { + // Parse version from string like "ovs-vswitchd (Open vSwitch) 3.5.1" + re := regexp.MustCompile(`\(Open vSwitch\)\s+([\d.]+)`) + matches := re.FindStringSubmatch(versionStr) + if len(matches) > 1 { + return matches[1] + } + // Fallback: return the whole string + return strings.TrimSpace(versionStr) +} + +func getSystemInfoFromOS() (string, string) { + // Read /etc/os-release to get system type and version + file, err := os.Open("/etc/os-release") + if err != nil { + return "", "" + } + defer file.Close() + + var systemType, systemVersion string + scanner := bufio.NewScanner(file) + for scanner.Scan() { + line := scanner.Text() + if strings.HasPrefix(line, "ID=") { + systemType = strings.Trim(strings.TrimPrefix(line, "ID="), "\"") + } else if strings.HasPrefix(line, "VERSION_ID=") { + systemVersion = strings.Trim(strings.TrimPrefix(line, "VERSION_ID="), "\"") + } + } + return systemType, systemVersion +} + +func populateVersionFromAppctl(systemInfo map[string]string, sock string, timeout int, schema *Schema) { + // Get OVS version via ovs-appctl if missing from DB + if val, exists := systemInfo["ovs_version"]; !exists || val == "" { + versionStr, err := getVersionViaAppctl(sock, timeout) + if err == nil { + systemInfo["ovs_version"] = parseOvsVersion(versionStr) + } else { + systemInfo["ovs_version"] = "unknown" + } + } + + // Get DB version from schema if missing from DB + if val, exists := systemInfo["db_version"]; !exists || val == "" { + if schema != nil && schema.Version != "" { + systemInfo["db_version"] = schema.Version + } else { + systemInfo["db_version"] = "unknown" + } + } + + // Get system type and version from /etc/os-release if missing from DB + if val, exists := systemInfo["system_type"]; !exists || val == "" { + systemType, systemVersion := getSystemInfoFromOS() + if systemType != "" { + systemInfo["system_type"] = systemType + } else { + systemInfo["system_type"] = "unknown" + } + if systemVersion != "" { + systemInfo["system_version"] = systemVersion + } else { + systemInfo["system_version"] = "unknown" + } + } else if val, exists := systemInfo["system_version"]; !exists || val == "" { + _, systemVersion := getSystemInfoFromOS() + if systemVersion != "" { + systemInfo["system_version"] = systemVersion + } else { + systemInfo["system_version"] = "unknown" + } + } +} + func parseSystemInfo(systemID string, result Result) (map[string]string, error) { systemInfo := make(map[string]string) for _, row := range result.Rows { @@ -107,12 +234,21 @@ func parseSystemInfo(systemID string, result Result) (map[string]string, error) if err != nil { return systemInfo, fmt.Errorf("parsing '%s' failed: %s", col, err) } - if dataType != "string" { + switch dataType { + case "string": + systemInfo[col] = rowData.(string) + case "[]string": + arr := rowData.([]string) + if len(arr) > 0 { + systemInfo[col] = arr[0] + } else { + systemInfo[col] = "" + } + default: return systemInfo, fmt.Errorf("data type '%s' for '%s' column is unexpected in this context", dataType, col) } - systemInfo[col] = rowData.(string) } - break + break //nolint:staticcheck } if dbSystemID, exists := systemInfo["system-id"]; exists { if dbSystemID != systemID { @@ -121,9 +257,14 @@ func parseSystemInfo(systemID string, result Result) (map[string]string, error) } else { return systemInfo, fmt.Errorf("no 'system-id' found") } - requiredKeys := []string{"rundir", "hostname", "ovs_version", "db_version", "system_type", "system_version"} + // Set defaults for optional keys that may not be in external_ids + if _, exists := systemInfo["rundir"]; !exists { + systemInfo["rundir"] = "/var/run/openvswitch" + } + // Only hostname is truly required + requiredKeys := []string{"hostname"} for _, key := range requiredKeys { - if _, exists := systemInfo[key]; exists == false { + if _, exists := systemInfo[key]; !exists { return systemInfo, fmt.Errorf("no mandatory '%s' found", key) } } @@ -133,11 +274,12 @@ func parseSystemInfo(systemID string, result Result) (map[string]string, error) // GetSystemInfo returns a hash containing system information, e.g. `system_id` // associated with the Open_vSwitch database. func (cli *OvnClient) GetSystemInfo() error { - systemID, err := getSystemID(cli.Database.Vswitch.File.SystemID.Path) + // Get system-id (tries database first, falls back to file) + systemID, err := getSystemID(cli.Database.Vswitch.Client, cli.Database.Vswitch.Name, cli.Database.Vswitch.File.SystemID.Path) if err != nil { return err } - cli.System.ID = systemID + query := fmt.Sprintf("SELECT ovs_version, db_version, system_type, system_version, external_ids FROM %s", cli.Database.Vswitch.Name) result, err := cli.Database.Vswitch.Client.Transact(cli.Database.Vswitch.Name, query) if err != nil { @@ -150,6 +292,18 @@ func (cli *OvnClient) GetSystemInfo() error { if err != nil { return fmt.Errorf("The '%s' query returned results but erred: %s", query, err) } + // Get schema for db_version + schema, _ := cli.Database.Vswitch.Client.GetSchema(cli.Database.Vswitch.Name) + // Ensure PID is read and socket path is updated before using control socket + if cli.Database.Vswitch.Process.ID == 0 { + p, pidErr := getProcessInfoFromFile(cli.Database.Vswitch.File.Pid.Path) + if pidErr == nil { + cli.Database.Vswitch.Process = p + } + } + cli.updateRefs() + // Query version information via ovs-appctl for fields not in DB (OVS 3.x+) + populateVersionFromAppctl(systemInfo, cli.Database.Vswitch.Socket.Control, cli.Timeout, &schema) cli.System.ID = systemInfo["system-id"] cli.System.RunDir = systemInfo["rundir"] cli.System.Hostname = systemInfo["hostname"] @@ -163,11 +317,12 @@ func (cli *OvnClient) GetSystemInfo() error { // GetSystemInfo returns a hash containing system information, e.g. `system_id` // associated with the Open_vSwitch database. func (cli *OvsClient) GetSystemInfo() error { - systemID, err := getSystemID(cli.Database.Vswitch.File.SystemID.Path) + // Get system-id (tries database first, falls back to file) + systemID, err := getSystemID(cli.Database.Vswitch.Client, cli.Database.Vswitch.Name, cli.Database.Vswitch.File.SystemID.Path) if err != nil { return err } - cli.System.ID = systemID + query := fmt.Sprintf("SELECT ovs_version, db_version, system_type, system_version, external_ids FROM %s", cli.Database.Vswitch.Name) result, err := cli.Database.Vswitch.Client.Transact(cli.Database.Vswitch.Name, query) if err != nil { @@ -180,6 +335,18 @@ func (cli *OvsClient) GetSystemInfo() error { if err != nil { return fmt.Errorf("The '%s' query returned results but erred: %s", query, err) } + // Get schema for db_version + schema, _ := cli.Database.Vswitch.Client.GetSchema(cli.Database.Vswitch.Name) + // Ensure PID is read and socket path is updated before using control socket + if cli.Database.Vswitch.Process.ID == 0 { + p, pidErr := getProcessInfoFromFile(cli.Database.Vswitch.File.Pid.Path) + if pidErr == nil { + cli.Database.Vswitch.Process = p + } + } + cli.updateRefs() + // Query version information via ovs-appctl for fields not in DB (OVS 3.x+) + populateVersionFromAppctl(systemInfo, cli.Database.Vswitch.Socket.Control, cli.Timeout, &schema) cli.System.ID = systemInfo["system-id"] cli.System.RunDir = systemInfo["rundir"] cli.System.Hostname = systemInfo["hostname"] diff --git a/system_test.go b/system_test.go new file mode 100644 index 0000000..38cd79a --- /dev/null +++ b/system_test.go @@ -0,0 +1,217 @@ +// Copyright 2018 Paul Greenberg (greenpau@outlook.com) +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ovsdb + +import ( + "os" + "testing" +) + +func TestParseOvsVersion(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "OVS 3.5.1 format", + input: "ovs-vswitchd (Open vSwitch) 3.5.1", + expected: "3.5.1", + }, + { + name: "OVS 2.17.0 format", + input: "ovs-vswitchd (Open vSwitch) 2.17.0", + expected: "2.17.0", + }, + { + name: "OVS with patch version", + input: "ovs-vswitchd (Open vSwitch) 2.17.1", + expected: "2.17.1", + }, + { + name: "Malformed version string", + input: "some random text", + expected: "some random text", + }, + { + name: "Empty string", + input: "", + expected: "", + }, + { + name: "Version with extra whitespace", + input: "ovs-vswitchd (Open vSwitch) 3.5.1 ", + expected: "3.5.1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := parseOvsVersion(tt.input) + if result != tt.expected { + t.Errorf("parseOvsVersion(%q) = %q, expected %q", tt.input, result, tt.expected) + } + }) + } +} + +func TestGetSystemInfoFromOS(t *testing.T) { + // This test checks if the function can read /etc/os-release + // The actual values will vary depending on the system + systemType, systemVersion := getSystemInfoFromOS() + + // On a system with /etc/os-release, we should get non-empty values + if _, err := os.Stat("/etc/os-release"); err == nil { + if systemType == "" { + t.Error("Expected non-empty system type when /etc/os-release exists") + } + if systemVersion == "" { + t.Error("Expected non-empty system version when /etc/os-release exists") + } + t.Logf("Detected system: %s %s", systemType, systemVersion) + } else { + // If /etc/os-release doesn't exist, both should be empty + if systemType != "" || systemVersion != "" { + t.Error("Expected empty values when /etc/os-release doesn't exist") + } + } +} + +func TestPopulateVersionFromAppctl(t *testing.T) { + tests := []struct { + name string + systemInfo map[string]string + schema *Schema + expectOvsVer bool + expectDbVer bool + expectSysTyp bool + expectSysVer bool + }{ + { + name: "All fields present in systemInfo", + systemInfo: map[string]string{ + "ovs_version": "2.17.0", + "db_version": "7.16.1", + "system_type": "ubuntu", + "system_version": "20.04", + }, + schema: &Schema{Version: "7.16.1"}, + expectOvsVer: false, // Should not query when already present + expectDbVer: false, + expectSysTyp: false, + expectSysVer: false, + }, + { + name: "All fields missing", + systemInfo: map[string]string{}, + schema: &Schema{Version: "7.16.1"}, + expectOvsVer: true, // Should populate with "unknown" or queried value + expectDbVer: true, // Should populate from schema + expectSysTyp: true, // Should populate from OS + expectSysVer: true, + }, + { + name: "Only ovs_version missing", + systemInfo: map[string]string{ + "db_version": "7.16.1", + "system_type": "ubuntu", + "system_version": "20.04", + }, + schema: &Schema{Version: "7.16.1"}, + expectOvsVer: true, + expectDbVer: false, + expectSysTyp: false, + expectSysVer: false, + }, + { + name: "Empty values treated as missing", + systemInfo: map[string]string{ + "ovs_version": "", + "db_version": "", + "system_type": "", + "system_version": "", + }, + schema: &Schema{Version: "7.16.1"}, + expectOvsVer: true, + expectDbVer: true, + expectSysTyp: true, + expectSysVer: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Note: We can't test the actual appctl query without a running OVS + // but we can test the logic with a fake socket that will fail + populateVersionFromAppctl(tt.systemInfo, "/nonexistent/socket", 1, tt.schema) + + // Check that fields were populated (either with real values or "unknown") + if tt.expectOvsVer { + if _, exists := tt.systemInfo["ovs_version"]; !exists { + t.Error("Expected ovs_version to be populated") + } + } + if tt.expectDbVer { + if val, exists := tt.systemInfo["db_version"]; !exists { + t.Error("Expected db_version to be populated") + } else if tt.schema != nil && tt.schema.Version != "" && val != tt.schema.Version { + t.Errorf("Expected db_version to be %q from schema, got %q", tt.schema.Version, val) + } + } + if tt.expectSysTyp { + if _, exists := tt.systemInfo["system_type"]; !exists { + t.Error("Expected system_type to be populated") + } + } + if tt.expectSysVer { + if _, exists := tt.systemInfo["system_version"]; !exists { + t.Error("Expected system_version to be populated") + } + } + + t.Logf("systemInfo after populate: %+v", tt.systemInfo) + }) + } +} + +func TestPopulateVersionFromAppctlWithSchema(t *testing.T) { + systemInfo := map[string]string{} + schema := &Schema{Version: "7.16.1"} + + // Populate with a fake socket (will fail to connect, but should still populate from schema) + populateVersionFromAppctl(systemInfo, "/nonexistent/socket", 1, schema) + + // db_version should be populated from schema + if dbVersion, exists := systemInfo["db_version"]; !exists { + t.Error("Expected db_version to be populated") + } else if dbVersion != "7.16.1" { + t.Errorf("Expected db_version to be '7.16.1', got %q", dbVersion) + } + + // ovs_version should be "unknown" since socket doesn't exist + if ovsVersion, exists := systemInfo["ovs_version"]; !exists { + t.Error("Expected ovs_version to be populated") + } else if ovsVersion != "unknown" { + t.Errorf("Expected ovs_version to be 'unknown' when socket fails, got %q", ovsVersion) + } + + // system_type and system_version should be populated from OS or "unknown" + if _, exists := systemInfo["system_type"]; !exists { + t.Error("Expected system_type to be populated") + } + if _, exists := systemInfo["system_version"]; !exists { + t.Error("Expected system_version to be populated") + } +} diff --git a/transact_test.go b/transact_test.go index 284764a..83891f4 100644 --- a/transact_test.go +++ b/transact_test.go @@ -66,7 +66,6 @@ func TestTransactMethod(t *testing.T) { t.Logf("FAIL: Test %d: database '%s', query '%s', row: %d, unsupported data type '%s' for value: %v", i, test.db, test.query, j, valueType, value) testFailed = true testsFailed++ - break } if err != nil { if !test.shouldFail { diff --git a/util.go b/util.go index e7911a4..7d65f9d 100644 --- a/util.go +++ b/util.go @@ -36,7 +36,7 @@ func encodeString(s string) (string, error) { if err != nil { return "", err } - return string(b[:len(b)]), nil + return string(b[:]), nil } func indentAnalysis(s string) int { @@ -53,7 +53,7 @@ func indentAnalysis(s string) int { func dedupInts(arr []int) []int { newArr := []int{} - sort.Sort(sort.IntSlice(arr)) + sort.Ints(sort.IntSlice(arr)) var prevItem int for i, item := range arr { if i == 0 { @@ -72,7 +72,7 @@ func dedupInts(arr []int) []int { func indentDepthAnalysis(arr []int) (map[int]int, error) { depth := make(map[int]int) - sort.Sort(sort.IntSlice(arr)) + sort.Ints(sort.IntSlice(arr)) indents := dedupInts(arr) for i, indent := range indents { depth[indent] = i