[Draft] Feat/autodiscovery application code #1217

cwadhwani-splunk · 2025-07-14T07:10:06Z

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

Dependency update
Bug fix
New feature
Refactor/improvement
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Checklist

My commit message is conventional
I have run pre-commit on all files before creating the PR
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
I have checked my code and corrected any misspellings

omrozowicz-splunk

can you add another example to rendered and run make render? this way we'll see k8s manifests of how it looks exactly

charts/splunk-connect-for-snmp/templates/NOTES.txt

splunk_connect_for_snmp/common/csv_record_manager.py

splunk_connect_for_snmp/discovery/discovery_manager.py

ajasnosz

Let's keep the commit convention names with feat, fix, docs etc.

charts/splunk-connect-for-snmp/templates/NOTES.txt

charts/splunk-connect-for-snmp/templates/worker/discovery/deployment.yaml

splunk_connect_for_snmp/common/csv_record_manager.py

ajasnosz · 2025-07-15T13:42:24Z

splunk_connect_for_snmp/common/discovery_record.py

+            return value
+
+    @validator("port", pre=True)
+    def port_validator(cls, value):


It can include the same constrains as InvenotryRecord:

if value < 1 or value > 65535: raise ValueError(f"Port out of range {value}")

This validation is already handled in the schema check, so it's not needed here.
But if this check should be validated here, it can be removed from schema check.

I think we can keep it in both places, as schema that we use only applies for the kubernetes deployment.

Added the validation here as well

…hout it

…ounting

ajasnosz · 2025-07-29T09:43:33Z

docker_compose/docker-compose.yaml

-      LOG_LEVEL: ${WORKER_LOG_LEVEL:-INFO}
+      PREFETCH_COUNT: ${PREFETCH_SENDER_COUNT:-1}
+      DISCOVERY_PATH: ${DISCOVERY_PATH:-/app/discovery/discovery_devices.csv}
+      IPv6_ENABLED: ${IPv6_ENABLED:-false}


can be switched to referencing *ipv6 in line 287

Updated the code

ajasnosz · 2025-07-29T09:46:08Z

docker_compose/docker-compose.yaml

@@ -284,13 +284,14 @@ services:
          cpus: ${WORKER_DISCOVERY_CPU_RESERVATIONS:-0.25}
          memory: ${WORKER_DISCOVERY_MEMORY_RESERVATIONS:-250M}
    environment:
-      <<: [ *general_sc4snmp_data, *splunk_general_setup, *splunk_extended_setup,
-            *pysnmp_debug, *ipv6 ]
+      <<: [*general_sc4snmp_data, *pysnmp_debug, *workers_general_setup, ]


are params from workers_general_setup necessary in discovery? I think they are more related to polling.

Correct, we don't require them all of them. We just require SC4SNMP_VERSION and LOG_LEVEL, so I thought of reusing *workers_general_setup. But since it has extra variables, updated the code accordingly.

ajasnosz · 2025-07-29T09:48:12Z

docker_compose/docker-compose.yaml

-      SC4SNMP_VERSION: ${SC4SNMP_VERSION:-latest}
-      LOG_LEVEL: ${WORKER_LOG_LEVEL:-INFO}
+      PREFETCH_COUNT: ${PREFETCH_SENDER_COUNT:-1}
+      DISCOVERY_PATH: ${DISCOVERY_PATH:-/app/discovery/discovery_devices.csv}


shouldn't this be in volumes?

This is just the path inside the pod where csv will be created. It is required inside the code to add device details. Volume for the same is present in the volume section.
Note: In the latest commit, I have changed the name of this env to DISCOVERY_CSV_PATH

ajasnosz · 2025-07-29T09:56:51Z

docker_compose/.env

+# Discovery
+DISCOVERY_ENABLE=true
+DISCOVERY_LOG_LEVEL=INFO
+DISCOVERY_CONFIG_PATH=/app/discovery/discovery-config.yaml


I'm not sure if I know which variable is responsible for what here, could you describe them in the reply:
DISCOVERY_FILE_ABSOLUTE_PATH
DISCOVERY_PATH
LOCAL_DISCOVERY_PATH
DISCOVERY_CONFIG_PATH

DISCOVERY_LOG_LEVEL - we use manage_logs.py to set up the logs for docker compose (as it is possible to sent the logs directly from docker to splunk) did you test the script with discovery?

There was some misunderstanding among the discovery-related env variables. I've removed the unnecessary ones and updated the code accordingly.
Now using below variables for discovery:

DISCOVERY_CONFIG_FILE_ABSOLUTE_PATH: the absolute path on host to the discovery config file

DISCOVERY_PATH: the path to the folder (on host) where the discovery CSV file will be created

Below envs are that are hardcoded in docker-compose.yaml file. Need these envs inside the code.

DISCOVERY_CONFIG_PATH: This is the path where customer's discovery-config.yaml will mounted inside the pod/container.

DISCOVERY_CSV_PATH: This is the path where the final output file will be stored inside the container.

I have not tested the manage_logs.py, I ll check it.

I tested the manage_logs.py file, and it is adding the logging sections with splunk as its log-driver in the discovery and worker-discovery service. I further checked that the logs from the docker container are getting ingested to Splunk.

…d unnecessary variables

cwadhwani-splunk added 2 commits July 8, 2025 18:15

feat: initial commit with application code

7f96cde

Added logic for mounting the final output csv file

3e9e95b

cwadhwani-splunk requested review from ikheifets-splunk and ajasnosz July 14, 2025 07:10

cwadhwani-splunk requested a review from omrozowicz-splunk as a code owner July 14, 2025 07:10

omrozowicz-splunk reviewed Jul 14, 2025

View reviewed changes

charts/splunk-connect-for-snmp/templates/NOTES.txt Outdated Show resolved Hide resolved

splunk_connect_for_snmp/common/csv_record_manager.py Outdated Show resolved Hide resolved

splunk_connect_for_snmp/discovery/discovery_manager.py Outdated Show resolved Hide resolved

ajasnosz requested changes Jul 15, 2025

View reviewed changes

cwadhwani-splunk added 5 commits July 17, 2025 11:06

feat: addressed review comments

9e0eebf

chore: removed the init container and handled the volume mounting wit…

3d2b522

…hout it

feat: added support for scaling discovery worker pods

0673205

chore: Updated port validation

e7c88b3

feat: Added support for snmp version 3 in docker and handled volume m…

3d1ff4d

…ounting

ajasnosz requested changes Jul 29, 2025

View reviewed changes

fix: Corrected the discovery paths in environment variable and remove…

b1840e2

…d unnecessary variables

[Draft] Feat/autodiscovery application code #1217

Are you sure you want to change the base?

[Draft] Feat/autodiscovery application code #1217

Uh oh!

Conversation

cwadhwani-splunk commented Jul 14, 2025

Description

Type of change

How Has This Been Tested?

Checklist

Uh oh!

omrozowicz-splunk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ajasnosz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cwadhwani-splunk Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cwadhwani-splunk Jul 29, 2025 •

edited

Loading