Cut release 1.2.1-1

debian/changelog

@@ -1,9 +1,41 @@
-govpp-snmp-agentx (1.1.6-1) bookworm; urgency=medium
+govpp-snmp-agentx (1.2.1-1) bookworm; urgency=medium
+
+  * Fix OID visibility bug after go-agentx 0.3.0 upgrade
+  * Use reflection to clear handler contents instead of creating new handlers
+  * Simplify AgentX session creation and registration logic
+  * Add proper session cleanup method (Close())
+  * Improve error handling in AgentX interactions
+  * Code cleanup and maintainability improvements
+
+ -- Pim van Pelt <pim@ipng.ch>  Fri, 22 Nov 2024 00:00:00 +0000
+
+govpp-snmp-agentx (1.2.0-1) bookworm; urgency=medium
+
+  * Update go-agentx dependency from 0.2.1 to 0.3.0 to fix compilation issues
+  * Adapt to breaking changes in go-agentx Session API (now requires nameOID, name, handler)
+  * Update Client configuration to use dial options (WithTimeout, WithReconnectInterval)
+  * Remove access to unexported Session.Handler field (API change)
+  * NOTE: This version fixes compilation broken in 1.1.6-1 and 1.1.7-1
+
+ -- Pim van Pelt <pim@ipng.ch>  Wed, 20 Nov 2024 00:00:00 +0000
+
+govpp-snmp-agentx (1.1.7-1) bookworm; urgency=critical
+
+  * Refactor VPPClient constructor to use idiomatic Go patterns
+  * Remove redundant NewVPPClient() constructor function
+  * Update all code to use direct struct initialization (&VPPClient{})
+  * Improve code maintainability and follow Go best practices
+  * WARNING: This version is BROKEN due to go-agentx 0.2.1 incompatibility
+
+ -- Pim van Pelt <pim@ipng.ch>  Fri, 15 Nov 2024 00:00:00 +0000
+
+govpp-snmp-agentx (1.1.6-1) bookworm; urgency=critical
 
   * Replace forked go-agentx dependency with upstream github.com/posteo/go-agentx
   * Remove local src/go-agentx/ directory and use official upstream package
   * Upgrade to go-agentx v0.2.1 from official GitHub repository
   * Improve dependency management and reduce maintenance overhead
+  * WARNING: This version is BROKEN due to go-agentx 0.2.1 incompatibility
 
  -- Pim van Pelt <pim@ipng.ch>  Fri, 08 Nov 2024 00:00:00 +0000
 

src/agentx/agentx.go

@@ -20,29 +20,27 @@ var (
 
 // StartAgentXRoutine initializes the AgentX client and registers the interface MIB
 func StartAgentXRoutine(interfaceMIB *ifmib.InterfaceMIB) error {
-	var network, address string
+	// Determine network type based on address format
+	network := "tcp"
 	if strings.HasPrefix(*AgentXAddr, "/") {
 		network = "unix"
-		address = *AgentXAddr
-	} else {
-		network = "tcp"
-		address = *AgentXAddr
 	}
 
-	logger.Debugf("Connecting to AgentX at %s://%s", network, address)
+	logger.Debugf("Connecting to AgentX at %s://%s", network, *AgentXAddr)
 
-	client, err := agentx.Dial(network, address)
+	client, err := agentx.Dial(network, *AgentXAddr,
+		agentx.WithTimeout(1*time.Minute),
+		agentx.WithReconnectInterval(1*time.Second),
+	)
 	if err != nil {
 		return err
 	}
-	client.Timeout = 1 * time.Minute
-	client.ReconnectInterval = 1 * time.Second
 
 	// Register the interface MIB with the AgentX client
 	if err := interfaceMIB.RegisterWithClient(client); err != nil {
 		return err
 	}
 
-	logger.Printf("Successfully registered with AgentX at %s://%s", network, address)
+	logger.Printf("Successfully registered with AgentX at %s://%s", network, *AgentXAddr)
 	return nil
-}
+}

src/agentx/agentx_test.go

