refactor(investigation): Rework ResourceBuilder for improved usability

This commit refactors the \`ResourceBuilder\` to be more user-friendly and robust. The previous implementation required callers to manage dependencies between resources manually and was prone to runtime errors if resources were not requested in the correct order.

The key changes include:
- A new \`NewResourceBuilder\` constructor now handles the initial setup of required components like OCM and PagerDuty clients, simplifying the builder's API.
- The \`Build()\` method now lazily initializes resources only when they are explicitly requested via \`With...\` methods. This includes deferring the \`GetClusterInfo\` call until necessary.
- Dependency management is now handled internally by the builder. For example, requesting an AWS client via \`WithAwsClient()\` will automatically ensure the cluster object is also built.
- Investigation \`Run\` methods have been updated to explicitly declare their resource dependencies using the new builder methods.

This refactoring resolves several build and runtime errors, improves the developer experience by providing a clearer and safer API, and makes investigations easier to write and maintain.

Co-authored-by: Aider <using Gemini>

Author: bergmannf

.gitignore

@@ -10,3 +10,4 @@ e2e-suite.test
 payload
 test/testinfra/*.log
 test/testinfra/*.pem
+.aider*

cadctl/cmd/investigate/investigate.go

@@ -28,7 +28,6 @@ import (
 	"github.com/openshift/configuration-anomaly-detection/pkg/investigations/precheck"
 	"github.com/openshift/configuration-anomaly-detection/pkg/logging"
 	"github.com/openshift/configuration-anomaly-detection/pkg/metrics"
-	ocm "github.com/openshift/configuration-anomaly-detection/pkg/ocm"
 	"github.com/openshift/configuration-anomaly-detection/pkg/pagerduty"
 
 	"github.com/spf13/cobra"
@@ -101,26 +100,28 @@ func run(cmd *cobra.Command, _ []string) error {
 		return err
 	}
 
-	ocmClient, err := ocm.New()
+	builder, err := investigation.NewResourceBuilder(pdClient, clusterID, alertInvestigation.Name(), logLevelFlag, pipelineNameEnv)
 	if err != nil {
-		return fmt.Errorf("could not initialize ocm client: %w", err)
+		return fmt.Errorf("failed to create resource builder: %w", err)
 	}
 
-	builder := &investigation.ResourceBuilderT{}
 	defer func() {
 		if err != nil {
 			handleCADFailure(err, builder, pdClient)
 		}
 	}()
 
-	// Prime the builder with information required for all investigations.
-	builder.WithName(alertInvestigation.Name()).WithCluster(clusterID).WithPagerDutyClient(pdClient).WithOcmClient(ocmClient).WithLogger(logLevelFlag, pipelineNameEnv)
-
 	precheck := precheck.ClusterStatePrecheck{}
 	result, err := precheck.Run(builder)
-	if err != nil && strings.Contains(err.Error(), "no cluster found") {
-		logging.Warnf("No cluster found with ID '%s'. Escalating and exiting.", clusterID)
-		return pdClient.EscalateIncidentWithNote("CAD was unable to find the incident cluster in OCM. An alert for a non-existing cluster is unexpected. Please investigate manually.")
+	if err != nil {
+		if strings.Contains(err.Error(), "no cluster found") {
+			logging.Warnf("No cluster found with ID '%s'. Escalating and exiting.", clusterID)
+			return pdClient.EscalateIncidentWithNote("CAD was unable to find the incident cluster in OCM. An alert for a non-existing cluster is unexpected. Please investigate manually.")
+		}
+		return err
+	}
+	if result.StopInvestigations {
+		return nil
 	}
 
 	ccamInvestigation := ccam.Investigation{}
@@ -140,7 +141,7 @@ func run(cmd *cobra.Command, _ []string) error {
 	return updateIncidentTitle(pdClient)
 }
 
-func handleCADFailure(err error, rb *investigation.ResourceBuilderT, pdClient *pagerduty.SdkClient) {
+func handleCADFailure(err error, rb investigation.ResourceBuilder, pdClient *pagerduty.SdkClient) {
 	logging.Errorf("CAD investigation failed: %v", err)
 
 	var notes string

pkg/investigations/cpd/cpd.go

@@ -29,7 +29,7 @@ var byovpcRoutingSL = &ocm.ServiceLog{Severity: "Major", Summary: "Installation
 // In the future, we want to automate service logs based on the network verifier output.
 func (c *Investigation) Run(rb investigation.ResourceBuilder) (investigation.InvestigationResult, error) {
 	result := investigation.InvestigationResult{}
-	r, err := rb.WithClusterDeployment().Build()
+	r, err := rb.WithClusterDeployment().WithAwsClient().Build()
 	if err != nil {
 		return result, err
 	}

pkg/investigations/investigation/investigation.go

@@ -1,7 +1,6 @@
 package investigation
 
 import (
-	"errors"
 	"fmt"
 
 	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
@@ -29,6 +28,28 @@ type InvestigationResult struct {
 	StopInvestigations bool
 }
 
+func NewResourceBuilder(pdClient pagerduty.Client, clusterId string, name string, logLevel string, pipelineName string) (ResourceBuilder, error) {
+	ocmClient, err := ocm.New()
+	if err != nil {
+		return nil, fmt.Errorf("could not create ocm client: %w", err)
+	}
+
+	rb := &ResourceBuilderT{
+		buildLogger:  true,
+		clusterId:    clusterId,
+		name:         name,
+		logLevel:     logLevel,
+		pipelineName: pipelineName,
+		ocmClient:    ocmClient,
+		builtResources: &Resources{
+			PdClient:  pdClient,
+			OcmClient: ocmClient,
+		},
+	}
+
+	return rb, nil
+}
+
 type Investigation interface {
 	Run(builder ResourceBuilder) (InvestigationResult, error)
 	// Please note that when adding an investigation the name and the directory currently need to be the same,
@@ -51,14 +72,10 @@ type Resources struct {
 }
 
 type ResourceBuilder interface {
-	WithCluster(clusterId string) ResourceBuilder
+	WithCluster() ResourceBuilder
 	WithClusterDeployment() ResourceBuilder
 	WithAwsClient() ResourceBuilder
-	WithOcmClient(*ocm.SdkClient) ResourceBuilder
-	WithPagerDutyClient(client *pagerduty.SdkClient) ResourceBuilder
 	WithNotes() ResourceBuilder
-	WithName(name string) ResourceBuilder
-	WithLogger(logLevel string, pipelineName string) ResourceBuilder
 	Build() (*Resources, error)
 }
 
@@ -69,87 +86,50 @@ type ResourceBuilderT struct {
 	buildNotes             bool
 	buildLogger            bool
 
-	// These clients are required for all investigations and all clusters, so they are pre-filled.
-	pdClient  *pagerduty.SdkClient
-	ocmClient *ocm.SdkClient
-
 	clusterId    string
 	name         string
 	logLevel     string
 	pipelineName string
 
+	ocmClient *ocm.SdkClient
+
 	// cache
 	builtResources *Resources
 	buildErr       error
 }
 
-func (r *ResourceBuilderT) WithCluster(clusterId string) ResourceBuilder {
+func (r *ResourceBuilderT) WithCluster() ResourceBuilder {
 	r.buildCluster = true
-	r.clusterId = clusterId
 	return r
 }
 
 func (r *ResourceBuilderT) WithClusterDeployment() ResourceBuilder {
+	r.WithCluster()
 	r.buildClusterDeployment = true
 	return r
 }
 
 func (r *ResourceBuilderT) WithAwsClient() ResourceBuilder {
+	r.WithCluster()
 	r.buildAwsClient = true
 	return r
 }
 
-func (r *ResourceBuilderT) WithOcmClient(client *ocm.SdkClient) ResourceBuilder {
-	r.ocmClient = client
-	return r
-}
-
-func (r *ResourceBuilderT) WithPagerDutyClient(client *pagerduty.SdkClient) ResourceBuilder {
-	r.pdClient = client
-	return r
-}
-
 func (r *ResourceBuilderT) WithNotes() ResourceBuilder {
 	r.buildNotes = true
 	return r
 }
 
-func (r *ResourceBuilderT) WithName(name string) ResourceBuilder {
-	r.name = name
-	return r
-}
-
-func (r *ResourceBuilderT) WithLogger(logLevel string, pipelineName string) ResourceBuilder {
-	r.buildLogger = true
-	r.logLevel = logLevel
-	r.pipelineName = pipelineName
-	return r
-}
-
 func (r *ResourceBuilderT) Build() (*Resources, error) {
 	if r.buildErr != nil {
 		return nil, r.buildErr
 	}
 
-	if r.builtResources == nil {
-		r.builtResources = &Resources{
-			Name:      r.name,
-			OcmClient: r.ocmClient,
-			PdClient:  r.pdClient,
-		}
-	}
+	// The Name is now set during construction.
+	r.builtResources.Name = r.name
 
 	var err error
 
-	if r.buildClusterDeployment && !r.buildCluster {
-		r.buildErr = errors.New("cannot build ClusterDeployment without Cluster")
-		return nil, r.buildErr
-	}
-	if r.buildAwsClient && !r.buildCluster {
-		r.buildErr = errors.New("cannot build AwsClient without Cluster")
-		return nil, r.buildErr
-	}
-
 	if r.buildCluster && r.builtResources.Cluster == nil {
 		r.builtResources.Cluster, err = r.ocmClient.GetClusterInfo(r.clusterId)
 		if err != nil {
@@ -183,7 +163,7 @@ func (r *ResourceBuilderT) Build() (*Resources, error) {
 
 		if r.buildLogger {
 			// Re-initialize the logger with the cluster ID.
-			logging.RawLogger = logging.InitLogger(r.logLevel, "", internalClusterId)
+			logging.RawLogger = logging.InitLogger(r.logLevel, r.pipelineName, internalClusterId)
 		}
 	}
 
@@ -200,7 +180,7 @@ type ResourceBuilderMock struct {
 	BuildError error
 }
 
-func (r *ResourceBuilderMock) WithCluster(clusterId string) ResourceBuilder {
+func (r *ResourceBuilderMock) WithCluster() ResourceBuilder {
 	return r
 }
 
@@ -212,26 +192,10 @@ func (r *ResourceBuilderMock) WithAwsClient() ResourceBuilder {
 	return r
 }
 
-func (r *ResourceBuilderMock) WithOcmClient(client *ocm.SdkClient) ResourceBuilder {
-	return r
-}
-
-func (r *ResourceBuilderMock) WithPagerDutyClient(client *pagerduty.SdkClient) ResourceBuilder {
-	return r
-}
-
 func (r *ResourceBuilderMock) WithNotes() ResourceBuilder {
 	return r
 }
 
-func (r *ResourceBuilderMock) WithName(name string) ResourceBuilder {
-	return r
-}
-
-func (r *ResourceBuilderMock) WithLogger(logLevel string, pipelineName string) ResourceBuilder {
-	return r
-}
-
 func (r *ResourceBuilderMock) Build() (*Resources, error) {
 	if r.BuildError != nil {
 		return nil, r.BuildError

pkg/investigations/machinehealthcheckunterminatedshortcircuitsre/machinehealthcheckunterminatedshortcircuitsre.go

@@ -63,7 +63,7 @@ func (i *Investigation) teardown() error {
 func (i *Investigation) Run(rb investigation.ResourceBuilder) (investigation.InvestigationResult, error) {
 	ctx := context.Background()
 	result := investigation.InvestigationResult{}
-	r, err := rb.Build()
+	r, err := rb.WithCluster().Build()
 	if err != nil {
 		return result, err
 	}

pkg/investigations/precheck/precheck.go

@@ -14,7 +14,7 @@ type ClusterStatePrecheck struct{}
 // Performs according pagerduty actions and returns whether CAD needs to investigate the cluster
 func (c *ClusterStatePrecheck) Run(rb investigation.ResourceBuilder) (investigation.InvestigationResult, error) {
 	result := investigation.InvestigationResult{}
-	r, err := rb.Build()
+	r, err := rb.WithCluster().Build()
 	if err != nil {
 		return result, err
 	}