Skip to content
This repository was archived by the owner on Aug 15, 2025. It is now read-only.

Conversation

@oniko94
Copy link
Contributor

@oniko94 oniko94 commented Apr 22, 2025

Change Summary

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Other (please describe):

Refactoring the resource usage (CPU and memory) checks for VPP configuration mode on verify stage for more fine-grained resource utilization when configuring.

Related Task(s)

https://vyos.dev/T7424

Related PR(s)

Proposed changes

How to test

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@oniko94 oniko94 marked this pull request as draft April 22, 2025 11:18
@sever-sever
Copy link
Contributor

Add the task number on the https://vyos.dev/ and refer the task in the commit message and PR message

@oniko94 oniko94 force-pushed the feature/VD-10-validate-resource-usage branch 2 times, most recently from 152860d to 64dc57c Compare April 29, 2025 11:31
@oniko94 oniko94 marked this pull request as ready for review April 29, 2025 11:32
Copy link
Contributor

@sever-sever sever-sever left a comment

Choose a reason for hiding this comment

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

There should be a task number on the https://vyos.dev/ and the task should be in the commit message and PR titile

@oniko94 oniko94 force-pushed the feature/VD-10-validate-resource-usage branch from 64dc57c to a3981ee Compare May 2, 2025 12:26
@oniko94 oniko94 changed the title VD-10: WIP: Refactor resource validation and broaden cases T7424: WIP: Refactor resource validation and broaden cases May 2, 2025
@oniko94
Copy link
Contributor Author

oniko94 commented May 2, 2025

There should be a task number on the https://vyos.dev/ and the task should be in the commit message and PR titile

@sever-sever done and linked

@oniko94 oniko94 force-pushed the feature/VD-10-validate-resource-usage branch 6 times, most recently from ed335e7 to b7599bb Compare May 2, 2025 16:25
@oniko94 oniko94 requested a review from sever-sever May 2, 2025 16:25
@oniko94 oniko94 force-pushed the feature/VD-10-validate-resource-usage branch from b7599bb to 0079ff5 Compare May 13, 2025 09:01
@oniko94 oniko94 force-pushed the feature/VD-10-validate-resource-usage branch 5 times, most recently from 08ef111 to ff79b95 Compare May 13, 2025 14:13
@sever-sever sever-sever requested a review from Copilot May 13, 2025 19:20
Copy link

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 refactors the resource validation logic for VPP configuration by modularizing CPU and memory checks and enhancing error messages for better granularity.

  • Refactored interface address extraction in NAT configuration.
  • Updated CPU and memory resource validations by introducing dedicated helper functions across multiple modules.
  • Removed redundant memory checks and improved consistency in error messaging.

Reviewed Changes

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

Show a summary per file
File Description
src/conf_mode/vpp_nat.py Reformatted chained method calls for clarity in interface address retrieval.
src/conf_mode/vpp.py Updated resource validations and removed deprecated memory check functions.
python/vyos/vpp/config_verify.py Added new functions to verify minimal CPU/memory requirements and resource usage.
python/vyos/vpp/config_resource_checks/memory.py Introduced functions for calculating and validating memory consumption.
python/vyos/vpp/config_resource_checks/cpu.py Added functions for CPU core and worker validations.
python/vyos/vpp/config_resource_checks/constants.py Defined constants for resource thresholds and defaults.
Comments suppressed due to low confidence (2)

python/vyos/vpp/config_verify.py:263

  • The parameter 'workers' is annotated as a string but is treated as an iterable of worker range strings. Consider updating its type annotation to List[str] to accurately reflect its expected usage.
def verify_vpp_settings_cpu_corelist_workers(cpus: int, main_core: int, workers: str) -> int:

python/vyos/vpp/config_resource_checks/memory.py:107

  • The function 'available_core_count' is used here but not imported. Please update the import statement to import 'available_core_count' from 'vyos.vpp.config_resource_checks.cpu' or use an already imported equivalent.
'memory_buffers': buffer_size(settings, available_core_count()),

