Config file: camelCase naming, support for soc config file added (#348)

Summary: Added camel case naming in rialto config file. Added support
for soc config file.
Type: Feature
Test Plan: UT/ Fullstack
Jira: RIALTO-702 RIALTO-709 RIALTO-710

CMakeLists.txt

@@ -159,6 +159,10 @@ if ( NOT ENVIRONMENT_VARIABLES)
     set( ENVIRONMENT_VARIABLES "\"XDG_RUNTIME_DIR=/tmp\",\"GST_REGISTRY=/tmp/rialto-server-gstreamer-cache.bin\",\"WESTEROS_SINK_USE_ESSRMGR=1\"" )
 endif()
 
+if ( NOT EXTRA_ENV_VARIABLES)
+    set( EXTRA_ENV_VARIABLES "" )
+endif()
+
 if (NOT SESSION_SERVER_PATH)
     set( SESSION_SERVER_PATH "\"/usr/bin/RialtoServer\"" )
 endif()

serverManager/config/rialto-config.in.json

@@ -21,14 +21,15 @@
     // with fields that mimic the structure of this file, but with
     // alternative values
 
-    "environment_variables" : [@ENVIRONMENT_VARIABLES@],
-    "session_server_path" : @SESSION_SERVER_PATH@,
-    "startup_timeout_ms" : @STARTUP_TIMEOUT_MS@,
-    "healthcheck_interval_s" : @HEALTHCHECK_INTERVAL_S@,
-    "socket_permissions" : @SOCKET_PERMISSIONS@,
-    "socket_owner" : @SOCKET_OWNER@,
-    "socket_group" : @SOCKET_GROUP@,
-    "num_of_preloaded_servers" : @NUM_OF_PRELOADED_SERVERS@,
-    "log_level" : @LOG_LEVEL@,
-    "num_of_pings_before_recovery" : @NUM_OF_PINGS_BEFORE_RECOVERY@
+    "environmentVariables" : [@ENVIRONMENT_VARIABLES@],
+    "extraEnvVariables" : [@EXTRA_ENV_VARIABLES@],
+    "sessionServerPath" : @SESSION_SERVER_PATH@,
+    "startupTimeoutMs" : @STARTUP_TIMEOUT_MS@,
+    "healthcheckIntervalInSeconds" : @HEALTHCHECK_INTERVAL_S@,
+    "socketPermissions" : @SOCKET_PERMISSIONS@,
+    "socketOwner" : @SOCKET_OWNER@,
+    "socketGroup" : @SOCKET_GROUP@,
+    "numOfPreloadedServers" : @NUM_OF_PRELOADED_SERVERS@,
+    "logLevel" : @LOG_LEVEL@,
+    "numOfPingsBeforeRecovery" : @NUM_OF_PINGS_BEFORE_RECOVERY@
 }

serverManager/service/CMakeLists.txt

@@ -68,6 +68,7 @@ if( RIALTO_ENABLE_CONFIG_FILE )
     add_compile_definitions(RIALTO_ENABLE_CONFIG_FILE)
     add_compile_definitions(RIALTO_CONFIG_PATH="${RIALTO_CONFIG_FILE_DIR}/rialto-config.json")
     add_compile_definitions(RIALTO_CONFIG_OVERRIDES_PATH="${RIALTO_CONFIG_OVERRIDES_FILE_DIR}/rialto-config-overrides.json")
