Skip to content

Fix exporter disabling v2#1023

Merged
theyoprst merged 4 commits intodevfrom
fix-exporter-disabling-v2
Jun 19, 2025
Merged

Fix exporter disabling v2#1023
theyoprst merged 4 commits intodevfrom
fix-exporter-disabling-v2

Conversation

@theyoprst
Copy link
Collaborator

@theyoprst theyoprst commented Jun 19, 2025

Implemented proper cleanup of exporter resources when the Slurm exporter is disabled. Previously, when SlurmExporter.Enabled was set to false, the reconciliation would skip creating new resources but wouldn't remove existing ones, leading to inconsistent cluster state.

Changes

  • Modified ReconcileSoperatorExporter to properly handle both enabled and disabled states
  • Added Cleanup methods to reconcilers for ServiceAccount, Role, RoleBinding, PodMonitor, and Deployment resources
  • Simplified Reconcile methods

Issue

#1024

@theyoprst theyoprst requested a review from Copilot June 19, 2025 14:21
Copy link
Contributor

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 exporter rendering and reconciliation to support clean disable flows (v2), unifying naming conventions and switching from error returns to cleanup methods.

  • Removed error returns for disabled exporters; controllers now call Cleanup when disabled.
  • Renamed unexported helper functions (buildExporter*) to exported (BuildExporter*) for consistency.
  • Updated all controller reconcile loops to use value‐based Reconcile signatures and new Cleanup methods.

Reviewed Changes

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

Show a summary per file
File Description
internal/render/exporter/pod_monitor.go Changed RenderPodMonitor to return value and removed error path for disabled state.
internal/render/exporter/names.go Renamed helper functions to exported BuildExporter*.
internal/controller/reconciler/pod_monitor.go Switched reconcile to use Cleanup and removed nil‐check branch.
internal/controller/reconciler/k8s_* Unified Reconcile signatures to accept values, added Cleanup methods.
internal/controller/clustercontroller/soperator_exporter.go Added enable/disable branches to call Reconcile or Cleanup.
Comments suppressed due to low confidence (2)

internal/controller/reconciler/k8s_rolebinging.go:1

  • Filename has a typo 'rolebinging'; consider renaming it to 'k8s_rolebinding.go' to match the RoleBinding resource and maintain consistency.
package reconciler

internal/controller/reconciler/pod_monitor.go:63

  • Add import for "k8s.io/apimachinery/pkg/api/errors" as apierrors; without it, apierrors.IsNotFound will not be recognized and the code will not compile.
	if apierrors.IsNotFound(err) {

@theyoprst theyoprst merged commit 782de3f into dev Jun 19, 2025
4 checks passed
@asteny asteny deleted the fix-exporter-disabling-v2 branch June 23, 2025 13:46
@asteny asteny added the fix label Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants