feat(storage): netapp-monitoring plugin by RockSolidScripts · Pull Request #1728 · cloudoperators/greenhouse-extensions

RockSolidScripts · 2026-06-11T15:35:14Z

Submit a pull request

Thank you for submitting a pull request!
To speed up the review process, please ensure that everything below
is true:

This is not a duplicate of an existing Plugin.
No existing features have been broken without good reason.
The Documentation has been updated to reflect your changes.
Tests have been added or updated to reflect your changes.
All tests pass.

Replace any ":question:" below with information about your pull request.

Pull Request Details

Provide details about your pull request and what it adds, fixes, or changes

This plugin deploys a monitoring stack for NetApp storage systems using NetApp Harvest. Harvest collects performance, capacity, and health metrics from ONTAP systems and exposes them via a Prometheus exporter.

The chart includes a service discovery component (netappsd) that automatically discovers NetApp filers from Netbox and spawns Harvest instances to collect metrics.

Breaking Changes

Describe what features are broken by this pull request and why, if any.

None

Issues Fixed

Enter the issue numbers resolved by this pull request below, if any.

None

Other Relevant Information

Provide any other important details below.

None

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a new netapp-monitoring Helm chart/plugin that deploys NetApp Harvest along with a netappsd service discovery component to dynamically discover filers from Netbox and run per-app master/worker workloads.

Changes:

Introduces the Helm chart scaffolding (Chart/values/helpers) and Greenhouse PluginDefinition.
Adds Kubernetes manifests (Deployments/Services/SA+RBAC/Secrets/ConfigMap) for netappsd master/worker and Harvest pollers.
Documents usage and configuration in a new README.

Reviewed changes

Copilot reviewed 14 out of 16 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
netapp-monitoring/values.yaml	Adds default values for Harvest + netappsd, apps map, and ownership metadata
netapp-monitoring/templates/harvest-netappsd-worker-service.yaml	Adds per-app worker Service
netapp-monitoring/templates/harvest-netappsd-worker-deployment.yaml	Adds per-app worker Deployment (Harvest poller + netappsd worker)
netapp-monitoring/templates/harvest-netappsd-serviceaccount.yaml	Adds ServiceAccount + Role/RoleBinding for netappsd
netapp-monitoring/templates/harvest-netappsd-secret.yaml	Adds Secret for netappsd runtime credentials/config
netapp-monitoring/templates/harvest-netappsd-master-service.yaml	Adds per-app master Service
netapp-monitoring/templates/harvest-netappsd-master-deployment.yaml	Adds per-app master Deployment
netapp-monitoring/templates/harvest-netappsd-configmap.yaml	Adds ConfigMap with poller start script and Harvest template
netapp-monitoring/templates/harvest-basic-auth.yaml	Adds Secret containing Harvest credentials YAML
netapp-monitoring/templates/_helpers.tpl	Adds standard Helm helpers and credentials template helper
netapp-monitoring/README.md	Adds documentation for architecture and configuration
netapp-monitoring/PluginDefinition.yaml	Adds Greenhouse PluginDefinition and option schema
netapp-monitoring/Chart.yaml	Adds chart metadata and dependency
netapp-monitoring/Chart.lock	Locks owner-info dependency
netapp-monitoring/.helmignore	Adds Helm ignore patterns

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 14 out of 16 changed files in this pull request and generated 8 comments.

RockSolidScripts · 2026-06-11T18:22:48Z

+  - apiGroups: [""]
+    resources: ["endpoints"]
+    verbs: ["get", "list"]
+  - apiGroups: [""]
+    resources: ["pods"]
+    verbs: ["get", "list", "update", "patch"]
+  - apiGroups: ["apps"]
+    resources: ["deployments"]
+    verbs: ["get", "list", "update", "patch"]


Master deployment is responsible for managing the worker pod replicas based on the service discovery.
Hence the permissions are granted at the least privileges only at the ns level.