+    add_compile_definitions(RIALTO_CONFIG_SOC_PATH="${RIALTO_CONFIG_FILE_DIR}/rialto_soc.json")
 
     target_link_libraries(
         RialtoServerManager

serverManager/service/include/ConfigHelper.h

@@ -50,11 +50,14 @@ class ConfigHelper
 #ifdef RIALTO_ENABLE_CONFIG_FILE
 private:
     void readConfigFile(const std::string &filePath);
+    void mergeEnvVariables();
 #endif // RIALTO_ENABLE_CONFIG_FILE
 
 private:
     std::unique_ptr<IConfigReaderFactory> m_configReaderFactory;
     std::map<std::string, std::string> m_sessionServerEnvVars;
+    std::map<std::string, std::string> m_envVarsFromConfigFile;
+    std::map<std::string, std::string> m_extraEnvVarsFromConfigFile;
     std::string m_sessionServerPath;
     std::chrono::milliseconds m_sessionServerStartupTimeout;
     std::chrono::seconds m_healthcheckInterval;

serverManager/service/include/ConfigReader.h

@@ -39,6 +39,7 @@ class ConfigReader : public IConfigReader
     bool read() override;
 
     std::list<std::string> getEnvironmentVariables() override;
+    std::list<std::string> getExtraEnvVariables() override;
     std::optional<std::string> getSessionServerPath() override;
     std::optional<std::chrono::milliseconds> getSessionServerStartupTimeout() override;
     std::optional<std::chrono::seconds> getHealthcheckInterval() override;
@@ -51,6 +52,7 @@ class ConfigReader : public IConfigReader
 
 private:
     void parseEnvironmentVariables(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root);
+    void parseExtraEnvVariables(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root);
     void parseSessionServerPath(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root);
     void parseSessionServerStartupTimeout(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root);
     void parseHealthcheckInterval(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root);
@@ -61,10 +63,18 @@ class ConfigReader : public IConfigReader
     void parseLogLevel(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root);
     void parseNumOfPingsBeforeRecovery(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root);
 
+    std::list<std::string> getListOfStrings(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root,
+                                            const std::string &valueName) const;
+    std::optional<std::string> getString(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root,
+                                         const std::string &valueName) const;
+    std::optional<unsigned int> getUInt(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root,
+                                        const std::string &valueName) const;
+
     std::shared_ptr<firebolt::rialto::wrappers::IJsonCppWrapper> m_jsonWrapper;
     std::shared_ptr<IFileReader> m_fileReader;
 
     std::list<std::string> m_envVars;
+    std::list<std::string> m_extraEnvVars;
     std::optional<std::string> m_sessionServerPath;
     std::optional<std::chrono::milliseconds> m_sessionServerStartupTimeout;
     std::optional<std::chrono::seconds> m_healthcheckInterval;

serverManager/service/include/IConfigReader.h

@@ -35,6 +35,7 @@ class IConfigReader
     virtual bool read() = 0;
 
     virtual std::list<std::string> getEnvironmentVariables() = 0;
+    virtual std::list<std::string> getExtraEnvVariables() = 0;
     virtual std::optional<std::string> getSessionServerPath() = 0;
     virtual std::optional<std::chrono::milliseconds> getSessionServerStartupTimeout() = 0;
     virtual std::optional<std::chrono::seconds> getHealthcheckInterval() = 0;

serverManager/service/source/ConfigHelper.cpp

@@ -66,8 +66,11 @@ ConfigHelper::ConfigHelper(std::unique_ptr<IConfigReaderFactory> &&configReaderF
       m_numOfFailedPingsBeforeRecovery{config.numOfFailedPingsBeforeRecovery}, m_loggingLevels{}
 {
 #ifdef RIALTO_ENABLE_CONFIG_FILE
+    // Read from least to most important file
     readConfigFile(RIALTO_CONFIG_PATH);
+    readConfigFile(RIALTO_CONFIG_SOC_PATH);
     readConfigFile(RIALTO_CONFIG_OVERRIDES_PATH);
+    mergeEnvVariables();
 #endif // RIALTO_ENABLE_CONFIG_FILE
 }
 
@@ -126,14 +129,18 @@ void ConfigHelper::readConfigFile(const std::string &filePath)
         return;
     }
 
-    std::map<std::string, std::string> envVariables{convertToMap(configReader->getEnvironmentVariables())};
-    for (const auto &[name, value] : envVariables)
+    // Always override env variables when present in "more important" file
+    // (envVariables + extraEnvVariables from less important file will be wiped if envVariables are present)
+    const std::map<std::string, std::string> envVariables{convertToMap(configReader->getEnvironmentVariables())};
+    if (!envVariables.empty())
     {
-        // If environment variable exists in ServerManagerConfig, do not overwrite it
-        if (m_sessionServerEnvVars.end() == m_sessionServerEnvVars.find(name))
-        {
-            m_sessionServerEnvVars.emplace(name, value);
-        }
+        m_envVarsFromConfigFile = envVariables;
+        m_extraEnvVarsFromConfigFile.clear();
+    }
+    const std::map<std::string, std::string> extraEnvVariables{convertToMap(configReader->getExtraEnvVariables())};
+    if (!extraEnvVariables.empty())
+    {
+        m_extraEnvVarsFromConfigFile = extraEnvVariables;
     }
 
     if (configReader->getSessionServerPath())
@@ -163,5 +170,19 @@ void ConfigHelper::readConfigFile(const std::string &filePath)
     if (configReader->getLoggingLevels())
         m_loggingLevels = configReader->getLoggingLevels().value();
 }
+
+void ConfigHelper::mergeEnvVariables()
+{
+    // Env vars from json are more important than values from config struct
+    if (!m_envVarsFromConfigFile.empty())
+    {
+        m_sessionServerEnvVars = m_envVarsFromConfigFile;
+    }
+    for (const auto &[name, value] : m_extraEnvVarsFromConfigFile)
+    {
+        // If env variable exists both in envVariables and extraEnvVariables, overwrite it.
+        m_sessionServerEnvVars[name] = value;
+    }
+}
 #endif // RIALTO_ENABLE_CONFIG_FILE
 } // namespace rialto::servermanager::service

