Skip to content
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

api,agent,server,engine-schema: scalability improvements #9840

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 6 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
2 changes: 1 addition & 1 deletion .python-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.6
3.10
6 changes: 6 additions & 0 deletions agent/conf/agent.properties
Original file line number Diff line number Diff line change
Expand Up @@ -433,3 +433,9 @@ iscsi.session.cleanup.enabled=false

# Implicit host tags managed by agent.properties
# host.tags=

# Timeout(in seconds) for SSL handshake when agent connects to server
#ssl.handshake.timeout=
Copy link
Member

@weizhouapache weizhouapache Oct 31, 2024

Choose a reason for hiding this comment

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

since this line is commented by default, can we add the default value ?
if there is no default value, add a line to explain what will happen (unlimited ?) ?


# Wait(in seconds) during agent reconnections
#backoff.seconds=
Copy link
Member

@weizhouapache weizhouapache Oct 31, 2024

Choose a reason for hiding this comment

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

same here, add default value if exists ?

774 changes: 402 additions & 372 deletions agent/src/main/java/com/cloud/agent/Agent.java

Large diffs are not rendered by default.

57 changes: 33 additions & 24 deletions agent/src/main/java/com/cloud/agent/AgentShell.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,29 +16,6 @@
// under the License.
package com.cloud.agent;

import com.cloud.agent.Agent.ExitStatus;
import com.cloud.agent.dao.StorageComponent;
import com.cloud.agent.dao.impl.PropertiesStorage;
import com.cloud.agent.properties.AgentProperties;
import com.cloud.agent.properties.AgentPropertiesFileHandler;
import com.cloud.resource.ServerResource;
import com.cloud.utils.LogUtils;
import com.cloud.utils.ProcessUtil;
import com.cloud.utils.PropertiesUtil;
import com.cloud.utils.backoff.BackoffAlgorithm;
import com.cloud.utils.backoff.impl.ConstantTimeBackoff;
import com.cloud.utils.exception.CloudRuntimeException;
import org.apache.commons.daemon.Daemon;
import org.apache.commons.daemon.DaemonContext;
import org.apache.commons.daemon.DaemonInitException;
import org.apache.commons.lang.math.NumberUtils;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.core.config.Configurator;

import javax.naming.ConfigurationException;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
Expand All @@ -53,6 +30,31 @@
import java.util.Properties;
import java.util.UUID;

import javax.naming.ConfigurationException;

import org.apache.commons.daemon.Daemon;
import org.apache.commons.daemon.DaemonContext;
import org.apache.commons.daemon.DaemonInitException;
import org.apache.commons.lang.math.NumberUtils;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.core.config.Configurator;

import com.cloud.agent.Agent.ExitStatus;
import com.cloud.agent.dao.StorageComponent;
import com.cloud.agent.dao.impl.PropertiesStorage;
import com.cloud.agent.properties.AgentProperties;
import com.cloud.agent.properties.AgentPropertiesFileHandler;
import com.cloud.resource.ServerResource;
import com.cloud.utils.LogUtils;
import com.cloud.utils.ProcessUtil;
import com.cloud.utils.PropertiesUtil;
import com.cloud.utils.backoff.BackoffAlgorithm;
import com.cloud.utils.backoff.impl.ConstantTimeBackoff;
import com.cloud.utils.exception.CloudRuntimeException;

public class AgentShell implements IAgentShell, Daemon {
protected static Logger LOGGER = LogManager.getLogger(AgentShell.class);

Expand Down Expand Up @@ -406,7 +408,9 @@

LOGGER.info("Defaulting to the constant time backoff algorithm");
_backoff = new ConstantTimeBackoff();
_backoff.configure("ConstantTimeBackoff", new HashMap<String, Object>());
Map<String, Object> map = new HashMap<>();
map.put("seconds", _properties.getProperty("backoff.seconds"));
_backoff.configure("ConstantTimeBackoff", map);

Check warning on line 413 in agent/src/main/java/com/cloud/agent/AgentShell.java

View check run for this annotation

Codecov / codecov/patch

agent/src/main/java/com/cloud/agent/AgentShell.java#L411-L413

Added lines #L411 - L413 were not covered by tests
}

private void launchAgent() throws ConfigurationException {
Expand Down Expand Up @@ -455,6 +459,11 @@
agent.start();
}

@Override
public Integer getSslHandshakeTimeout() {
return AgentPropertiesFileHandler.getPropertyValue(AgentProperties.SSL_HANDSHAKE_TIMEOUT);
}

