Fix config file not found handling (#17578)

Signed-off-by: Manan Gupta <[email protected]>

go/viperutil/config.go

@@ -18,6 +18,7 @@ package viperutil
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"reflect"
@@ -162,19 +163,19 @@ func LoadConfig() (context.CancelFunc, error) {
 	}
 
 	if err != nil {
-		if nferr, ok := err.(viper.ConfigFileNotFoundError); ok {
+		if isConfigFileNotFoundError(err) {
 			msg := "Failed to read in config %s: %s"
 			switch configFileNotFoundHandling.Get() {
 			case WarnOnConfigFileNotFound:
 				msg += ". This is optional, and can be ignored if you are not using config files. For a detailed explanation, see https://github.com/vitessio/vitess/blob/main/doc/viper/viper.md#config-files."
-				log.WARN(msg, registry.Static.ConfigFileUsed(), nferr.Error())
+				log.WARN(msg, registry.Static.ConfigFileUsed(), err.Error())
 				fallthrough // after warning, ignore the error
 			case IgnoreConfigFileNotFound:
-				err = nil
+				return func() {}, nil
 			case ErrorOnConfigFileNotFound:
-				log.ERROR(msg, registry.Static.ConfigFileUsed(), nferr.Error())
+				log.ERROR(msg, registry.Static.ConfigFileUsed(), err.Error())
 			case ExitOnConfigFileNotFound:
-				log.CRITICAL(msg, registry.Static.ConfigFileUsed(), nferr.Error())
+				log.CRITICAL(msg, registry.Static.ConfigFileUsed(), err.Error())
 			}
 		}
 	}
@@ -186,6 +187,14 @@ func LoadConfig() (context.CancelFunc, error) {
 	return registry.Dynamic.Watch(context.Background(), registry.Static, configPersistenceMinInterval.Get())
 }
 
+// isConfigFileNotFoundError checks if the error is caused because the file wasn't found.
+func isConfigFileNotFoundError(err error) bool {
+	if errors.As(err, &viper.ConfigFileNotFoundError{}) {
+		return true
+	}
+	return errors.Is(err, os.ErrNotExist)
+}
+
 // NotifyConfigReload adds a subscription that the dynamic registry will attempt
 // to notify on config changes. The notification fires after the updated config
 // has been loaded from disk into the live config.

go/viperutil/config_test.go

@@ -47,3 +47,60 @@ duration: 10h
 	assert.Equal(t, IgnoreConfigFileNotFound, getHandlingValueFunc("duration"), "failed to get value on duration key")
 	assert.Equal(t, ExitOnConfigFileNotFound, getHandlingValueFunc("default"), "failed to get value on default key")
 }
+
+// TestLoadConfig tests that LoadConfig behaves in the way expected when the config file doesn't exist.
+func TestLoadConfig(t *testing.T) {
+	origConfigFile := configFile.Get()
+	origConfigName := configName.Get()
+	origConfigFileNotFoundHandling := configFileNotFoundHandling.Get()
+	defer func() {
+		configFile.Set(origConfigFile)
+		configName.Set(origConfigName)
+		configFileNotFoundHandling.Set(origConfigFileNotFoundHandling)
+	}()
+
+	t.Run("Ignore file not found error", func(t *testing.T) {
+		configFile.Set("notfound.yaml")
+		configFileNotFoundHandling.Set(IgnoreConfigFileNotFound)
+		_, err := LoadConfig()
+		require.NoError(t, err)
+	})
+
+	t.Run("Ignore file not found error from config name", func(t *testing.T) {
+		configFile.Set("")
+		configName.Set("notfound")
+		configFileNotFoundHandling.Set(IgnoreConfigFileNotFound)
+		_, err := LoadConfig()
+		require.NoError(t, err)
+	})
+
+	t.Run("Warn file not found error", func(t *testing.T) {
+		configFile.Set("notfound.yaml")
+		configFileNotFoundHandling.Set(WarnOnConfigFileNotFound)
+		_, err := LoadConfig()
+		require.NoError(t, err)
+	})
+
+	t.Run("Ignore file not found error from config name", func(t *testing.T) {
+		configFile.Set("")
+		configName.Set("notfound")
+		configFileNotFoundHandling.Set(WarnOnConfigFileNotFound)
+		_, err := LoadConfig()
+		require.NoError(t, err)
+	})
+
+	t.Run("Error file not found error", func(t *testing.T) {
+		configFile.Set("notfound.yaml")
+		configFileNotFoundHandling.Set(ErrorOnConfigFileNotFound)
+		_, err := LoadConfig()
+		require.Error(t, err)
+	})
+
+	t.Run("Ignore file not found error from config name", func(t *testing.T) {
+		configFile.Set("")
+		configName.Set("notfound")
+		configFileNotFoundHandling.Set(ErrorOnConfigFileNotFound)
+		_, err := LoadConfig()
+		require.Error(t, err)
+	})
+}