Skip to content

Conversation

@arudell
Copy link
Member

@arudell arudell commented Feb 21, 2025

Description

This pull request includes significant updates to the SdnDiagnostics module, focusing on enhancing role management, improving data collection, and refining configuration handling. The key changes are grouped below by their themes:

Role Management Enhancements:

  • Added a new Role property to the $Global:SdnDiagnostics.Config object to define the current roles for the node. (src/SdnDiagnostics.psm1)
  • Updated the Get-SdnConfigState function to support multiple roles and set the default role from the global configuration. (src/SdnDiagnostics.psm1)

Data Collection Improvements:

  • Introduced the ComputerName property to the data collection object in Start-SdnDataCollection for better tracking of nodes involved in data collection. (src/SdnDiagnostics.psm1)
  • Refined the data collection process to handle node limits and ensure proper role-based data collection. (src/SdnDiagnostics.psm1)
  • Simplified the cleanup and data collection logic, removing redundant variables and streamlining the process. (src/SdnDiagnostics.psm1) [1] [2]

Configuration Handling:

  • Replaced the method to determine the environment mode and role, using Get-EnvironmentMode and Get-EnvironmentRole instead of registry-based methods. (src/SdnDiagnostics.psm1)
  • Updated the Get-SdnConfigState function to make the Role parameter optional and default to the global configuration. (src/SdnDiagnostics.psm1)

Miscellaneous Updates:

  • Added new cmdlets to the module manifest for enhanced diagnostic capabilities. (src/SdnDiagnostics.psd1)
  • Removed unused and redundant code blocks to improve code readability and maintainability. (src/SdnDiagnostics.psm1) [1] [2]

These changes collectively enhance the functionality and maintainability of the SdnDiagnostics module by improving role management, streamlining data collection, and refining configuration handling.

Change type

  • Bug fix (non-breaking change)
  • Code style update (formatting, local variables)
  • New Feature (non-breaking change that adds new functionality without impacting existing)
  • Breaking change (fix or feature that may cause functionality impact)
  • Other

Checklist:

  • My code follows the style and contribution guidelines of this project.
  • I have tested and validated my code changes.

@arudell arudell marked this pull request as ready for review March 4, 2025 21:07
@arudell arudell requested a review from a team as a code owner March 4, 2025 21:07
@sbgms
Copy link
Contributor

sbgms commented Mar 13, 2025

        }

Why are we doing this ? Why not out-null ?


Refers to: src/SdnDiagnostics.psm1:333 in f096334. [](commit_id = f096334, deletion_comment = False)

@sbgms
Copy link
Contributor

sbgms commented Mar 13, 2025

function Get-CommonConfigState {

fyi, almost everything here is already collected in netview. so to simplify, you could always collect netview and drop this in future perhaps. Nothing for now though.


Refers to: src/modules/SdnDiag.Common.psm1:485 in f096334. [](commit_id = f096334, deletion_comment = False)

sbgms
sbgms previously approved these changes Mar 13, 2025
Copy link
Contributor

@sbgms sbgms left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Contributor

@sbgms sbgms left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Contributor

@sbgms sbgms left a comment

Choose a reason for hiding this comment

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

Looks good!

@arudell arudell added this pull request to the merge queue Mar 17, 2025
Merged via the queue into main with commit e853af5 Mar 17, 2025
4 checks passed
@arudell arudell deleted the support-multi-role branch March 17, 2025 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compress AuditLogs taken from hosts Speed up data collection as part of Start-SdnDataCollection

3 participants