Merge pull request #347 from bergmannf/OSD-27978-metrics-return-type

[OSD-27978] Change investigation return type

Author: openshift-merge-bot[bot]

.editorconfig

@@ -4,8 +4,8 @@ root = true
 end_of_line = lf
 charset = utf-8
 trim_trailing_whitespace = true
-insert_final_newline = false
+insert_final_newline = true
 
 [*.go]
 indent_style = tab
-tab_width = 4
+tab_width = 4

cadctl/cmd/investigate/investigate.go

@@ -120,13 +120,18 @@ func run(_ *cobra.Command, _ []string) error {
 
 	customerAwsClient, err := managedcloud.CreateCustomerAWSClient(cluster, ocmClient)
 	if err != nil {
-		return ccam.Evaluate(cluster, err, ocmClient, pdClient, alertInvestigation.Name)
+		ccamResources := &investigation.Resources{Cluster: cluster, ClusterDeployment: clusterDeployment, AwsClient: customerAwsClient, OcmClient: ocmClient, PdClient: pdClient, AdditionalResources: map[string]interface{}{"error": err}}
+		result, err := ccam.Investigate(ccamResources)
+		updateMetrics(alertInvestigation.Name, &result)
+		return err
 	}
 
 	investigationResources := &investigation.Resources{Cluster: cluster, ClusterDeployment: clusterDeployment, AwsClient: customerAwsClient, OcmClient: ocmClient, PdClient: pdClient}
 
 	logging.Infof("Starting investigation for %s", alertInvestigation.Name)
-	return alertInvestigation.Run(investigationResources)
+	result, err := alertInvestigation.Run(investigationResources)
+	updateMetrics(alertInvestigation.Name, &result)
+	return err
 }
 
 // GetOCMClient will retrieve the OcmClient from the 'ocm' package
@@ -170,3 +175,15 @@ func clusterRequiresInvestigation(cluster *cmv1.Cluster, pdClient *pagerduty.Sdk
 	}
 	return true, nil
 }
+
+func updateMetrics(investigationName string, result *investigation.InvestigationResult) {
+	if result.ServiceLogSent.Performed {
+		metrics.Inc(metrics.ServicelogSent, append([]string{investigationName}, result.ServiceLogSent.Labels...)...)
+	}
+	if result.ServiceLogPrepared.Performed {
+		metrics.Inc(metrics.ServicelogPrepared, append([]string{investigationName}, result.ServiceLogPrepared.Labels...)...)
+	}
+	if result.LimitedSupportSet.Performed {
+		metrics.Inc(metrics.LimitedSupportSet, append([]string{investigationName}, result.LimitedSupportSet.Labels...)...)
+	}
+}

pkg/investigations/ccam/ccam.go

