more graceful error handling

Azure · Jul 26, 2023 · 2204604 · 2204604
1 parent 6428bb6
commit 2204604
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 12 deletions.
diff --git a/pkg/monitor/cluster/certificateexpirationstatuses.go b/pkg/monitor/cluster/certificateexpirationstatuses.go
@@ -8,6 +8,7 @@ import (
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/Azure/ARO-RP/pkg/operator"
@@ -17,30 +18,44 @@ import (
 
 // Copyright (c) Microsoft Corporation.
 // Licensed under the Apache License 2.0.
+const (
+	certificateExpirationMetricName = "certificate.expirationdate"
+	secretMissingMetricName         = "certificate.secretnotfound"
+)
 
 func (mon *Monitor) emitCertificateExpirationStatuses(ctx context.Context) error {
 	// report NotAfter dates for Geneva (always), Ingress, and API (on managed domain) certificates
 	var certs []*x509.Certificate
 
 	mdsdCert, err := mon.getCertificate(ctx, operator.SecretName, operator.Namespace, genevalogging.GenevaCertName)
-	if err != nil {
+	if kerrors.IsNotFound(err) {
+		mon.emitGauge(secretMissingMetricName, int64(1), map[string]string{
+			"secretMissing": operator.SecretName,
+		})
+	} else if err != nil {
 		return err
+	} else {
+		certs = append(certs, mdsdCert)
 	}
-	certs = append(certs, mdsdCert)
 
 	if dns.IsManagedDomain(mon.oc.Properties.ClusterProfile.Domain) {
 		infraID := mon.oc.Properties.InfraID
 		for _, secretName := range []string{infraID + "-ingress", infraID + "-apiserver"} {
 			certificate, err := mon.getCertificate(ctx, secretName, operator.Namespace, corev1.TLSCertKey)
-			if err != nil {
+			if kerrors.IsNotFound(err) {
+				mon.emitGauge(secretMissingMetricName, int64(1), map[string]string{
+					"secretMissing": secretName,
+				})
+			} else if err != nil {
 				return err
+			} else {
+				certs = append(certs, certificate)
 			}
-			certs = append(certs, certificate)
 		}
 	}
 
 	for _, cert := range certs {
-		mon.emitGauge("certificate.expirationdate", 1, map[string]string{
+		mon.emitGauge(certificateExpirationMetricName, 1, map[string]string{
 			"subject":        cert.Subject.CommonName,
 			"expirationDate": cert.NotAfter.UTC().Format(time.RFC3339),
 		})

diff --git a/pkg/monitor/cluster/certificateexpirationstatuses_test.go b/pkg/monitor/cluster/certificateexpirationstatuses_test.go
@@ -38,6 +38,7 @@ func TestEmitCertificateExpirationStatuses(t *testing.T) {
 		domain          string
 		certsPresent    []certInfo
 		wantExpirations []map[string]string
+		wantWarning     []map[string]string
 		wantErr         string
 	}{
 		{
@@ -75,18 +76,36 @@ func TestEmitCertificateExpirationStatuses(t *testing.T) {
 			},
 		},
 		{
-			name:    "returns error when cluster secret has been deleted",
-			domain:  unmanagedDomainName,
-			wantErr: `secrets "cluster" not found`,
+			name:   "emits warning metric when cluster secret has been deleted",
+			domain: unmanagedDomainName,
+			wantWarning: []map[string]string{
+				{
+					"secretMissing": "cluster",
+				},
+			},
 		},
 		{
-			name:   "returns error when managed domain secret has been deleted",
+			name:   "emits warning metric when managed domain secret has been deleted",
 			domain: managedDomainName,
 			certsPresent: []certInfo{
 				{"cluster", "geneva.certificate"},
 				{"foo12-ingress", managedDomainName},
 			},
-			wantErr: `secrets "foo12-apiserver" not found`,
+			wantExpirations: []map[string]string{
+				{
+					"subject":        "geneva.certificate",
+					"expirationDate": expirationString,
+				},
+				{
+					"subject": "contoso.aroapp.io",
+					"expirationDate": expirationString,
+				},
+			},
+			wantWarning: []map[string]string{
+				{
+					"secretMissing": "foo12-apiserver",
+				},
+			},
 		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
@@ -100,8 +119,11 @@ func TestEmitCertificateExpirationStatuses(t *testing.T) {
 			secrets = append(secrets, secretsFromCertInfo...)
 
 			m := mock_metrics.NewMockEmitter(gomock.NewController(t))
-			for _, gauge := range tt.wantExpirations {
-				m.EXPECT().EmitGauge("certificate.expirationdate", int64(1), gauge)
+			for _, w := range tt.wantWarning {
+				m.EXPECT().EmitGauge("certificate.secretnotfound", int64(1), w)
+			}
+			for _, g := range tt.wantExpirations {
+				m.EXPECT().EmitGauge(certificateExpirationMetricName, int64(1), g)
 			}
 
 			mon := buildMonitor(m, tt.domain, secrets...)