This is an automated email from the ASF dual-hosted git repository.
pearl11594 pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.20 by this push:
new 6a3314c40b9 Fixup alerting and logging error in BGPServiceImpl (#10252)
6a3314c40b9 is described below
commit 6a3314c40b9769bda90b5798b8878668c764c1a2
Author: Vishesh <[email protected]>
AuthorDate: Tue Feb 18 19:53:43 2025 +0530
Fixup alerting and logging error in BGPServiceImpl (#10252)
* Fixup alerting and logging error in BGPServiceImpl
* Update unit tests
* Apply suggestions from code review
Co-authored-by: dahn <[email protected]>
---------
Co-authored-by: dahn <[email protected]>
---
.../java/com/cloud/alert/AlertManagerImpl.java | 5 +-
.../main/java/com/cloud/bgp/BGPServiceImpl.java | 6 +-
.../java/com/cloud/alert/AlertManagerImplTest.java | 71 ++++++++++++++++++----
3 files changed, 65 insertions(+), 17 deletions(-)
diff --git a/server/src/main/java/com/cloud/alert/AlertManagerImpl.java
b/server/src/main/java/com/cloud/alert/AlertManagerImpl.java
index f4a8167c5a2..60ba90b77d5 100644
--- a/server/src/main/java/com/cloud/alert/AlertManagerImpl.java
+++ b/server/src/main/java/com/cloud/alert/AlertManagerImpl.java
@@ -778,13 +778,14 @@ public class AlertManagerImpl extends ManagerBase
implements AlertManager, Confi
AlertVO alert = null;
Long clusterId = cluster == null ? null : cluster.getId();
Long podId = pod == null ? null : pod.getId();
+ long dcId = dataCenter == null ? 0L : dataCenter.getId();
if ((alertType != AlertManager.AlertType.ALERT_TYPE_HOST) &&
(alertType != AlertManager.AlertType.ALERT_TYPE_USERVM)
&& (alertType !=
AlertManager.AlertType.ALERT_TYPE_DOMAIN_ROUTER) && (alertType !=
AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY)
&& (alertType != AlertManager.AlertType.ALERT_TYPE_SSVM) &&
(alertType != AlertManager.AlertType.ALERT_TYPE_STORAGE_MISC)
&& (alertType !=
AlertManager.AlertType.ALERT_TYPE_MANAGEMENT_NODE) && (alertType !=
AlertManager.AlertType.ALERT_TYPE_RESOURCE_LIMIT_EXCEEDED)
&& (alertType !=
AlertManager.AlertType.ALERT_TYPE_UPLOAD_FAILED) && (alertType !=
AlertManager.AlertType.ALERT_TYPE_OOBM_AUTH_ERROR)
&& (alertType != AlertManager.AlertType.ALERT_TYPE_HA_ACTION)
&& (alertType != AlertManager.AlertType.ALERT_TYPE_CA_CERT)) {
- alert = _alertDao.getLastAlert(alertType.getType(),
dataCenter.getId(), podId, clusterId);
+ alert = _alertDao.getLastAlert(alertType.getType(), dcId, podId,
clusterId);
}
if (alert == null) {
@@ -794,7 +795,7 @@ public class AlertManagerImpl extends ManagerBase
implements AlertManager, Confi
newAlert.setContent(content);
newAlert.setClusterId(clusterId);
newAlert.setPodId(podId);
- newAlert.setDataCenterId(dataCenter.getId());
+ newAlert.setDataCenterId(dcId);
newAlert.setSentCount(1);
newAlert.setLastSent(new Date());
newAlert.setName(alertType.getName());
diff --git a/server/src/main/java/com/cloud/bgp/BGPServiceImpl.java
b/server/src/main/java/com/cloud/bgp/BGPServiceImpl.java
index 101731774e7..c003bd05a51 100644
--- a/server/src/main/java/com/cloud/bgp/BGPServiceImpl.java
+++ b/server/src/main/java/com/cloud/bgp/BGPServiceImpl.java
@@ -255,9 +255,9 @@ public class BGPServiceImpl implements BGPService {
netName = network.getName();
}
- LOGGER.debug("Allocating the AS Number {} to {} on zone {}",
asNumber::toString,
- (Objects.nonNull(vpcId) ? "VPC " + vpc : "network " +
network)::toString,
- () -> dataCenterDao.findById(zoneId));
+ String networkName = Objects.nonNull(vpcId) ? ("VPC " + vpc) :
("network " + network);
+ LOGGER.debug("Allocating the AS Number {} to {} on zone {}",
asNumberVO::toString,
+ networkName::toString, () -> dataCenterDao.findById(zoneId));
asNumberVO.setAllocated(true);
asNumberVO.setAllocatedTime(new Date());
if (Objects.nonNull(vpcId)) {
diff --git a/server/src/test/java/com/cloud/alert/AlertManagerImplTest.java
b/server/src/test/java/com/cloud/alert/AlertManagerImplTest.java
index 2959e73ae9e..d34d0b5873f 100644
--- a/server/src/test/java/com/cloud/alert/AlertManagerImplTest.java
+++ b/server/src/test/java/com/cloud/alert/AlertManagerImplTest.java
@@ -26,8 +26,10 @@ import
org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.utils.mailing.SMTPMailSender;
import org.apache.logging.log4j.Logger;
import org.junit.Assert;
+import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
@@ -48,6 +50,12 @@ import com.cloud.host.HostVO;
import com.cloud.host.dao.HostDao;
import com.cloud.storage.StorageManager;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.verify;
+
@RunWith(MockitoJUnitRunner.class)
public class AlertManagerImplTest {
@@ -56,7 +64,7 @@ public class AlertManagerImplTest {
AlertManagerImpl alertManagerImplMock;
@Mock
- AlertDao alertDaoMock;
+ AlertDao _alertDao;
@Mock
private DataCenterDao _dcDao;
@@ -88,7 +96,16 @@ public class AlertManagerImplTest {
@Mock
SMTPMailSender mailSenderMock;
- private void sendMessage (){
+ private final String[] recipients = new String[]{"[email protected]"};
+ private final String senderAddress = "[email protected]";
+
+ @Before
+ public void setUp() {
+ alertManagerImplMock.recipients = recipients;
+ alertManagerImplMock.senderAddress = senderAddress;
+ }
+
+ private void sendMessage() {
try {
DataCenterVO zone = Mockito.mock(DataCenterVO.class);
Mockito.when(zone.getId()).thenReturn(0L);
@@ -100,7 +117,7 @@ public class AlertManagerImplTest {
Mockito.when(cluster.getId()).thenReturn(1L);
Mockito.when(_clusterDao.findById(1L)).thenReturn(cluster);
-
alertManagerImplMock.sendAlert(AlertManager.AlertType.ALERT_TYPE_CPU, 0, 1l,
1l, "", "");
+
alertManagerImplMock.sendAlert(AlertManager.AlertType.ALERT_TYPE_CPU, 0, 1L,
1L, "", "");
} catch (UnsupportedEncodingException | MessagingException e) {
Assert.fail();
}
@@ -108,39 +125,69 @@ public class AlertManagerImplTest {
@Test
public void sendAlertTestSendMail() {
-
Mockito.doReturn(null).when(alertDaoMock).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
+
Mockito.doReturn(null).when(_alertDao).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
Mockito.anyLong(), Mockito.anyLong());
- Mockito.doReturn(null).when(alertDaoMock).persist(Mockito.any());
- alertManagerImplMock.recipients = new String [] {""};
+ Mockito.doReturn(null).when(_alertDao).persist(any());
+ alertManagerImplMock.recipients = new String[]{""};
sendMessage();
- Mockito.verify(alertManagerImplMock).sendMessage(Mockito.any());
+ Mockito.verify(alertManagerImplMock).sendMessage(any());
}
@Test
public void sendAlertTestDebugLogging() {
Mockito.doReturn(0).when(alertVOMock).getSentCount();
-
Mockito.doReturn(alertVOMock).when(alertDaoMock).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
+
Mockito.doReturn(alertVOMock).when(_alertDao).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
Mockito.anyLong(), Mockito.anyLong());
sendMessage();
Mockito.verify(alertManagerImplMock.logger).debug(Mockito.anyString());
- Mockito.verify(alertManagerImplMock,
Mockito.never()).sendMessage(Mockito.any());
+ Mockito.verify(alertManagerImplMock,
Mockito.never()).sendMessage(any());
}
@Test
public void sendAlertTestWarnLogging() {
-
Mockito.doReturn(null).when(alertDaoMock).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
+
Mockito.doReturn(null).when(_alertDao).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
Mockito.anyLong(), Mockito.anyLong());
- Mockito.doReturn(null).when(alertDaoMock).persist(Mockito.any());
+ Mockito.doReturn(null).when(_alertDao).persist(Mockito.any());
alertManagerImplMock.recipients = null;
sendMessage();
Mockito.verify(alertManagerImplMock.logger,
Mockito.times(2)).warn(Mockito.anyString());
- Mockito.verify(alertManagerImplMock,
Mockito.never()).sendMessage(Mockito.any());
+ Mockito.verify(alertManagerImplMock,
Mockito.never()).sendMessage(any());
+ }
+
+ @Test
+ public void testSendAlertWithNullParameters() throws MessagingException,
UnsupportedEncodingException {
+ // Given
+ String subject = "Test Subject";
+ String content = "Test Content";
+ AlertManager.AlertType alertType =
AlertManager.AlertType.ALERT_TYPE_MEMORY;
+
+ // When
+ alertManagerImplMock.sendAlert(alertType, null, null, null, subject,
content);
+
+ // Then
+ ArgumentCaptor<AlertVO> alertCaptor =
ArgumentCaptor.forClass(AlertVO.class);
+ verify(_alertDao).persist(alertCaptor.capture());
+
+ AlertVO capturedAlert = alertCaptor.getValue();
+ assertNotNull("Captured alert should not be null", capturedAlert);
+ assertEquals(0L, capturedAlert.getDataCenterId());
+ assertNull(capturedAlert.getPodId());
+ assertNull(capturedAlert.getClusterId());
+ assertEquals(subject, capturedAlert.getSubject());
+ assertEquals(content, capturedAlert.getContent());
+ assertEquals(alertType.getType(), capturedAlert.getType());
+ }
+
+ @Test(expected = NullPointerException.class)
+ public void testSendAlertWithNullAlertType() throws MessagingException,
UnsupportedEncodingException {
+ // When
+ alertManagerImplMock.sendAlert(null, 0, 1L, 1L, "subject", "content");
}
@Test
