Fix CNS regression (Azure#489)

log.SetTarget creates the log file under log directory using golang os package. Whenever code sets the log directory, it needed to call SetTarget to create the actual log file under that directory. In the recent logger changes, InitLogger by default set the log directory to the current folder. This created the log file in the current folder. The code then set the log directory to a different location without a subsequent call to log.SetTarget. This resulted into the logger to not find the actual log file in the set log directory.

This fix updates the logger InitLogger function to accept the log directory to create the file in correct log directory. To avoid having such issue, this fix also combines the function calls to set log directory and set target into a single function. This prevents any out of order calls resulting into such issue.

aitelemetry/telemetrywrapper_test.go

@@ -22,10 +22,9 @@ var (
 )
 
 func TestMain(m *testing.M) {
-	log.SetLogDirectory("/var/log/")
 	log.SetName("testaitelemetry")
 	log.SetLevel(log.LevelInfo)
-	err := log.SetTarget(log.TargetLogfile)
+	err := log.SetTargetLogDirectory(log.TargetLogfile, "/var/log/")
 	if err == nil {
 		fmt.Printf("TestST LogDir configuration succeeded\n")
 	}

cni/ipam/plugin/main.go

@@ -24,10 +24,11 @@ var version string
 func main() {
 	var config common.PluginConfig
 	config.Version = version
+	logDirectory := "" // Sets the current location as log directory
 
 	log.SetName(name)
 	log.SetLevel(log.LevelInfo)
-	if err := log.SetTarget(log.TargetLogfile); err != nil {
+	if err := log.SetTargetLogDirectory(log.TargetLogfile, logDirectory); err != nil {
 		fmt.Printf("Failed to setup cni logging: %v\n", err)
 		return
 	}

cni/network/plugin/main.go

@@ -139,14 +139,15 @@ func main() {
 	}
 
 	var (
-		config    common.PluginConfig
-		err       error
-		cnimetric telemetry.AIMetric
+		config       common.PluginConfig
+		err          error
+		cnimetric    telemetry.AIMetric
+		logDirectory string // This sets empty string i.e. current location
 	)
 
 	log.SetName(name)
 	log.SetLevel(log.LevelInfo)
-	if err = log.SetTarget(log.TargetLogfile); err != nil {
+	if err = log.SetTargetLogDirectory(log.TargetLogfile, logDirectory); err != nil {
 		fmt.Printf("Failed to setup cni logging: %v\n", err)
 		return
 	}

cni/telemetry/service/telemetrymain.go

@@ -129,11 +129,7 @@ func main() {
 
 	log.SetName(azureVnetTelemetry)
 	log.SetLevel(logLevel)
-	if logDirectory != "" {
-		log.SetLogDirectory(logDirectory)
-	}
-
-	err = log.SetTarget(logTarget)
+	err = log.SetTargetLogDirectory(logTarget, logDirectory)
 	if err != nil {
 		fmt.Printf("Failed to configure logging: %v\n", err)
 		return

cnm/plugin/main.go

@@ -166,9 +166,10 @@ func main() {
 	}
 
 	// Create logging provider.
+	logDirectory := "" // Sets the current location as log directory
 	log.SetName(name)
 	log.SetLevel(logLevel)
-	err = log.SetTarget(logTarget)
+	err = log.SetTargetLogDirectory(logTarget, logDirectory)
 	if err != nil {
 		fmt.Printf("Failed to configure logging: %v\n", err)
 		return

cns/logger/log.go

@@ -28,9 +28,9 @@ type CNSLogger struct {
 }
 
 // Initialize CNS Logger
-func InitLogger(fileName string, logLevel, logTarget int) {
+func InitLogger(fileName string, logLevel, logTarget int, logDir string) {
 	Log = &CNSLogger{
-		logger: log.NewLogger(fileName, logLevel, logTarget),
+		logger: log.NewLogger(fileName, logLevel, logTarget, logDir),
 	}
 }
 
@@ -61,8 +61,8 @@ func Close() {
 	}
 }
 
-func SetLogDirectory(dir string) {
-	Log.logger.SetLogDirectory(dir)
+func SetTargetLogDirectory(target int, dir string) error {
+	return Log.logger.SetTargetLogDirectory(target, dir)
 }
 
 // Set context details for logs and metrics

cns/service/main.go

@@ -224,8 +224,7 @@ func main() {
 	config.ErrChan = make(chan error, 1)
 
 	// Create logging provider.
-	logger.InitLogger(name, logLevel, logTarget)
-	logger.SetLogDirectory(logDirectory)
+	logger.InitLogger(name, logLevel, logTarget, logDirectory)
 
 	if !telemetryEnabled {
 		logger.Errorf("[Azure CNS] Cannot disable telemetry via cmdline. Update cns_config.json to disable telemetry.")

log/logger.go

@@ -60,16 +60,16 @@ type Logger struct {
 var pid = os.Getpid()
 
 // NewLogger creates a new Logger.
-func NewLogger(name string, level int, target int) *Logger {
+func NewLogger(name string, level int, target int, logDir string) *Logger {
 	var logger Logger
 
 	logger.l = log.New(nil, logPrefix, log.LstdFlags)
 	logger.name = name
 	logger.level = level
+	logger.directory = logDir
 	logger.SetTarget(target)
 	logger.maxFileSize = maxLogFileSize
 	logger.maxFileCount = maxLogFileCount
-	logger.directory = ""
 	logger.mutex = &sync.Mutex{}
 
 	return &logger
@@ -103,9 +103,10 @@ func (logger *Logger) Close() {
 	}
 }
 
-// SetLogDirectory sets the directory location where logs should be stored.
-func (logger *Logger) SetLogDirectory(logDirectory string) {
+// SetTargetLogDirectory sets the directory location where logs should be stored along with the target
+func (logger *Logger) SetTargetLogDirectory(target int, logDirectory string) error {
 	logger.directory = logDirectory
+	return logger.SetTarget(target)
 }
 
 // GetLogDirectory gets the directory location where logs should be stored.

log/logger_test.go

@@ -17,7 +17,8 @@ const (
 
 // Tests that the log file rotates when size limit is reached.
 func TestLogFileRotatesWhenSizeLimitIsReached(t *testing.T) {
-	l := NewLogger(logName, LevelInfo, TargetLogfile)
+	logDirectory := "" // This sets the current location for logs
+	l := NewLogger(logName, LevelInfo, TargetLogfile, logDirectory)
 	if l == nil {
 		t.Fatalf("Failed to create logger.\n")
 	}
@@ -53,7 +54,8 @@ func TestLogFileRotatesWhenSizeLimitIsReached(t *testing.T) {
 }
 
 func TestPid(t *testing.T) {
-	l := NewLogger(logName, LevelInfo, TargetLogfile)
+	logDirectory := "" // This sets the current location for logs
+	l := NewLogger(logName, LevelInfo, TargetLogfile, logDirectory)
 	if l == nil {
 		t.Fatalf("Failed to create logger.")
 	}
@@ -62,15 +64,15 @@ func TestPid(t *testing.T) {
 	l.Close()
 	fn := l.GetLogDirectory() + logName + ".log"
 	defer os.Remove(fn)
-	
+
 	logBytes, err := ioutil.ReadFile(fn)
 	if err != nil {
 		t.Fatalf("Failed to read log, %v", err)
 	}
 	log := string(logBytes)
-	exptectedLog := fmt.Sprintf("[%v] LogText 1", os.Getpid());
+	exptectedLog := fmt.Sprintf("[%v] LogText 1", os.Getpid())
 
-	if  !strings.Contains(log, exptectedLog){
+	if !strings.Contains(log, exptectedLog) {
 		t.Fatalf("Unexpected log: %s.", log)
 	}
 }

log/stdapi.go

@@ -4,7 +4,8 @@
 package log
 
 // Standard logger is a pre-defined logger for convenience.
-var stdLog = NewLogger("azure-container-networking", LevelInfo, TargetStderr)
+// Set log directory as the current location
+var stdLog = NewLogger("azure-container-networking", LevelInfo, TargetStderr, "")
 
 // GetStd - Helper functions for the standard logger.
 func GetStd() *Logger {
@@ -15,10 +16,6 @@ func SetName(name string) {
 	stdLog.SetName(name)
 }
 
-func SetTarget(target int) error {
-	return stdLog.SetTarget(target)
-}
-
 func SetLevel(level int) {
 	stdLog.SetLevel(level)
 }
@@ -31,8 +28,8 @@ func Close() {
 	stdLog.Close()
 }
 
-func SetLogDirectory(logDirectory string) {
-	stdLog.SetLogDirectory(logDirectory)
+func SetTargetLogDirectory(target int, logDirectory string) error {
+	return stdLog.SetTargetLogDirectory(target, logDirectory)
 }
 
 func GetLogDirectory() string {

npm/plugin/main.go

@@ -22,7 +22,7 @@ var version string
 func initLogging() error {
 	log.SetName("azure-npm")
 	log.SetLevel(log.LevelInfo)
-	if err := log.SetTarget(log.TargetStdOutAndLogFile); err != nil {
+	if err := log.SetTargetLogDirectory(log.TargetStdOutAndLogFile, ""); err != nil {
 		log.Logf("Failed to configure logging, err:%v.", err)
 		return err
 	}