@oniko94 oniko94 force-pushed the feature/VD-10-validate-resource-usage branch from 0271a74 to f9bfcdd Compare May 19, 2025 13:38
@oniko94 oniko94 force-pushed the feature/VD-10-validate-resource-usage branch from f9bfcdd to 566b706 Compare May 20, 2025 13:45
@oniko94 oniko94 requested a review from mykolabas May 20, 2025 14:07
@oniko94 oniko94 force-pushed the feature/VD-10-validate-resource-usage branch from 638719c to 1a6f9f8 Compare May 21, 2025 15:42
@sever-sever sever-sever requested a review from Copilot May 21, 2025 21:48

This comment was marked as outdated.

@oniko94 oniko94 force-pushed the feature/VD-10-validate-resource-usage branch from 1a6f9f8 to e4c2efe Compare May 22, 2025 08:11
@sever-sever sever-sever requested review from natali-rs1985 and removed request for mykolabas May 22, 2025 11:52
@oniko94 oniko94 force-pushed the feature/VD-10-validate-resource-usage branch 2 times, most recently from 052d3d7 to fea23b7 Compare May 27, 2025 11:08
@oniko94 oniko94 requested a review from mykolabas May 27, 2025 11:13
…rements for test environments

T7424: Fix errors in calculating the skipped and reserved CPU cores; Adjust default main heap size value.
@oniko94 oniko94 force-pushed the feature/VD-10-validate-resource-usage branch from fea23b7 to d032a22 Compare May 27, 2025 11:33
@oniko94 oniko94 marked this pull request as draft May 27, 2025 12:18
@oniko94 oniko94 force-pushed the feature/VD-10-validate-resource-usage branch from c2ec32f to 5dd9d91 Compare May 29, 2025 18:47
@sever-sever sever-sever removed the request for review from mykolabas May 29, 2025 20:29
@oniko94 oniko94 force-pushed the feature/VD-10-validate-resource-usage branch from 5dd9d91 to 96b9881 Compare June 4, 2025 08:18
@oniko94 oniko94 requested a review from mykolabas June 4, 2025 08:23
T7424: Fix CPU reserve and skip cores calculations; Add total CPU usage check

T7424: Refactor smoketests to reflect new logic
@oniko94 oniko94 force-pushed the feature/VD-10-validate-resource-usage branch from 96b9881 to ca347e7 Compare June 4, 2025 13:36
@oniko94 oniko94 marked this pull request as ready for review June 4, 2025 14:26
@sever-sever sever-sever requested a review from Copilot June 10, 2025 20:12
Copy link

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 refactors the VPP resource validation logic by replacing direct memory and CPU checks with new, modular verification functions, and updates the related test cases for CPU settings.

  • Refactored VPP resource checks in configuration and verification modules.
  • Introduced new functions to verify minimal CPU, memory, and other VPP-specific settings in the configuration.
  • Updated smoketest cases to reflect the updated validation logic for CPU workers and core lists.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/conf_mode/vpp.py Removed unused imports and integrated new resource verification APIs.
smoketest/scripts/cli/test_vpp.py Adjusted test values to match refactored CPU settings.
python/vyos/vpp/config_verify.py Added several new verification functions for CPU and memory checks.
python/vyos/vpp/config_resource_checks/resource_defaults.py Defined default values for resource consumption checks.
python/vyos/vpp/config_resource_checks/memory.py Added functions for memory consumption calculations.
python/vyos/vpp/config_resource_checks/cpu.py Added CPU core validation functions and updated available cores logic.
interface-definitions/vpp.xml.in Updated XML defaults for memory sizes on VPP interface definitions.
Comments suppressed due to low confidence (1)

src/conf_mode/vpp.py:449

  • The function verify_vpp_settings_cpu_corelist_workers is defined to expect the 'workers' parameter as a string, yet a list is passed from cpu_settings. Consider updating the parameter type annotation (or the test data) to consistently use a list for corelist-workers.
if 'corelist_workers' in cpu_settings:

@oniko94 oniko94 closed this Jun 11, 2025
@oniko94 oniko94 deleted the feature/VD-10-validate-resource-usage branch June 11, 2025 10:22
@sever-sever
Copy link
Contributor

@oniko94, why was it closed?

@oniko94 oniko94 changed the title T7424: WIP: Refactor resource validation and broaden cases T7424: Refactor resource validation and broaden cases Jun 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants