Fixup alerting and logging error in BGPServiceImpl #10252

vishesh92 · 2025-01-23T10:26:09Z

Description

Fixes:

NPE when dcId is null or 0L on sending an alert.
Exception during logging in BGPServiceImpl

Generated summary

This pull request includes several changes to the AlertManagerImpl and BGPServiceImpl classes to improve code clarity and functionality. The most important changes involve the handling of data center IDs and the logging of AS number allocations.

Improvements to `AlertManagerImpl`:

server/src/main/java/com/cloud/alert/AlertManagerImpl.java: Added a new variable dcId to handle data center IDs more effectively, replacing direct calls to dataCenter.getId(). [1] [2]

Improvements to `BGPServiceImpl`:

server/src/main/java/com/cloud/bgp/BGPServiceImpl.java: Refactored the logging statement in allocateASNumber to use final variables for VpcVO and NetworkVO, improving the readability and ensuring correct variable references.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI
test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

vishesh92 · 2025-01-23T10:26:33Z

@blueorangutan package

blueorangutan · 2025-01-23T10:28:04Z

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

codecov · 2025-01-23T10:33:54Z

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 15.99%. Comparing base (96b757c) to head (f3e28fe).
Report is 74 commits behind head on 4.20.

Files with missing lines	Patch %	Lines
...er/src/main/java/com/cloud/bgp/BGPServiceImpl.java	0.00%	3 Missing ⚠️
...rc/main/java/com/cloud/alert/AlertManagerImpl.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.20   #10252      +/-   ##
============================================
- Coverage     16.13%   15.99%   -0.15%     
- Complexity    12972    13080     +108     
============================================
  Files          5639     5649      +10     
  Lines        494297   495623    +1326     
  Branches      59908    60003      +95     
============================================
- Hits          79773    79255     -518     
- Misses       405698   407519    +1821     
- Partials       8826     8849      +23

