Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.Map;
import java.util.Properties;

import com.cloud.agent.properties.AgentPropertiesFileHandler;
import org.apache.commons.io.IOUtils;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
Expand Down Expand Up @@ -59,6 +60,8 @@ public synchronized void persist(String key, String value) {
_properties.store(output, _name);
output.flush();
output.close();
AgentPropertiesFileHandler.clearCache();
logger.debug("Cleared agent properties cache after persisting key: {}", key);
} catch (IOException e) {
logger.error("Uh-oh: ", e);
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.cloud.utils.PropertiesUtil;
import java.io.File;
import java.io.IOException;
import java.util.Properties;
import org.apache.cloudstack.utils.security.KeyStoreUtils;
import org.apache.commons.beanutils.ConvertUtils;
import org.apache.commons.beanutils.converters.IntegerConverter;
Expand All @@ -29,13 +30,18 @@
* This class provides a facility to read the agent's properties file and get
* its properties, according to the {@link AgentProperties} properties constants.
*
* Properties are loaded lazily and cached until the cache is cleared (for example, after persisting updates).
*/
public class AgentPropertiesFileHandler {

protected static Logger LOGGER = LogManager.getLogger(AgentPropertiesFileHandler.class);

// Simple singleton caching - loaded once, cached forever
private static volatile Properties cachedProperties;

/**
* This method reads the property in the agent.properties file.
* Properties are loaded once and cached for the agent's lifetime.
*
* @param property the property to retrieve.
* @return The value of the property. If the property is not available, the
Expand All @@ -45,16 +51,15 @@ public static <T> T getPropertyValue(AgentProperties.Property<T> property) {
T defaultValue = property.getDefaultValue();
String name = property.getName();

File agentPropertiesFile = PropertiesUtil.findConfigFile(KeyStoreUtils.AGENT_PROPSFILE);

if (agentPropertiesFile == null) {
LOGGER.debug("File [{}] was not found, we will use default defined values. Property [{}]: [{}].", KeyStoreUtils.AGENT_PROPSFILE, name, defaultValue);
Properties properties = getCachedProperties();

if (properties == null) {
LOGGER.debug(String.format("Properties file was not found or could not be loaded, using default values. Property [%s]: [%s].", name, defaultValue));
return defaultValue;
}

try {
String configValue = PropertiesUtil.loadFromFile(agentPropertiesFile).getProperty(name);
String configValue = properties.getProperty(name);
if (StringUtils.isBlank(configValue)) {
LOGGER.debug("Property [{}] has empty or null value. Using default value [{}].", name, defaultValue);
return defaultValue;
Expand All @@ -71,11 +76,69 @@ public static <T> T getPropertyValue(AgentProperties.Property<T> property) {
LOGGER.debug("Property [{}] was altered. Now using the value [{}].", name, configValue);
return (T)ConvertUtils.convert(configValue, property.getTypeClass());

} catch (IOException ex) {
} catch (RuntimeException ex) {
LOGGER.debug("Failed to get property [{}]. Using default value [{}].", name, defaultValue, ex);
}

return defaultValue;
}

/**
* Gets the cached properties, loading them once if not already loaded.
* Agent properties are static configuration that don't change during runtime.
*
* @return cached Properties object or null if file cannot be loaded
*/
private static Properties getCachedProperties() {
Properties properties = cachedProperties;
if (properties == null) {
synchronized (AgentPropertiesFileHandler.class) {
properties = cachedProperties;
if (properties == null) {
loadProperties();
properties = cachedProperties;
}
}
}
return properties;
}

/**
* Loads properties from file and caches them for the agent's lifetime.
*/
private static void loadProperties() {
File agentPropertiesFile = PropertiesUtil.findConfigFile(KeyStoreUtils.AGENT_PROPSFILE);

if (agentPropertiesFile == null) {
LOGGER.debug("File [{}] was not found.", KeyStoreUtils.AGENT_PROPSFILE);
return;
}

try {
Properties newProperties = PropertiesUtil.loadFromFile(agentPropertiesFile);
cachedProperties = newProperties;

LOGGER.info("Loaded {} properties from [{}]", newProperties.size(), agentPropertiesFile.getAbsolutePath());

} catch (IOException ex) {
LOGGER.error("Failed to load properties from file [{}].", agentPropertiesFile.getAbsolutePath(), ex);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems FATAL to me. Any reason to keep running when this occurs

}
}

/**
* Clears the properties cache.
*/
public static synchronized void clearCache() {
LOGGER.info("Clearing agent properties cache");
cachedProperties = null;
}

/**
* Returns whether the properties cache is currently loaded.
*
* @return true if properties are cached, false otherwise.
*/
public static boolean isCacheLoaded() {
return cachedProperties != null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,14 @@ public class AgentPropertiesFileHandlerTest extends TestCase {
@Before
public void setUp() throws Exception {
propertiesUtilMocked = Mockito.mockStatic(PropertiesUtil.class);
AgentPropertiesFileHandler.clearCache();
}

@Override
@After
public void tearDown() throws Exception {
propertiesUtilMocked.close();
AgentPropertiesFileHandler.clearCache();
}

@Test
Expand Down Expand Up @@ -196,4 +198,39 @@ public void getPropertyValueTestValidLongPropertyReturnPropertyValue() throws Ex
Assert.assertEquals(expectedResult, result);
}

@Test
public void testCacheFunctionality() throws Exception {
String expectedResult = "cached-value";
AgentProperties.Property<String> agentProperty = new AgentProperties.Property<String>("test.property", "default", String.class);

propertiesUtilMocked.when(() -> PropertiesUtil.findConfigFile(Mockito.anyString())).thenReturn(fileMock);
propertiesUtilMocked.when(() -> PropertiesUtil.loadFromFile(Mockito.any())).thenReturn(propertiesMock);
Mockito.doReturn(expectedResult).when(propertiesMock).getProperty(Mockito.anyString());

Assert.assertFalse("Cache should be empty initially", AgentPropertiesFileHandler.isCacheLoaded());

String result1 = AgentPropertiesFileHandler.getPropertyValue(agentProperty);
Assert.assertEquals("First call should return correct value", expectedResult, result1);
Assert.assertTrue("Cache should be loaded after first call", AgentPropertiesFileHandler.isCacheLoaded());

Mockito.verify(PropertiesUtil.class, Mockito.times(1));
PropertiesUtil.loadFromFile(Mockito.any(File.class));

String result2 = AgentPropertiesFileHandler.getPropertyValue(agentProperty);
Assert.assertEquals("Second call should return same cached value", expectedResult, result2);
Assert.assertTrue("Cache should still be loaded", AgentPropertiesFileHandler.isCacheLoaded());

Mockito.verify(PropertiesUtil.class, Mockito.times(1));
PropertiesUtil.loadFromFile(Mockito.any(File.class));

AgentPropertiesFileHandler.clearCache();
Assert.assertFalse("Cache should be empty after clear", AgentPropertiesFileHandler.isCacheLoaded());

String result3 = AgentPropertiesFileHandler.getPropertyValue(agentProperty);
Assert.assertEquals("Third call should return correct value after cache clear", expectedResult, result3);
Assert.assertTrue("Cache should be loaded again", AgentPropertiesFileHandler.isCacheLoaded());

Mockito.verify(PropertiesUtil.class, Mockito.times(2));
PropertiesUtil.loadFromFile(Mockito.any(File.class));
}
}
Loading