Can you add the documentation piece please?

viennaa

Thanks for adding the plugin. Address the following issues please.

Move chart to netapp-monitoring/charts/, rename PluginDefinition.yaml → plugindefinition.yaml.
Add matrix entries to .github/workflows/helm-release.yaml and .github/workflows/ci-pr-build.yaml; add path to root kustomization.yaml.
Declare netappsd.lob and netappsd.credentials_secret in PluginDefinition.yaml, or remove their required guards. Chart currently cannot install with shipped defaults.
Reconcile required: true options vs. empty-string defaults in values.yaml so empty values fail loudly instead of silently producing empty secrets.
Add SPDX headers to all new YAML files.
Add an else { fail ... } branch to netapp-monitoring.credentials-entry.

Signed-off-by: I504010 <timo.johner@sap.com> Signed-off-by: Ganesh Kugulakrishnan <ganesh.kugulakrishnan@sap.com>

Signed-off-by: Ganesh Kugulakrishnan <ganesh.kugulakrishnan@sap.com>

Signed-off-by: Ganesh Kugulakrishnan <97788209+RockSolidScripts@users.noreply.github.com>

RockSolidScripts · 2026-06-16T19:04:23Z

Thanks for adding the plugin. Address the following issues please.

Move chart to netapp-monitoring/charts/, rename PluginDefinition.yaml → plugindefinition.yaml.

Add matrix entries to .github/workflows/helm-release.yaml and .github/workflows/ci-pr-build.yaml; add path to root kustomization.yaml.

Declare netappsd.lob and netappsd.credentials_secret in PluginDefinition.yaml, or remove their required guards. Chart currently cannot install with shipped defaults.

Reconcile required: true options vs. empty-string defaults in values.yaml so empty values fail loudly instead of silently producing empty secrets.

Add SPDX headers to all new YAML files.

Add an else { fail ... } branch to netapp-monitoring.credentials-entry.

These are addressed.

viennaa

Thanks for addressing the findings. Couple more remarks from my side.

viennaa · 2026-06-17T13:02:23Z

+  - apiGroups: [""]
+    resources: ["endpoints"]
+    verbs: ["get", "list"]
+  - apiGroups: [""]
+    resources: ["pods"]
+    verbs: ["get", "list", "update", "patch"]
+  - apiGroups: ["apps"]
+    resources: ["deployments"]
+    verbs: ["get", "list", "update", "patch"]


Can you add the documentation piece please?

viennaa · 2026-06-17T13:05:18Z

+              mountPath: /app/shared
+            - name: harvest-sd-config
+              mountPath: /app/scripts/start-poller.sh
+              subPath: start-poller.sh


is this just a startup script? since this is mounted with subPath it will only be copied at pod start. If changes occur to this configmap they need a rollout restart

Yes, just at the start. If there is any change. The startup script is a static script that won't change anywhere in the future as it just supposed to wait until the worker container able to fetch a filer from the master queue and render the harvest config file.

however, I added checksum-based rollout triggers so pod restarts happen automatically when config/secret templates change at any rare situations.

viennaa · 2026-06-17T13:05:57Z

+
+Configure the required options:
+
+| Parameter | Description | Required |


option table lists only 10 options; plugindefinition.yaml exposes 14. Add netappsd.lob, netappsd.credentials_file, netappsd.credentials_secret, apps.

viennaa · 2026-06-17T13:06:41Z

+    - name: netappsd.image.repository
+      value: keppel.eu-de-1.cloud.sap/ccloud/netappsd
+    - name: netappsd.image.tag
+      value: latest


this should be a version, even in the README example

viennaa · 2026-06-17T13:07:13Z

+          command: ["/netappsd", "master"]
+          args:
+            - --region
+            - {{ .Values.netappsd.region }}


guard this too pls

viennaa · 2026-06-17T13:08:23Z

