Skip to content

Conversation

weizhouapache
Copy link
Member

@weizhouapache weizhouapache commented Jul 14, 2025

Description

This PR fixes #10088

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

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

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

Copy link

codecov bot commented Jul 14, 2025

Codecov Report

❌ Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 15.18%. Comparing base (48648d4) to head (56f5ae9).

Files with missing lines Patch % Lines
.../resource/virtualnetwork/model/MonitorService.java 0.00% 3 Missing ⚠️
...ork/router/VirtualNetworkApplianceManagerImpl.java 0.00% 3 Missing ⚠️
...ualnetwork/facade/SetMonitorServiceConfigItem.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               4.19   #11190   +/-   ##
=========================================
  Coverage     15.18%   15.18%           
- Complexity    11370    11375    +5     
=========================================
  Files          5415     5415           
  Lines        476073   476079    +6     
  Branches      58125    58126    +1     
=========================================
+ Hits          72290    72307   +17     
+ Misses       395695   395681   -14     
- Partials       8088     8091    +3     
Flag Coverage Δ
uitests 4.28% <ø> (ø)
unittests 15.91% <0.00%> (+<0.01%) ⬆️

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@weizhouapache
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@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
Copy link

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

@sureshanaparti sureshanaparti requested a review from Copilot July 23, 2025 17:48
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes issue #10088 by modifying the VPC router monitoring behavior to only check SSH services when the VPC Virtual Router has no guest networks configured.

  • Adds an included_services parameter to control which services are monitored in the monitoring script
  • Implements logic to set SSH as the only included service for VPC routers without guest networks
  • Optimizes health check data retrieval by moving it earlier in the execution flow

Reviewed Changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
systemvm/debian/root/monitorServices.py Updates the monitoring script to support filtering services and optimizes health check data retrieval
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java Adds logic to set SSH as the only included service for VPC routers without guest networks
core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/MonitorService.java Adds includedServices field to the MonitorService model
core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetMonitorServiceConfigItem.java Updates configuration to handle the new included services parameter
core/src/main/java/com/cloud/agent/api/routing/SetMonitorServiceCommand.java Adds constant for the included services configuration key

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

clgtm

@DaanHoogland DaanHoogland self-assigned this Aug 25, 2025
@weizhouapache weizhouapache marked this pull request as ready for review August 25, 2025 11:21
@blueorangutan
Copy link

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

Test Result Time (s) Test File
test_isolate_network_password_server Failure 15.45 test_password_server.py
test_01_secure_vm_migration Error 135.47 test_vm_life_cycle.py
test_01_secure_vm_migration Error 135.48 test_vm_life_cycle.py

@weizhouapache weizhouapache marked this pull request as draft August 27, 2025 09:26
@weizhouapache
Copy link
Member Author

moved to draft
let's wait for #10710

@weizhouapache weizhouapache marked this pull request as ready for review September 2, 2025 09:48
@weizhouapache
Copy link
Member Author

since #10710 is targeted to main/4.22, open this PR for 4.19/4.20

cc @DaanHoogland

@weizhouapache weizhouapache added this to the 4.20.2 milestone Sep 3, 2025
@weizhouapache
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VR Healthcheck Fails when a VPC is created, but there are no VMs created in the VPC yet.
3 participants