@@ -7,10 +7,9 @@ import (
 	"regexp"
 
 	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
+	investigation "github.com/openshift/configuration-anomaly-detection/pkg/investigations"
 	"github.com/openshift/configuration-anomaly-detection/pkg/logging"
-	"github.com/openshift/configuration-anomaly-detection/pkg/metrics"
 	"github.com/openshift/configuration-anomaly-detection/pkg/ocm"
-	"github.com/openshift/configuration-anomaly-detection/pkg/pagerduty"
 )
 
 var ccamLimitedSupport = &ocm.LimitedSupportReason{
@@ -20,14 +19,22 @@ var ccamLimitedSupport = &ocm.LimitedSupportReason{
 
 // Evaluate estimates if the awsError is a cluster credentials are missing error. If it determines that it is,
 // the cluster is placed into limited support (if the cluster state allows it), otherwise an error is returned.
-func Evaluate(cluster *cmv1.Cluster, bpError error, ocmClient ocm.Client, pdClient pagerduty.Client, alertType string) error {
+func Investigate(r *investigation.Resources) (investigation.InvestigationResult, error) {
+	result := investigation.InvestigationResult{}
+	cluster := r.Cluster
+	ocmClient := r.OcmClient
+	pdClient := r.PdClient
+	bpError, ok := r.AdditionalResources["error"].(error)
+	if !ok {
+		return result, fmt.Errorf("Missing required CCAM field 'error'")
+	}
 	logging.Info("Investigating possible missing cloud credentials...")
 
 	if customerRemovedPermissions := customerRemovedPermissions(bpError.Error()); !customerRemovedPermissions {
 		// We aren't able to jumpRole because of an error that is different than
 		// a removed support role/policy or removed installer role/policy
 		// This would normally be a backplane failure.
-		return fmt.Errorf("credentials are there, error is different: %w", bpError)
+		return result, fmt.Errorf("credentials are there, error is different: %w", bpError)
 	}
 
 	// The jumprole failed because of a missing support role/policy:
@@ -37,20 +44,21 @@ func Evaluate(cluster *cmv1.Cluster, bpError error, ocmClient ocm.Client, pdClie
 	switch cluster.State() {
 	case cmv1.ClusterStateReady:
 		// Cluster is in functional sate but we can't jumprole to it: post limited support
-		metrics.Inc(metrics.LimitedSupportSet, alertType, ccamLimitedSupport.Summary)
+		result.LimitedSupportSet.Performed = true
+		result.LimitedSupportSet.Labels = []string{ccamLimitedSupport.Summary}
 		err := ocmClient.PostLimitedSupportReason(ccamLimitedSupport, cluster.ID())
 		if err != nil {
-			return fmt.Errorf("could not post limited support reason for %s: %w", cluster.Name(), err)
+			return result, fmt.Errorf("could not post limited support reason for %s: %w", cluster.Name(), err)
 		}
 
-		return pdClient.SilenceIncidentWithNote(fmt.Sprintf("Added the following Limited Support reason to cluster: %#v. Silencing alert.\n", ccamLimitedSupport))
+		return result, pdClient.SilenceIncidentWithNote(fmt.Sprintf("Added the following Limited Support reason to cluster: %#v. Silencing alert.\n", ccamLimitedSupport))
 	case cmv1.ClusterStateUninstalling:
 		// A cluster in uninstalling state should not alert primary - we just skip this
-		return pdClient.SilenceIncidentWithNote(fmt.Sprintf("Skipped adding limited support reason '%s': cluster is already uninstalling.", ccamLimitedSupport.Summary))
+		return result, pdClient.SilenceIncidentWithNote(fmt.Sprintf("Skipped adding limited support reason '%s': cluster is already uninstalling.", ccamLimitedSupport.Summary))
 	default:
 		// Anything else is an unknown state to us and/or requires investigation.
 		// E.g. we land here if we run into a CPD alert where credentials were removed (installing state) and don't want to put it in LS yet.
-		return pdClient.EscalateIncidentWithNote(fmt.Sprintf("Cluster has invalid cloud credentials (support role/policy is missing) and the cluster is in state '%s'. Please investigate.", cluster.State()))
+		return result, pdClient.EscalateIncidentWithNote(fmt.Sprintf("Cluster has invalid cloud credentials (support role/policy is missing) and the cluster is in state '%s'. Please investigate.", cluster.State()))
 	}
 }
 

pkg/investigations/ccam/ccam_test.go

@@ -3,11 +3,23 @@ package ccam
 import (
 	"errors"
 	"testing"
+
+	investigation "github.com/openshift/configuration-anomaly-detection/pkg/investigations"
 )
 
 func TestEvaluateRandomError(t *testing.T) {
 	timeoutError := errors.New("credentials are there, error is different: timeout")
-	err := Evaluate(nil, errors.New("timeout"), nil, nil, "")
+	input := investigation.Resources{
+		Cluster:           nil,
+		ClusterDeployment: nil,
+		AwsClient:         nil,
+		OcmClient:         nil,
+		PdClient:          nil,
+		AdditionalResources: map[string]interface{}{
+			"error": errors.New("timeout"),
+		},
+	}
+	_, err := Investigate(&input)
 	if err.Error() != timeoutError.Error() {
 		t.Fatalf("Expected error %v, but got %v", timeoutError, err)
 	}

pkg/investigations/chgm/chgm.go

@@ -10,7 +10,6 @@ import (
 	"github.com/openshift/configuration-anomaly-detection/pkg/aws"
 	investigation "github.com/openshift/configuration-anomaly-detection/pkg/investigations"
 	"github.com/openshift/configuration-anomaly-detection/pkg/logging"
-	"github.com/openshift/configuration-anomaly-detection/pkg/metrics"
 	"github.com/openshift/configuration-anomaly-detection/pkg/networkverifier"
 	"github.com/openshift/configuration-anomaly-detection/pkg/notewriter"
 	"github.com/openshift/configuration-anomaly-detection/pkg/ocm"
@@ -23,8 +22,6 @@ import (
 )
 
 var (
-	investigationName = "ClusterHasGoneMissing"
-
 	chgmSL = ocm.ServiceLog{
 		Severity:     "Critical",
 		Summary:      "Action required: cluster not checking in",
@@ -40,21 +37,23 @@ var (
 )
 
 // Investigate runs the investigation for a triggered chgm pagerduty event
-func Investigate(r *investigation.Resources) error {
+func Investigate(r *investigation.Resources) (investigation.InvestigationResult, error) {
+	result := investigation.InvestigationResult{}
 	notes := notewriter.New("CHGM", logging.RawLogger)
 
 	// 1. Check if the user stopped instances
 	res, err := investigateStoppedInstances(r.Cluster, r.ClusterDeployment, r.AwsClient, r.OcmClient)
 	if err != nil {
-		return r.PdClient.EscalateIncidentWithNote(fmt.Sprintf("InvestigateInstances failed: %s\n", err.Error()))
+		return result, r.PdClient.EscalateIncidentWithNote(fmt.Sprintf("InvestigateInstances failed: %s\n", err.Error()))
 	}
 	logging.Debugf("the investigation returned: [infras running: %d] - [masters running: %d]", res.RunningInstances.Infra, res.RunningInstances.Master)
 
 	if !res.UserAuthorized {
 		logging.Infof("Instances were stopped by unauthorized user: %s / arn: %s", res.User.UserName, res.User.IssuerUserName)
-		return utils.WithRetries(func() error {
+		return result, utils.WithRetries(func() error {
 			err := postChgmSLAndSilence(r.Cluster.ID(), r.OcmClient, r.PdClient)
-			metrics.Inc(metrics.ServicelogSent, investigationName)
+			// XXX: metrics.Inc(metrics.ServicelogSent, investigationName)
+			result.ServiceLogSent = investigation.InvestigationStep{Performed: true, Labels: nil}
 
 			return err
 		})
@@ -89,21 +88,23 @@ func Investigate(r *investigation.Resources) error {
 		if strings.Contains(failureReason, "nosnch.in") {
 			err := r.OcmClient.PostLimitedSupportReason(&egressLS, r.Cluster.ID())
 			if err != nil {
-				return err
+				return result, err
 			}
 
-			metrics.Inc(metrics.LimitedSupportSet, investigationName, "EgressBlocked")
+			// XXX: metrics.Inc(metrics.LimitedSupportSet, investigationName, "EgressBlocked")
+			result.LimitedSupportSet = investigation.InvestigationStep{Performed: true, Labels: []string{"EgressBlocked"}}
 
 			notes.AppendAutomation("Egress `nosnch.in` blocked, sent limited support.")
-			return r.PdClient.SilenceIncidentWithNote(notes.String())
+			return result, r.PdClient.SilenceIncidentWithNote(notes.String())
 		}
 
 		err := r.OcmClient.PostServiceLog(r.Cluster.ID(), createEgressSL(failureReason))
 		if err != nil {
-			return err
+			return result, err
 		}
 
-		metrics.Inc(metrics.ServicelogSent, investigationName)
+		// XXX: metrics.Inc(metrics.ServicelogSent, investigationName)
+		result.ServiceLogSent = investigation.InvestigationStep{Performed: true, Labels: nil}
 
 		notes.AppendWarning("NetworkVerifier found unreachable targets and sent the SL, but deadmanssnitch is not blocked! \n⚠️ Please investigate this cluster.\nUnreachable: \n%s", failureReason)
 	case networkverifier.Success:
@@ -112,7 +113,7 @@ func Investigate(r *investigation.Resources) error {
 	}
 
 	// Found no issues that CAD can handle by itself - forward notes to SRE.
-	return r.PdClient.EscalateIncidentWithNote(notes.String())
+	return result, r.PdClient.EscalateIncidentWithNote(notes.String())
 }
 
 // investigateHibernation checks if the cluster was recently woken up from