+  selector:
+    matchLabels:
+      name: {{ include "netapp-monitoring.fullname" . }}-{{ $appName }}-master


shouldn't they use this helper?

netapp-monitoring.selectorLabels

applies to all.

These are the same for every app (cinder, manila, apod, cinder-manila). Using it as the Deployment selector would cause all master/worker Deployments to select across each other's pods, breaking the isolation.

The current name: -{{ $appName }}-master label is intentionally specific — it uniquely identifies pods per-app. That needs to stay as-is for correct pod selection, since you have multiple Deployments created from the same range loop.

Signed-off-by: Ganesh Kugulakrishnan <ganesh.kugulakrishnan@sap.com>

Copilot AI review requested due to automatic review settings June 11, 2026 15:35

RockSolidScripts requested a review from a team as a code owner June 11, 2026 15:35

Copilot AI reviewed Jun 11, 2026

View reviewed changes

RockSolidScripts force-pushed the feat/storage-netapp-monitoring-chart branch 4 times, most recently from 936701d to 6d464e0 Compare June 11, 2026 15:53

RockSolidScripts requested a review from Copilot June 11, 2026 15:54

Copilot AI reviewed Jun 11, 2026

View reviewed changes

RockSolidScripts mentioned this pull request Jun 11, 2026

storage: netapp-monitoring chart sapcc/helm-charts#11869

Closed

viennaa requested changes Jun 16, 2026

View reviewed changes

RockSolidScripts force-pushed the feat/storage-netapp-monitoring-chart branch from ce406eb to b4c8c3f Compare June 16, 2026 18:57

RockSolidScripts requested review from a team as code owners June 16, 2026 18:57

timojohlo and others added 4 commits June 16, 2026 12:59

fix(logs): add scrape annotation for metrics (cloudoperators#1727)

1bc2fce

Signed-off-by: I504010 <timo.johner@sap.com> Signed-off-by: Ganesh Kugulakrishnan <ganesh.kugulakrishnan@sap.com>

feat(storage): netapp-monitoring plugin

7545f6a

Signed-off-by: Ganesh Kugulakrishnan <ganesh.kugulakrishnan@sap.com>

feat(storage): fixes

e597ee5

Signed-off-by: Ganesh Kugulakrishnan <ganesh.kugulakrishnan@sap.com>

feat(storage): fixes

7bc9b19

Signed-off-by: Ganesh Kugulakrishnan <ganesh.kugulakrishnan@sap.com>

RockSolidScripts force-pushed the feat/storage-netapp-monitoring-chart branch from aab6c6c to 7bc9b19 Compare June 16, 2026 19:01

Merge branch 'main' into feat/storage-netapp-monitoring-chart

0d164c6

Signed-off-by: Ganesh Kugulakrishnan <97788209+RockSolidScripts@users.noreply.github.com>

RockSolidScripts requested a review from viennaa June 16, 2026 19:04

viennaa requested changes Jun 17, 2026

View reviewed changes

RockSolidScripts added 2 commits June 17, 2026 09:07

feat(storage): fixes

ba9aeb5

Signed-off-by: Ganesh Kugulakrishnan <ganesh.kugulakrishnan@sap.com>

feat(storage): fixes

e4588bd

Signed-off-by: Ganesh Kugulakrishnan <ganesh.kugulakrishnan@sap.com>

RockSolidScripts requested a review from viennaa June 17, 2026 15:25

Merge branch 'main' into feat/storage-netapp-monitoring-chart

8d9dbcb


		Configure the required options:

		\| Parameter \| Description \| Required \|

Conversation

RockSolidScripts commented Jun 11, 2026

Submit a pull request

Pull Request Details

Breaking Changes

Issues Fixed

Other Relevant Information

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

viennaa left a comment • 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!

RockSolidScripts commented Jun 16, 2026

Uh oh!

viennaa left a comment

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

viennaa left a comment •

edited

Loading