@@ -22,7 +22,7 @@ func TestAgentXAddrFlagParsing(t *testing.T) {
 	// Test Unix socket path
 	testAddr := "/var/run/test.sock"
 	*AgentXAddr = testAddr
-	
+
 	if *AgentXAddr != testAddr {
 		t.Errorf("Expected AgentX address to be '%s', got '%s'", testAddr, *AgentXAddr)
 	}
@@ -30,7 +30,7 @@ func TestAgentXAddrFlagParsing(t *testing.T) {
 	// Test TCP address
 	testAddr = "192.168.1.1:705"
 	*AgentXAddr = testAddr
-	
+
 	if *AgentXAddr != testAddr {
 		t.Errorf("Expected AgentX address to be '%s', got '%s'", testAddr, *AgentXAddr)
 	}
@@ -43,12 +43,12 @@ func TestFlagRegistration(t *testing.T) {
 		t.Error("Expected agentx.addr flag to be registered")
 		return
 	}
-	
+
 	if f.DefValue != "localhost:705" {
 		t.Errorf("Expected flag default value to be 'localhost:705', got '%s'", f.DefValue)
 	}
-	
+
 	if f.Usage != "Address to connect to (hostname:port or Unix socket path)" {
 		t.Errorf("Unexpected flag usage string: %s", f.Usage)
 	}
-}
+}

src/config/config.go

@@ -5,4 +5,4 @@ package config
 // Global configuration variables
 var (
 	Debug bool
-)
+)

src/config/config_test.go

@@ -27,4 +27,4 @@ func TestDebugFlagSet(t *testing.T) {
 	if Debug != false {
 		t.Errorf("Expected Debug to be false after setting, got %v", Debug)
 	}
-}
+}

src/go.mod

@@ -3,7 +3,7 @@ module govpp-snmp-agentx
 go 1.23.8
 
 require (
-	github.com/posteo/go-agentx v0.2.1
+	github.com/posteo/go-agentx v0.3.0
 	go.fd.io/govpp v0.12.0
 	gopkg.in/yaml.v3 v3.0.1
 )

src/go.sum

@@ -18,12 +18,11 @@ github.com/onsi/gomega v1.37.0 h1:CdEG8g0S133B4OswTDC/5XPSzE1OeP29QOioj2PID2Y=
 github.com/onsi/gomega v1.37.0/go.mod h1:8D9+Txp43QWKhM24yyOBEdpkzN8FvJyAwecBgsU4KU0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
-github.com/posteo/go-agentx v0.2.1 h1:HO0zO/+GosL0RYEodu7KNH9OF/rL5bJbhXNP1z3hkT8=
-github.com/posteo/go-agentx v0.2.1/go.mod h1:EUR75CfAEDstQn3WqCs26Ti64EsggaSXDk2dgxPQ5TI=
+github.com/posteo/go-agentx v0.3.0 h1:Mqu0qzPHxbyZF3+fKwN2vjW49t6TPPgivjjplcuouNw=
+github.com/posteo/go-agentx v0.3.0/go.mod h1:YCWL7bzLlpSNeU9vnfEg1pdlllDs1v2mz+pRcg21CUg=
 github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
 github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 go.fd.io/govpp v0.12.0 h1:5HnMzsKHSFdxglsFyEhR0g+CzncWiLYXG2NDYgNUrnE=

src/ifmib/ifmib.go