public synchronized int getNextAgentId() {
return _nextAgentId++;
}
Expand Down
2 changes: 2 additions & 0 deletions agent/src/main/java/com/cloud/agent/IAgentShell.java
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,6 @@ public interface IAgentShell {
String getConnectedHost();

void launchNewAgent(ServerResource resource) throws ConfigurationException;

Integer getSslHandshakeTimeout();
}
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,13 @@ public Property<Integer> getWorkers() {
*/
public static final Property<String> HOST_TAGS = new Property<>("host.tags", null, String.class);

/**
* Timeout for SSL handshake in seconds
* Data type: Integer.<br>
* Default value: <code>null</code>
*/
public static final Property<Integer> SSL_HANDSHAKE_TIMEOUT = new Property<>("ssl.handshake.timeout", null, Integer.class);

public static class Property <T>{
private String name;
private T defaultValue;
Expand Down
7 changes: 7 additions & 0 deletions agent/src/test/java/com/cloud/agent/AgentShellTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -362,4 +362,11 @@ public void updateAndGetConnectedHost() {

Assert.assertEquals(expected, shell.getConnectedHost());
}

@Test
public void testGetSslHandshakeTimeout() {
Integer expected = 1;
agentPropertiesFileHandlerMocked.when(() -> AgentPropertiesFileHandler.getPropertyValue(Mockito.eq(AgentProperties.SSL_HANDSHAKE_TIMEOUT))).thenReturn(expected);
Assert.assertEquals(expected, agentShellSpy.getSslHandshakeTimeout());
}
}
258 changes: 258 additions & 0 deletions agent/src/test/java/com/cloud/agent/AgentTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,258 @@
// 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;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.io.IOException;
import java.net.InetSocketAddress;

import javax.naming.ConfigurationException;

import org.apache.logging.log4j.Logger;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.junit.MockitoJUnitRunner;
import org.springframework.test.util.ReflectionTestUtils;

import com.cloud.resource.ServerResource;
import com.cloud.utils.backoff.impl.ConstantTimeBackoff;
import com.cloud.utils.nio.Link;
import com.cloud.utils.nio.NioConnection;

@RunWith(MockitoJUnitRunner.class)
public class AgentTest {
Agent agent;
private AgentShell shell;
private ServerResource serverResource;
private Logger logger;

@Before
public void setUp() throws ConfigurationException {
shell = mock(AgentShell.class);
serverResource = mock(ServerResource.class);
doReturn(true).when(serverResource).configure(any(), any());
doReturn(1).when(shell).getWorkers();
doReturn(1).when(shell).getPingRetries();
agent = new Agent(shell, 1, serverResource);
logger = mock(Logger.class);
ReflectionTestUtils.setField(agent, "logger", logger);
}

@Test
public void testGetLinkLogNullLinkReturnsEmptyString() {
Link link = null;
String result = agent.getLinkLog(link);
assertEquals("", result);
}

@Test
public void testGetLinkLogLinkWithTraceEnabledReturnsLinkLogWithHashCode() {
Link link = mock(Link.class);
InetSocketAddress socketAddress = new InetSocketAddress("192.168.1.100", 1111);
when(link.getSocketAddress()).thenReturn(socketAddress);
when(logger.isTraceEnabled()).thenReturn(true);

String result = agent.getLinkLog(link);
System.out.println(result);
assertTrue(result.startsWith(System.identityHashCode(link) + "-"));
assertTrue(result.contains("192.168.1.100"));
}

@Test
public void testGetAgentNameWhenServerResourceIsNull() {
ReflectionTestUtils.setField(agent, "serverResource", null);
assertEquals("Agent", agent.getAgentName());
}

@Test
public void testGetAgentNameWhenAppendAgentNameIsTrue() {
when(serverResource.isAppendAgentNameToLogs()).thenReturn(true);
when(serverResource.getName()).thenReturn("TestAgent");

String agentName = agent.getAgentName();
assertEquals("TestAgent", agentName);
}

@Test
public void testGetAgentNameWhenAppendAgentNameIsFalse() {
when(serverResource.isAppendAgentNameToLogs()).thenReturn(false);

String agentName = agent.getAgentName();
assertEquals("Agent", agentName);
}

@Test
public void testAgentInitialization() {
Runtime.getRuntime().removeShutdownHook(agent.shutdownThread);
when(shell.getPingRetries()).thenReturn(3);
when(shell.getWorkers()).thenReturn(5);
agent.setupShutdownHookAndInitExecutors();
assertNotNull(agent.selfTaskExecutor);
assertNotNull(agent.outRequestHandler);
assertNotNull(agent.requestHandler);
}

@Test
public void testAgentShutdownHookAdded() {
Runtime.getRuntime().removeShutdownHook(agent.shutdownThread);
when(logger.isTraceEnabled()).thenReturn(true);
agent.setupShutdownHookAndInitExecutors();
verify(logger).trace("Adding shutdown hook");
}

@Test
public void testGetResourceGuidValidGuidAndResourceName() {
when(shell.getGuid()).thenReturn("12345");
String result = agent.getResourceGuid();
assertTrue(result.startsWith("12345-" + ServerResource.class.getSimpleName()));
}

@Test
public void testGetZoneReturnsValidZone() {
when(shell.getZone()).thenReturn("ZoneA");
String result = agent.getZone();
assertEquals("ZoneA", result);
}

@Test
public void testGetPodReturnsValidPod() {
when(shell.getPod()).thenReturn("PodA");
String result = agent.getPod();
assertEquals("PodA", result);
}

@Test
public void testSetLinkAssignsLink() {
Link mockLink = mock(Link.class);
agent.setLink(mockLink);
assertEquals(mockLink, agent.link);
}

@Test
public void testGetResourceReturnsServerResource() {
ServerResource mockResource = mock(ServerResource.class);
ReflectionTestUtils.setField(agent, "serverResource", mockResource);
ServerResource result = agent.getResource();
assertSame(mockResource, result);
}

@Test
public void testGetResourceName() {
String result = agent.getResourceName();
assertTrue(result.startsWith(ServerResource.class.getSimpleName()));
}

@Test
public void testUpdateLastPingResponseTimeUpdatesCurrentTime() {
long beforeUpdate = System.currentTimeMillis();
agent.updateLastPingResponseTime();
long updatedTime = agent.lastPingResponseTime.get();
assertTrue(updatedTime >= beforeUpdate);
assertTrue(updatedTime <= System.currentTimeMillis());
}

@Test
public void testGetNextSequenceIncrementsSequence() {
long initialSequence = agent.getNextSequence();
long nextSequence = agent.getNextSequence();
assertEquals(initialSequence + 1, nextSequence);
long thirdSequence = agent.getNextSequence();
assertEquals(nextSequence + 1, thirdSequence);
}

@Test
public void testRegisterControlListenerAddsListener() {
IAgentControlListener listener = mock(IAgentControlListener.class);
agent.registerControlListener(listener);
assertTrue(agent.controlListeners.contains(listener));
}

@Test
public void testUnregisterControlListenerRemovesListener() {
IAgentControlListener listener = mock(IAgentControlListener.class);
agent.registerControlListener(listener);
assertTrue(agent.controlListeners.contains(listener));
agent.unregisterControlListener(listener);
assertFalse(agent.controlListeners.contains(listener));
}

@Test
public void testCloseAndTerminateLinkLinkIsNullDoesNothing() {
agent.closeAndTerminateLink(null);
}

@Test
public void testCloseAndTerminateLinkValidLinkCallsCloseAndTerminate() {
Link mockLink = mock(Link.class);
agent.closeAndTerminateLink(mockLink);
verify(mockLink).close();
verify(mockLink).terminated();
}

@Test
public void testStopAndCleanupConnectionConnectionIsNullDoesNothing() {
agent.connection = null;
agent.stopAndCleanupConnection(false);
}

@Test
public void testStopAndCleanupConnectionValidConnectionNoWaitStopsAndCleansUp() throws IOException {
NioConnection mockConnection = mock(NioConnection.class);
agent.connection = mockConnection;
agent.stopAndCleanupConnection(false);
verify(mockConnection).stop();
verify(mockConnection).cleanUp();
}

@Test
public void testStopAndCleanupConnectionCleanupThrowsIOExceptionLogsWarning() throws IOException {
NioConnection mockConnection = mock(NioConnection.class);
agent.connection = mockConnection;
doThrow(new IOException("Cleanup failed")).when(mockConnection).cleanUp();
agent.stopAndCleanupConnection(false);
verify(mockConnection).stop();
verify(logger).warn(eq("Fail to clean up old connection. {}"), any(IOException.class));
}

@Test
public void testStopAndCleanupConnectionValidConnectionWaitForStopWaitsForStartupToStop() throws IOException {
NioConnection mockConnection = mock(NioConnection.class);
ConstantTimeBackoff mockBackoff = mock(ConstantTimeBackoff.class);
mockBackoff.setTimeToWait(0);
agent.connection = mockConnection;
when(shell.getBackoffAlgorithm()).thenReturn(mockBackoff);
when(mockConnection.isStartup()).thenReturn(true, true, false);
agent.stopAndCleanupConnection(true);
verify(mockConnection).stop();
verify(mockConnection).cleanUp();
verify(mockBackoff, times(3)).waitBeforeRetry();
}
}
Loading
Loading