Flag	Coverage Δ
uitests	`4.01% <ø> (-0.02%)`	⬇️
unittests	`16.82% <0.00%> (-0.16%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

server/src/main/java/com/cloud/bgp/BGPServiceImpl.java

weizhouapache · 2025-01-23T10:41:53Z

@blueorangutan package

blueorangutan · 2025-01-23T10:44:04Z

@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-01-23T11:38:37Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12187

blueorangutan · 2025-01-23T11:44:35Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12188

vishesh92 · 2025-01-24T07:47:10Z

@blueorangutan test

blueorangutan · 2025-01-24T07:48:03Z

@vishesh92 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2025-01-24T21:53:16Z

[SF] Trillian test result (tid-12180)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 48495 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10252-t12180-kvm-ol8.zip
Smoke tests completed. 141 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

weizhouapache

code lgtm

weizhouapache · 2025-02-04T08:55:14Z

@DaanHoogland
can you review this ?

DaanHoogland

code looks good, are there any tests, unit or integration, that cover this bit, @vishesh92 ?

vishesh92 · 2025-02-13T12:10:59Z

@blueorangutan package

blueorangutan · 2025-02-13T12:12:03Z

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-02-13T13:39:16Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 12442

vishesh92 · 2025-02-13T13:41:43Z

@blueorangutan package

blueorangutan · 2025-02-13T13:44:03Z

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-02-13T15:09:51Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12447

server/src/main/java/com/cloud/bgp/BGPServiceImpl.java

server/src/test/java/com/cloud/alert/AlertManagerImplTest.java

Co-authored-by: dahn <daan.hoogland@gmail.com>

vishesh92 · 2025-02-13T16:27:41Z

@blueorangutan package

blueorangutan · 2025-02-13T16:30:03Z

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-02-13T17:45:01Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12450

Pearl1594 · 2025-02-14T13:58:17Z

@blueorangutan test

blueorangutan · 2025-02-14T14:00:06Z

@Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

weizhouapache

code lgtm

smoke test results look good

blueorangutan · 2025-02-15T07:16:17Z

[SF] Trillian test result (tid-12411)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 58696 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10252-t12411-kvm-ol8.zip
Smoke tests completed. 140 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_06_purge_expunged_vm_background_task	`Failure`	388.39	test_purge_expunged_vms.py

Pearl1594

code LGTM

* Fixup alerting and logging error in BGPServiceImpl * Update unit tests * Apply suggestions from code review Co-authored-by: dahn <daan.hoogland@gmail.com> --------- Co-authored-by: dahn <daan.hoogland@gmail.com>

vishesh92 requested a review from weizhouapache January 23, 2025 10:26

weizhouapache reviewed Jan 23, 2025

View reviewed changes

server/src/main/java/com/cloud/bgp/BGPServiceImpl.java Outdated Show resolved Hide resolved

Fixup alerting and logging error in BGPServiceImpl

9194370

vishesh92 force-pushed the fixup-logging-and-alerting branch from 51f7677 to 9194370 Compare January 23, 2025 10:40

apache deleted a comment from blueorangutan Jan 24, 2025

weizhouapache mentioned this pull request Feb 3, 2025

VR: fix duplicated lines in .htaccess #10254

Merged

14 tasks

weizhouapache reviewed Feb 3, 2025

View reviewed changes

hsato03 mentioned this pull request Feb 3, 2025

Refactor alert email generation method #8831

Merged

13 tasks

DaanHoogland reviewed Feb 4, 2025

View reviewed changes

weizhouapache mentioned this pull request Feb 9, 2025

Validate the direct downloaded template file format (QCOW2) if the template file exists #10332

Merged

14 tasks

Update unit tests

c5b1e38

DaanHoogland added this to the 4.20.1 milestone Feb 13, 2025

DaanHoogland reviewed Feb 13, 2025

View reviewed changes

Apply suggestions from code review

f3e28fe

Co-authored-by: dahn <daan.hoogland@gmail.com>

weizhouapache approved these changes Feb 14, 2025

View reviewed changes

Pearl1594 approved these changes Feb 18, 2025

View reviewed changes

Pearl1594 merged commit 6a3314c into apache:4.20 Feb 18, 2025
24 of 26 checks passed

Pearl1594 deleted the fixup-logging-and-alerting branch February 18, 2025 14:23

Pearl1594 added this to ACS 4.20.1 Mar 17, 2025

Pearl1594 moved this to Done in ACS 4.20.1 Mar 17, 2025

Fixup alerting and logging error in BGPServiceImpl #10252

Fixup alerting and logging error in BGPServiceImpl #10252

Uh oh!

Conversation

vishesh92 commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Improvements to AlertManagerImpl:

Improvements to BGPServiceImpl:

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

vishesh92 commented Jan 23, 2025

Uh oh!

blueorangutan commented Jan 23, 2025

Uh oh!

codecov bot commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

weizhouapache commented Jan 23, 2025

Uh oh!

blueorangutan commented Jan 23, 2025

Uh oh!

blueorangutan commented Jan 23, 2025

Uh oh!

blueorangutan commented Jan 23, 2025

Uh oh!

vishesh92 commented Jan 24, 2025

Uh oh!

blueorangutan commented Jan 24, 2025

Uh oh!

blueorangutan commented Jan 24, 2025

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

weizhouapache commented Feb 4, 2025

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

vishesh92 commented Feb 13, 2025

Uh oh!

blueorangutan commented Feb 13, 2025

Uh oh!

blueorangutan commented Feb 13, 2025

Uh oh!

vishesh92 commented Feb 13, 2025

Uh oh!

blueorangutan commented Feb 13, 2025

Uh oh!

blueorangutan commented Feb 13, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vishesh92 commented Feb 13, 2025

Uh oh!

blueorangutan commented Feb 13, 2025

Uh oh!

blueorangutan commented Feb 13, 2025

Uh oh!

Pearl1594 commented Feb 14, 2025

Uh oh!

blueorangutan commented Feb 14, 2025

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Feb 15, 2025

Uh oh!

Pearl1594 left a comment

vishesh92 commented Jan 23, 2025 •

edited

Loading

Improvements to `AlertManagerImpl`:

Improvements to `BGPServiceImpl`:

codecov bot commented Jan 23, 2025 •

edited

Loading