@@ -5,6 +5,7 @@ package ifmib
 import (
 	"fmt"
 	"os"
+	"reflect"
 	"sync"
 	"time"
 
@@ -161,14 +162,32 @@ func (m *InterfaceMIB) UpdateInterfaceDetails(details []vpp.InterfaceDetails) {
 	logger.Debugf("Interface details updated for %d interfaces", len(details))
 }
 
+func (m *InterfaceMIB) clearHandler() {
+	// Use reflection to access and clear the private fields of ListHandler
+	// since it doesn't have a Clear() method
+	handlerValue := reflect.ValueOf(m.handler).Elem()
+
+	oidsField := handlerValue.FieldByName("oids")
+	itemsField := handlerValue.FieldByName("items")
+
+	if oidsField.IsValid() && oidsField.CanSet() {
+		oidsField.Set(reflect.Zero(oidsField.Type()))
+	}
+
+	if itemsField.IsValid() && itemsField.CanSet() {
+		itemsField.Set(reflect.Zero(itemsField.Type()))
+	}
+}
+
 func (m *InterfaceMIB) UpdateStats(interfaceStats *api.InterfaceStats) {
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 
 	logger.Debugf("Updating IF-MIB with %d interfaces", len(interfaceStats.Interfaces))
 
-	// Clear existing entries
-	m.handler = &agentx.ListHandler{}
+	// Clear existing entries while preserving the handler reference
+	// Since go-agentx 0.3.0 binds handlers to sessions at creation time
+	m.clearHandler()
 	m.stats = make(map[uint32]*api.InterfaceCounters)
 
 	// Add new entries
@@ -178,15 +197,7 @@ func (m *InterfaceMIB) UpdateStats(interfaceStats *api.InterfaceStats) {
 		m.addInterfaceToMIB(&iface)
 	}
 
-	// Update both sessions with the new handler
-	if m.ifEntrySession != nil {
-		m.ifEntrySession.Handler = m.handler
-	}
-	if m.ifXTableSession != nil {
-		m.ifXTableSession.Handler = m.handler
-		logger.Printf("Updated session handlers with new IF-MIB data for %d interfaces", len(m.stats))
-	}
-
+	logger.Printf("Updated IF-MIB data for %d interfaces", len(m.stats))
 	logger.Debugf("IF-MIB now contains %d interfaces", len(m.stats))
 }
 
@@ -456,41 +467,55 @@ func (m *InterfaceMIB) addIfXTable(iface *api.InterfaceCounters, idx int) {
 	}
 }
 
+func (m *InterfaceMIB) createAndRegisterSession(client *agentx.Client, oid, name string) (*agentx.Session, error) {
+	session, err := client.Session(value.MustParseOID(oid), name, m.handler)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create %s session: %v", name, err)
+	}
+
+	err = session.Register(127, value.MustParseOID(oid))
+	if err != nil {
+		session.Close()
+		return nil, fmt.Errorf("failed to register %s: %v", name, err)
+	}
+
+	return session, nil
+}
+
 func (m *InterfaceMIB) RegisterWithClient(client *agentx.Client) error {
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 
-	// Create separate sessions for each MIB
-	ifEntrySession, err := client.Session()
+	// Create and register sessions
+	ifEntrySession, err := m.createAndRegisterSession(client, ifEntryOID, "ifEntry")
 	if err != nil {
-		return fmt.Errorf("failed to create ifEntry session: %v", err)
+		return err
 	}
 
-	ifXTableSession, err := client.Session()
+	ifXTableSession, err := m.createAndRegisterSession(client, ifXTableOID, "ifXTable")
 	if err != nil {
-		return fmt.Errorf("failed to create ifXTable session: %v", err)
+		ifEntrySession.Close()
+		return err
 	}
 
 	m.ifEntrySession = ifEntrySession
 	m.ifXTableSession = ifXTableSession
 
-	// Set handlers for both sessions
-	ifEntrySession.Handler = m.handler
-	ifXTableSession.Handler = m.handler
-
-	// Register the classic ifEntry
-	err = ifEntrySession.Register(127, value.MustParseOID(ifEntryOID))
-	if err != nil {
-		return fmt.Errorf("failed to register ifEntry: %v", err)
-	}
-
-	// Register the extended ifXTable
-	err = ifXTableSession.Register(127, value.MustParseOID(ifXTableOID))
-	if err != nil {
-		return fmt.Errorf("failed to register ifXTable: %v", err)
-	}
-
-	logger.Debugf("Registered IF-MIB ifEntry at OID %s", ifEntryOID)
-	logger.Debugf("Registered IF-MIB ifXTable at OID %s", ifXTableOID)
+	logger.Debugf("Registered IF-MIB sessions: ifEntry (%s) and ifXTable (%s)", ifEntryOID, ifXTableOID)
 	return nil
 }
+
+// Close cleans up AgentX sessions
+func (m *InterfaceMIB) Close() {
+	m.mutex.Lock()
+	defer m.mutex.Unlock()
+
+	if m.ifEntrySession != nil {
+		m.ifEntrySession.Close()
+		m.ifEntrySession = nil
+	}
+	if m.ifXTableSession != nil {
+		m.ifXTableSession.Close()
+		m.ifXTableSession = nil
+	}
+}

