Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.4k
Externalize kvm agent storage timeout configuration#4585
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
79a7aac0351ecaf9bfb55c221230abd447adebb3e989792f80c1b1a217f2b79001e74e7d62d24695da42353ff5ec80ff53c3c868011d8225File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| /* | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package com.cloud.agent.properties; | ||
| /** | ||
| * Class of constant agent's properties available to configure on | ||
| * "agent.properties". | ||
| *<br><br> | ||
| * Not all available agent properties are defined here, but we should work to | ||
| * migrate them on demand to this class. | ||
| * | ||
| * @param <T> type of the default value. | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 useful javadoc, nice to see | ||
| */ | ||
| public class AgentProperties{ | ||
| /** | ||
| * Heartbeat update timeout. <br> | ||
| * Data type: int. <br> | ||
| * Default value: 60000 (ms). | ||
| */ | ||
| public static final Property<Integer> HEARTBEAT_UPDATE_TIMEOUT = new Property<Integer>("heartbeat.update.timeout", 60000); | ||
| public static class Property <T>{ | ||
| private final String name; | ||
| private final T defaultValue; | ||
| private Property(String name, T value) { | ||
| this.name = name; | ||
| this.defaultValue = value; | ||
| } | ||
| public String getName() { | ||
| return name; | ||
| } | ||
| public T getDefaultValue() { | ||
| return defaultValue; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| /* | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package com.cloud.agent.properties; | ||
| import com.cloud.utils.PropertiesUtil; | ||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import org.apache.cloudstack.utils.security.KeyStoreUtils; | ||
| import org.apache.commons.beanutils.ConvertUtils; | ||
| import org.apache.commons.beanutils.converters.IntegerConverter; | ||
| import org.apache.commons.lang3.StringUtils; | ||
| import org.apache.log4j.Logger; | ||
| /** | ||
| * This class provides a facility to read the agent's properties file and get | ||
| * its properties, according to the {@link AgentProperties} properties constants. | ||
| * | ||
| */ | ||
| public class AgentPropertiesFileHandler { | ||
| private static final Logger logger = Logger.getLogger(AgentPropertiesFileHandler.class); | ||
| /** | ||
| * This method reads the property in the agent.properties file. | ||
| * | ||
| * @param property the property to retrieve. | ||
| * @return The value of the property. If the property is not available, the | ||
| * default defined value will be used. | ||
| */ | ||
| 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) { | ||
| try { | ||
| String configValue = PropertiesUtil.loadFromFile(agentPropertiesFile).getProperty(name); | ||
| if (StringUtils.isNotBlank(configValue)) { | ||
| if (defaultValue instanceof Integer) { | ||
| ConvertUtils.register(new IntegerConverter(defaultValue), Integer.class); | ||
| } | ||
| return (T)ConvertUtils.convert(configValue, defaultValue.getClass()); | ||
| } else { | ||
| logger.debug(String.format("Property [%s] has empty or null value. Using default value [%s].", name, defaultValue)); | ||
| } | ||
| } catch (IOException ex) { | ||
| logger.debug(String.format("Failed to get property [%s]. Using default value [%s].", name, defaultValue), ex); | ||
| } | ||
| } else { | ||
| logger.debug(String.format("File [%s] was not found, we will use default defined values. Property [%s]: [%s].", KeyStoreUtils.AGENT_PROPSFILE, name, defaultValue)); | ||
| } | ||
| return defaultValue; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
| package com.cloud.agent.properties; | ||
| import com.cloud.utils.PropertiesUtil; | ||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.util.Properties; | ||
| import junit.framework.TestCase; | ||
| import org.apache.commons.beanutils.ConvertUtils; | ||
| import org.junit.Assert; | ||
| import org.junit.Test; | ||
| import org.junit.runner.RunWith; | ||
| import org.mockito.Mock; | ||
| import org.mockito.Mockito; | ||
| import org.powermock.api.mockito.PowerMockito; | ||
| import org.powermock.core.classloader.annotations.PrepareForTest; | ||
| import org.powermock.modules.junit4.PowerMockRunner; | ||
| @RunWith(PowerMockRunner.class) | ||
| @PrepareForTest({PropertiesUtil.class, ConvertUtils.class}) | ||
| public class AgentPropertiesFileHandlerTest extends TestCase { | ||
| @Mock | ||
| AgentProperties.Property<String> agentPropertiesStringMock; | ||
| @Mock | ||
| AgentProperties.Property<Integer> agentPropertiesIntegerMock; | ||
| @Mock | ||
| File fileMock; | ||
| @Mock | ||
| Properties propertiesMock; | ||
| @Test | ||
| public void validateGetPropertyValueFileNotFoundReturnDefaultValue() throws Exception{ | ||
| String expectedResult = "default value"; | ||
| Mockito.doReturn(expectedResult).when(agentPropertiesStringMock).getDefaultValue(); | ||
| PowerMockito.mockStatic(PropertiesUtil.class); | ||
| PowerMockito.doReturn(null).when(PropertiesUtil.class, "findConfigFile", Mockito.anyString()); | ||
| String result = AgentPropertiesFileHandler.getPropertyValue(agentPropertiesStringMock); | ||
| Assert.assertEquals(expectedResult, result); | ||
| } | ||
| @Test | ||
| public void validateGetPropertyValueLoadFromFileThrowsIOExceptionReturnDefaultValue() throws Exception{ | ||
| String expectedResult = "default value"; | ||
| Mockito.doReturn(expectedResult).when(agentPropertiesStringMock).getDefaultValue(); | ||
| PowerMockito.mockStatic(PropertiesUtil.class); | ||
| PowerMockito.doReturn(fileMock).when(PropertiesUtil.class, "findConfigFile", Mockito.anyString()); | ||
| PowerMockito.doThrow(new IOException()).when(PropertiesUtil.class, "loadFromFile", Mockito.any()); | ||
| String result = AgentPropertiesFileHandler.getPropertyValue(agentPropertiesStringMock); | ||
| Assert.assertEquals(expectedResult, result); | ||
| } | ||
| @Test | ||
| public void validateGetPropertyValuePropertyIsEmptyReturnDefaultValue() throws Exception{ | ||
| String expectedResult = "default value"; | ||
| Mockito.doReturn(expectedResult).when(agentPropertiesStringMock).getDefaultValue(); | ||
| Mockito.doReturn("name").when(agentPropertiesStringMock).getName(); | ||
| PowerMockito.mockStatic(PropertiesUtil.class); | ||
| PowerMockito.doReturn(fileMock).when(PropertiesUtil.class, "findConfigFile", Mockito.anyString()); | ||
| PowerMockito.doReturn(propertiesMock).when(PropertiesUtil.class, "loadFromFile", Mockito.any()); | ||
| PowerMockito.doReturn("").when(propertiesMock).getProperty(Mockito.anyString()); | ||
| String result = AgentPropertiesFileHandler.getPropertyValue(agentPropertiesStringMock); | ||
| Assert.assertEquals(expectedResult, result); | ||
| } | ||
| @Test | ||
| public void validateGetPropertyValuePropertyIsNullReturnDefaultValue() throws Exception{ | ||
| String expectedResult = "default value"; | ||
| Mockito.doReturn(expectedResult).when(agentPropertiesStringMock).getDefaultValue(); | ||
| Mockito.doReturn("name").when(agentPropertiesStringMock).getName(); | ||
| PowerMockito.mockStatic(PropertiesUtil.class); | ||
| PowerMockito.doReturn(fileMock).when(PropertiesUtil.class, "findConfigFile", Mockito.anyString()); | ||
| PowerMockito.doReturn(propertiesMock).when(PropertiesUtil.class, "loadFromFile", Mockito.any()); | ||
| PowerMockito.doReturn(null).when(propertiesMock).getProperty(Mockito.anyString()); | ||
| String result = AgentPropertiesFileHandler.getPropertyValue(agentPropertiesStringMock); | ||
| Assert.assertEquals(expectedResult, result); | ||
| } | ||
| @Test | ||
| public void validateGetPropertyValueValidPropertyReturnPropertyValue() throws Exception{ | ||
| String expectedResult = "test"; | ||
| Mockito.doReturn("default value").when(agentPropertiesStringMock).getDefaultValue(); | ||
| Mockito.doReturn("name").when(agentPropertiesStringMock).getName(); | ||
| PowerMockito.mockStatic(PropertiesUtil.class); | ||
| PowerMockito.doReturn(fileMock).when(PropertiesUtil.class, "findConfigFile", Mockito.anyString()); | ||
| PowerMockito.doReturn(propertiesMock).when(PropertiesUtil.class, "loadFromFile", Mockito.any()); | ||
| Mockito.doReturn(expectedResult).when(propertiesMock).getProperty(Mockito.anyString()); | ||
| String result = AgentPropertiesFileHandler.getPropertyValue(agentPropertiesStringMock); | ||
| Assert.assertEquals(expectedResult, result); | ||
| } | ||
| @Test | ||
| public void validateGetPropertyValueValidIntegerPropertyReturnPropertyValue() throws Exception{ | ||
| Integer expectedResult = 2; | ||
| Mockito.doReturn(1).when(agentPropertiesIntegerMock).getDefaultValue(); | ||
| Mockito.doReturn("name").when(agentPropertiesIntegerMock).getName(); | ||
| PowerMockito.mockStatic(PropertiesUtil.class); | ||
| PowerMockito.doReturn(fileMock).when(PropertiesUtil.class, "findConfigFile", Mockito.anyString()); | ||
| PowerMockito.doReturn(propertiesMock).when(PropertiesUtil.class, "loadFromFile", Mockito.any()); | ||
| Mockito.doReturn(String.valueOf(expectedResult)).when(propertiesMock).getProperty(Mockito.anyString()); | ||
| Integer result = AgentPropertiesFileHandler.getPropertyValue(agentPropertiesIntegerMock); | ||
| Assert.assertEquals(expectedResult, result); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose 60000 is the default (1 min) wouldn't hurt to make that explicit in here for operator convenience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DaanHoogland done.