From 91943705a93da923d851fc314778b3543b7a4e6d Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 23 Jan 2025 15:54:52 +0530 Subject: [PATCH 1/3] Fixup alerting and logging error in BGPServiceImpl --- server/src/main/java/com/cloud/alert/AlertManagerImpl.java | 5 +++-- server/src/main/java/com/cloud/bgp/BGPServiceImpl.java | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/cloud/alert/AlertManagerImpl.java b/server/src/main/java/com/cloud/alert/AlertManagerImpl.java index a9e66c6aecea..a35164e6b2a0 100644 --- a/server/src/main/java/com/cloud/alert/AlertManagerImpl.java +++ b/server/src/main/java/com/cloud/alert/AlertManagerImpl.java @@ -738,13 +738,14 @@ public void sendAlert(AlertType alertType, DataCenter dataCenter, Pod pod, Clust 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) { @@ -754,7 +755,7 @@ public void sendAlert(AlertType alertType, DataCenter dataCenter, Pod pod, Clust 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 101731774e72..d46dcde0462c 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 boolean allocateASNumber(long zoneId, Long asNumber, Long networkId, Long 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 logMsg = Objects.nonNull(vpcId) ? ("VPC " + vpc) : ("network " + network); + LOGGER.debug("Allocating the AS Number {} to {} on zone {}", asNumberVO::toString, + logMsg::toString, () -> dataCenterDao.findById(zoneId)); asNumberVO.setAllocated(true); asNumberVO.setAllocatedTime(new Date()); if (Objects.nonNull(vpcId)) { From c5b1e381f33d4ef3f4c7eec353fd6233cd21f055 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 13 Feb 2025 17:40:17 +0530 Subject: [PATCH 2/3] Update unit tests --- .../com/cloud/alert/AlertManagerImplTest.java | 73 ++++++++++++++++--- 1 file changed, 61 insertions(+), 12 deletions(-) diff --git a/server/src/test/java/com/cloud/alert/AlertManagerImplTest.java b/server/src/test/java/com/cloud/alert/AlertManagerImplTest.java index e04c5e181e74..c0314843bb46 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.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; @@ -37,6 +39,12 @@ import javax.mail.MessagingException; import java.io.UnsupportedEncodingException; +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 { @@ -45,7 +53,7 @@ public class AlertManagerImplTest { AlertManagerImpl alertManagerImplMock; @Mock - AlertDao alertDaoMock; + AlertDao _alertDao; @Mock private DataCenterDao _dcDao; @@ -65,7 +73,16 @@ public class AlertManagerImplTest { @Mock SMTPMailSender mailSenderMock; - private void sendMessage (){ + private final String[] recipients = new String[]{"test@test.com"}; + private final String senderAddress = "sender@test.com"; + + @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); @@ -77,7 +94,7 @@ private void sendMessage (){ 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(); } @@ -85,38 +102,70 @@ private void sendMessage (){ @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 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"); } } From f3e28fedc5e42efc9907fa9303b16004cc691791 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 13 Feb 2025 21:57:18 +0530 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: dahn --- server/src/main/java/com/cloud/bgp/BGPServiceImpl.java | 4 ++-- .../src/test/java/com/cloud/alert/AlertManagerImplTest.java | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/com/cloud/bgp/BGPServiceImpl.java b/server/src/main/java/com/cloud/bgp/BGPServiceImpl.java index d46dcde0462c..c003bd05a518 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 boolean allocateASNumber(long zoneId, Long asNumber, Long networkId, Long netName = network.getName(); } - String logMsg = Objects.nonNull(vpcId) ? ("VPC " + vpc) : ("network " + network); + String networkName = Objects.nonNull(vpcId) ? ("VPC " + vpc) : ("network " + network); LOGGER.debug("Allocating the AS Number {} to {} on zone {}", asNumberVO::toString, - logMsg::toString, () -> dataCenterDao.findById(zoneId)); + 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 c0314843bb46..257411a72e32 100644 --- a/server/src/test/java/com/cloud/alert/AlertManagerImplTest.java +++ b/server/src/test/java/com/cloud/alert/AlertManagerImplTest.java @@ -137,7 +137,6 @@ public void sendAlertTestWarnLogging() { Mockito.verify(alertManagerImplMock, Mockito.never()).sendMessage(any()); } - @Test public void testSendAlertWithNullParameters() throws MessagingException, UnsupportedEncodingException { // Given @@ -162,7 +161,6 @@ public void testSendAlertWithNullParameters() throws MessagingException, Unsuppo assertEquals(alertType.getType(), capturedAlert.getType()); } - @Test(expected = NullPointerException.class) public void testSendAlertWithNullAlertType() throws MessagingException, UnsupportedEncodingException { // When