src/logger/logger.go

@@ -6,7 +6,7 @@ import (
 	"fmt"
 	"path/filepath"
 	"runtime"
-	
+
 	"govpp-snmp-agentx/config"
 )
 
@@ -16,15 +16,15 @@ func getCallerInfo() string {
 	if !ok {
 		return "unknown:unknown"
 	}
-	
+
 	fn := runtime.FuncForPC(pc)
 	if fn == nil {
 		return "unknown:unknown"
 	}
-	
+
 	funcName := filepath.Base(fn.Name())
 	fileName := filepath.Base(file)
-	
+
 	return fmt.Sprintf("%s:%s", fileName, funcName)
 }
 
@@ -49,4 +49,4 @@ func Debugf(format string, args ...interface{}) {
 // Sync flushes any buffered log entries (no-op for fmt.Println)
 func Sync() {
 	// No buffering with fmt.Println, so this is a no-op
-}
+}

src/logger/logger_test.go

@@ -33,11 +33,11 @@ func TestPrintf(t *testing.T) {
 	if !strings.HasPrefix(output, "INFO ") {
 		t.Errorf("Expected output to start with 'INFO ', got: %s", output)
 	}
-	
+
 	if !strings.Contains(output, "logger_test.go:logger.TestPrintf") {
 		t.Errorf("Expected output to contain caller info, got: %s", output)
 	}
-	
+
 	if !strings.Contains(output, "test message: hello") {
 		t.Errorf("Expected output to contain message, got: %s", output)
 	}
@@ -71,7 +71,7 @@ func TestDebugfWithDebugEnabled(t *testing.T) {
 	if !strings.HasPrefix(output, "DEBUG ") {
 		t.Errorf("Expected output to start with 'DEBUG ', got: %s", output)
 	}
-	
+
 	if !strings.Contains(output, "debug message: test") {
 		t.Errorf("Expected output to contain message, got: %s", output)
 	}
@@ -110,4 +110,4 @@ func TestDebugfWithDebugDisabled(t *testing.T) {
 func TestSync(t *testing.T) {
 	// Test that Sync doesn't panic (it's a no-op now)
 	Sync()
-}
+}

src/main.go

@@ -16,7 +16,7 @@ import (
 	"govpp-snmp-agentx/vpp"
 )
 
-const Version = "1.1.6-1"
+const Version = "1.2.1-1"
 
 func main() {
 	debug := flag.Bool("debug", false, "Enable debug logging")
@@ -46,7 +46,7 @@ func main() {
 	}
 
 	// Create VPP client and managers
-	vppClient := vpp.NewVPPClient()
+	vppClient := &vpp.VPPClient{}
 	interfaceManager := vpp.NewInterfaceManager(vppClient)
 	statsManager := vpp.NewStatsManager(vppClient)
 

src/vpp/vpp.go

@@ -29,11 +29,6 @@ type VPPClient struct {
 	connected bool
 }
 
-// NewVPPClient creates a new VPP client instance
-func NewVPPClient() *VPPClient {
-	return &VPPClient{}
-}
-
 // Connect establishes connections to both VPP API and Stats sockets
 func (c *VPPClient) Connect() error {
 	logger.Debugf("Connecting to VPP (API: %s, Stats: %s)", *ApiAddr, *StatsAddr)

src/vpp/vpp_iface_test.go

@@ -10,7 +10,7 @@ import (
 )
 
 func TestNewInterfaceManager(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	if manager == nil {
@@ -36,7 +36,7 @@ func TestNewInterfaceManager(t *testing.T) {
 }
 
 func TestInterfaceManagerSetEventCallback(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	var callbackCalled bool
@@ -82,7 +82,7 @@ func TestInterfaceManagerSetEventCallback(t *testing.T) {
 }
 
 func TestInterfaceManagerGetAllInterfaceDetailsWithoutConnection(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	_, err := manager.GetAllInterfaceDetails()
@@ -101,7 +101,7 @@ func TestInterfaceManagerGetAllInterfaceDetailsWithoutConnection(t *testing.T) {
 }
 
 func TestInterfaceManagerStartEventWatcherWithoutConnection(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	err := manager.StartEventWatcher()
@@ -120,7 +120,7 @@ func TestInterfaceManagerStartEventWatcherWithoutConnection(t *testing.T) {
 }
 
 func TestInterfaceManagerHandleInterfaceEventWithoutCallback(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	// Should not panic when callback is nil
@@ -128,7 +128,7 @@ func TestInterfaceManagerHandleInterfaceEventWithoutCallback(t *testing.T) {
 }
 
 func TestInterfaceManagerInitializeEventWatchingWithoutConnection(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	err := manager.InitializeEventWatching()
@@ -220,7 +220,7 @@ func TestInterfaceEventCallback(t *testing.T) {
 }
 
 func TestInterfaceManagerStartStopEventMonitoring(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	if manager.running {
@@ -248,7 +248,7 @@ func TestInterfaceManagerStartStopEventMonitoring(t *testing.T) {
 }
 
 func TestInterfaceManagerEventMonitoringWithConnectionChanges(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewInterfaceManager(client)
 
 	// Set a callback to track calls

src/vpp/vpp_stats_test.go

@@ -10,7 +10,7 @@ import (
 )
 
 func TestNewStatsManager(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewStatsManager(client)
 
 	if manager == nil {
@@ -36,7 +36,7 @@ func TestNewStatsManager(t *testing.T) {
 }
 
 func TestStatsManagerSetStatsCallback(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewStatsManager(client)
 
 	var callbackCalled bool
@@ -85,7 +85,7 @@ func TestStatsManagerSetStatsCallback(t *testing.T) {
 }
 
 func TestStatsManagerSetPeriod(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewStatsManager(client)
 
 	newPeriod := 5 * time.Second
@@ -97,7 +97,7 @@ func TestStatsManagerSetPeriod(t *testing.T) {
 }
 
 func TestStatsManagerStartStopStatsRoutine(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewStatsManager(client)
 
 	if manager.running {
@@ -125,7 +125,7 @@ func TestStatsManagerStartStopStatsRoutine(t *testing.T) {
 }
 
 func TestStatsManagerGetInterfaceStatsWithoutConnection(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewStatsManager(client)
 
 	_, err := manager.GetInterfaceStats()
@@ -206,7 +206,7 @@ func TestStatsCallback(t *testing.T) {
 }
 
 func TestStatsManagerQueryAndReportStatsWithoutConnection(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewStatsManager(client)
 
 	// Should return false when not connected
@@ -216,7 +216,7 @@ func TestStatsManagerQueryAndReportStatsWithoutConnection(t *testing.T) {
 }
 
 func TestStatsManagerWithShortPeriod(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 	manager := NewStatsManager(client)
 
 	// Set a very short period for testing

src/vpp/vpp_test.go

@@ -7,7 +7,7 @@ import (
 )
 
 func TestNewVPPClient(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 
 	if client == nil {
 		t.Fatal("NewVPPClient() returned nil")
@@ -27,7 +27,7 @@ func TestNewVPPClient(t *testing.T) {
 }
 
 func TestVPPClientDisconnect(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 
 	// Should be safe to call disconnect on unconnected client
 	client.Disconnect()
@@ -38,7 +38,7 @@ func TestVPPClientDisconnect(t *testing.T) {
 }
 
 func TestVPPClientNewAPIChannelWithoutConnection(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 
 	_, err := client.NewAPIChannel()
 	if err == nil {
@@ -56,7 +56,7 @@ func TestVPPClientNewAPIChannelWithoutConnection(t *testing.T) {
 }
 
 func TestVPPClientCheckLivenessWithoutConnection(t *testing.T) {
-	client := NewVPPClient()
+	client := &VPPClient{}
 
 	if client.CheckLiveness() {
 		t.Error("CheckLiveness() should return false when not connected")
@@ -86,7 +86,7 @@ func TestVPPClientConnectWithInvalidPaths(t *testing.T) {
 		*StatsAddr = origStatsAddr
 	}()
 
-	client := NewVPPClient()
+	client := &VPPClient{}
 	err := client.Connect()
 
 	if err == nil {