serverManager/service/source/ConfigReader.cpp

@@ -46,6 +46,7 @@ bool ConfigReader::read()
     }
 
     parseEnvironmentVariables(root);
+    parseExtraEnvVariables(root);
     parseSessionServerPath(root);
     parseSessionServerStartupTimeout(root);
     parseHealthcheckInterval(root);
@@ -61,92 +62,110 @@ bool ConfigReader::read()
 
 void ConfigReader::parseEnvironmentVariables(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root)
 {
-    if (root->isMember("environment_variables") && root->at("environment_variables")->isArray())
+    m_envVars = getListOfStrings(root, "environmentVariables");
+    if (m_envVars.empty())
     {
-        std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> envVarsJson = root->at("environment_variables");
-        Json::ArrayIndex size = envVarsJson->size();
-        for (Json::ArrayIndex index = 0; index < size; ++index)
-        {
-            if (envVarsJson->at(index)->isString())
-            {
-                m_envVars.emplace_back(envVarsJson->at(index)->asString());
-            }
-        }
+        m_envVars = getListOfStrings(root, "environment_variables");
     }
 }
 
+void ConfigReader::parseExtraEnvVariables(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root)
+{
+    m_extraEnvVars = getListOfStrings(root, "extraEnvVariables");
+}
+
 void ConfigReader::parseSessionServerPath(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root)
 {
-    if (root->isMember("session_server_path") && root->at("session_server_path")->isString())
+    m_sessionServerPath = getString(root, "sessionServerPath");
+    if (!m_sessionServerPath.has_value())
     {
-        m_sessionServerPath = root->at("session_server_path")->asString();
+        m_sessionServerPath = getString(root, "session_server_path");
     }
 }
 
 void ConfigReader::parseSessionServerStartupTimeout(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root)
 {
-    if (root->isMember("startup_timeout_ms") && root->at("startup_timeout_ms")->isUInt())
+    auto timeout{getUInt(root, "startupTimeoutMs")};
+    if (!timeout.has_value())
+    {
+        timeout = getUInt(root, "startup_timeout_ms");
+    }
+    if (timeout.has_value())
     {
-        m_sessionServerStartupTimeout = std::chrono::milliseconds(root->at("startup_timeout_ms")->asUInt());
+        m_sessionServerStartupTimeout = std::chrono::milliseconds{timeout.value()};
     }
 }
 
 void ConfigReader::parseHealthcheckInterval(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root)
 {
-    if (root->isMember("healthcheck_interval_s") && root->at("healthcheck_interval_s")->isUInt())
+    auto interval{getUInt(root, "healthcheckIntervalInSeconds")};
+    if (!interval.has_value())
     {
-        m_healthcheckInterval = std::chrono::seconds(root->at("healthcheck_interval_s")->asUInt());
+        interval = getUInt(root, "healthcheck_interval_s");
+    }
+    if (interval.has_value())
+    {
+        m_healthcheckInterval = std::chrono::seconds{*interval};
     }
 }
 
 void ConfigReader::parseSocketPermissions(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root)
 {
-    if (root->isMember("socket_permissions") && root->at("socket_permissions")->isUInt())
+    auto permissions{getUInt(root, "socketPermissions")};
+    if (!permissions.has_value())
+    {
+        permissions = getUInt(root, "socket_permissions");
+    }
+    if (permissions.has_value())
     {
-        unsigned permissions = root->at("socket_permissions")->asUInt();
-
         firebolt::rialto::common::SocketPermissions socketPermissions;
-        socketPermissions.ownerPermissions = (permissions / 100) % 10;
-        socketPermissions.groupPermissions = (permissions / 10) % 10;
-        socketPermissions.otherPermissions = (permissions) % 10;
+        socketPermissions.ownerPermissions = (*permissions / 100) % 10;
+        socketPermissions.groupPermissions = (*permissions / 10) % 10;
+        socketPermissions.otherPermissions = (*permissions) % 10;
         m_socketPermissions = socketPermissions;
     }
 }
 
 void ConfigReader::parseSocketOwner(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root)
 {
-    const char *kFieldString = "socket_owner";
-    if (root->isMember(kFieldString) && root->at(kFieldString)->isString())
+    m_socketOwner = getString(root, "socketOwner");
+    if (!m_socketOwner.has_value())
     {
-        m_socketOwner = root->at(kFieldString)->asString();
+        m_socketOwner = getString(root, "socket_owner");
     }
 }
 
 void ConfigReader::parseSocketGroup(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root)
 {
-    const char *kFieldString = "socket_group";
-    if (root->isMember(kFieldString) && root->at(kFieldString)->isString())
+    m_socketGroup = getString(root, "socketGroup");
+    if (!m_socketGroup.has_value())
     {
-        m_socketGroup = root->at(kFieldString)->asString();
+        m_socketGroup = getString(root, "socket_group");
     }
 }
 
 void ConfigReader::parseNumOfPreloadedServers(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root)
 {
-    if (root->isMember("num_of_preloaded_servers") && root->at("num_of_preloaded_servers")->isUInt())
+    m_numOfPreloadedServers = getUInt(root, "numOfPreloadedServers");
+    if (!m_numOfPreloadedServers.has_value())
     {
-        m_numOfPreloadedServers = root->at("num_of_preloaded_servers")->asUInt();
+        m_numOfPreloadedServers = getUInt(root, "num_of_preloaded_servers");
     }
 }
 
 void ConfigReader::parseLogLevel(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root)
 {
-    if (root->isMember("log_level") && root->at("log_level")->isUInt())
+    std::optional<unsigned> loggingLevel{getUInt(root, "logLevel")};
+    if (!loggingLevel.has_value())
+    {
+        loggingLevel = getUInt(root, "log_level");
+    }
+
+    if (loggingLevel)
     {
-        unsigned loggingLevel = root->at("log_level")->asUInt();
         rialto::servermanager::service::LoggingLevel newLevel{rialto::servermanager::service::LoggingLevel::UNCHANGED};
 
-        switch (loggingLevel)
+        switch (*loggingLevel)
         {
         case 0:
             newLevel = rialto::servermanager::service::LoggingLevel::FATAL;
@@ -176,9 +195,10 @@ void ConfigReader::parseLogLevel(std::shared_ptr<firebolt::rialto::wrappers::IJs
 
 void ConfigReader::parseNumOfPingsBeforeRecovery(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root)
 {
-    if (root->isMember("num_of_pings_before_recovery") && root->at("num_of_pings_before_recovery")->isUInt())
+    m_numOfPingsBeforeRecovery = getUInt(root, "numOfPingsBeforeRecovery");
+    if (!m_numOfPingsBeforeRecovery.has_value())
     {
-        m_numOfPingsBeforeRecovery = root->at("num_of_pings_before_recovery")->asUInt();
+        m_numOfPingsBeforeRecovery = getUInt(root, "num_of_pings_before_recovery");
     }
 }
 
@@ -187,6 +207,11 @@ std::list<std::string> ConfigReader::getEnvironmentVariables()
     return m_envVars;
 }
 
+std::list<std::string> ConfigReader::getExtraEnvVariables()
+{
+    return m_extraEnvVars;
+}
+
 std::optional<std::string> ConfigReader::getSessionServerPath()
 {
     return m_sessionServerPath;
@@ -232,4 +257,43 @@ std::optional<unsigned int> ConfigReader::getNumOfPingsBeforeRecovery()
     return m_numOfPingsBeforeRecovery;
 }
 
+std::list<std::string> ConfigReader::getListOfStrings(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root,
+                                                      const std::string &valueName) const
+{
+    std::list<std::string> result;
+    if (root->isMember(valueName) && root->at(valueName)->isArray())
+    {
+        std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> envVarsJson = root->at(valueName);
+        Json::ArrayIndex size = envVarsJson->size();
+        for (Json::ArrayIndex index = 0; index < size; ++index)
+        {
+            if (envVarsJson->at(index)->isString())
+            {
+                result.emplace_back(envVarsJson->at(index)->asString());
+            }
+        }
+    }
+    return result;
+}
+
+std::optional<std::string> ConfigReader::getString(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root,
+                                                   const std::string &valueName) const
+{
+    if (root->isMember(valueName) && root->at(valueName)->isString())
+    {
+        return root->at(valueName)->asString();
+    }
+    return std::nullopt;
+}
+
+std::optional<unsigned int> ConfigReader::getUInt(std::shared_ptr<firebolt::rialto::wrappers::IJsonValueWrapper> root,
+                                                  const std::string &valueName) const
+{
+    if (root->isMember(valueName) && root->at(valueName)->isUInt())
+    {
+        return root->at(valueName)->asUInt();
+    }
+    return std::nullopt;
+}
+
 } // namespace rialto::servermanager::service

tests/unittests/serverManager/mocks/ConfigReaderMock.h

@@ -31,6 +31,7 @@ class ConfigReaderMock : public IConfigReader
 public:
     MOCK_METHOD(bool, read, (), (override));
     MOCK_METHOD(std::list<std::string>, getEnvironmentVariables, (), (override));
+    MOCK_METHOD(std::list<std::string>, getExtraEnvVariables, (), (override));
     MOCK_METHOD(std::optional<std::string>, getSessionServerPath, (), (override));
     MOCK_METHOD(std::optional<std::chrono::milliseconds>, getSessionServerStartupTimeout, (), (override));
     MOCK_METHOD(std::optional<std::chrono::seconds>, getHealthcheckInterval, (), (override));