[Bugfix] Do not block reconciliation in case of Resource failure (#1193)

CHANGELOG.md

@@ -28,6 +28,7 @@
 - (Improvement) Do not check checksums for DeploymentReplicationStatus.IncomingSynchronization field values
 - (Improvement) Add ServerGroup details into ServerGroupSpec
 - (Improvement) Add Resource kerror Type
+- (Bugfix) Do not block reconciliation in case of Resource failure
 
 ## [1.2.20](https://github.com/arangodb/kube-arangodb/tree/1.2.20) (2022-10-25)
 - (Feature) Add action progress

pkg/deployment/deployment.go

@@ -326,13 +326,6 @@ func (d *Deployment) run() {
 	}
 
 	if d.GetPhase() == api.DeploymentPhaseNone {
-		// Create service monitor
-		if d.haveServiceMonitorCRD {
-			if err := d.resources.EnsureServiceMonitor(context.TODO()); err != nil {
-				d.CreateEvent(k8sutil.NewErrorEvent("Failed to create service monitor", err, d.GetAPIObject()))
-			}
-		}
-
 		// Create initial topology
 		if err := d.createInitialTopology(context.TODO()); err != nil {
 			d.CreateEvent(k8sutil.NewErrorEvent("Failed to create initial topology", err, d.GetAPIObject()))

pkg/deployment/deployment_inspector.go

@@ -34,7 +34,6 @@ import (
 	"github.com/arangodb/kube-arangodb/pkg/util/errors"
 	"github.com/arangodb/kube-arangodb/pkg/util/globals"
 	"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
-	inspectorInterface "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector"
 	"github.com/arangodb/kube-arangodb/pkg/util/k8sutil/kerrors"
 )
 
@@ -251,21 +250,8 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva
 	} else {
 		nextInterval = nextInterval.ReduceTo(x)
 	}
-
-	if err := d.resources.EnsureLeader(ctx, d.GetCachedStatus()); err != nil {
-		return minInspectionInterval, errors.Wrapf(err, "Creating leaders failed")
-	}
-
-	if err := d.resources.EnsureArangoMembers(ctx, d.GetCachedStatus()); err != nil {
-		return minInspectionInterval, errors.Wrapf(err, "ArangoMember creation failed")
-	}
-
-	if err := d.resources.EnsureServices(ctx, d.GetCachedStatus()); err != nil {
-		return minInspectionInterval, errors.Wrapf(err, "Service creation failed")
-	}
-
-	if err := d.resources.EnsureSecrets(ctx, d.GetCachedStatus()); err != nil {
-		return minInspectionInterval, errors.Wrapf(err, "Secret creation failed")
+	if err := d.resources.EnsureCoreResources(ctx, d.GetCachedStatus()); err != nil {
+		d.log.Err(err).Error("Unable to ensure core resources")
 	}
 
 	// Inspect secret hashes
@@ -308,10 +294,8 @@ func (d *Deployment) inspectDeploymentWithError(ctx context.Context, lastInterva
 		return minInspectionInterval, errors.Wrapf(err, "Reconciler immediate actions failed")
 	}
 
-	if interval, err := d.ensureResources(ctx, nextInterval, d.GetCachedStatus()); err != nil {
-		return minInspectionInterval, errors.Wrapf(err, "Reconciler resource recreation failed")
-	} else {
-		nextInterval = interval
+	if err := d.resources.EnsureResources(ctx, d.haveServiceMonitorCRD, d.GetCachedStatus()); err != nil {
+		d.log.Err(err).Error("Unable to ensure resources")
 	}
 
 	d.metrics.Agency.Fetches++
@@ -521,38 +505,6 @@ func (d *Deployment) refreshMaintenanceTTL(ctx context.Context) {
 	}
 }
 
-// ensureResources creates all required resources for the deployment
-func (d *Deployment) ensureResources(ctx context.Context, lastInterval util.Interval, cachedStatus inspectorInterface.Inspector) (util.Interval, error) {
-	// Ensure all resources are created
-	if d.haveServiceMonitorCRD {
-		if err := d.resources.EnsureServiceMonitor(ctx); err != nil {
-			return minInspectionInterval, errors.Wrapf(err, "Service monitor creation failed")
-		}
-	}
-
-	if err := d.resources.EnsurePVCs(ctx, cachedStatus); err != nil {
-		return minInspectionInterval, errors.Wrapf(err, "PVC creation failed")
-	}
-
-	if err := d.resources.EnsurePods(ctx, cachedStatus); err != nil {
-		return minInspectionInterval, errors.Wrapf(err, "Pod creation failed")
-	}
-
-	if err := d.resources.EnsurePDBs(ctx); err != nil {
-		return minInspectionInterval, errors.Wrapf(err, "PDB creation failed")
-	}
-
-	if err := d.resources.EnsureAnnotations(ctx, cachedStatus); err != nil {
-		return minInspectionInterval, errors.Wrapf(err, "Annotation update failed")
-	}
-
-	if err := d.resources.EnsureLabels(ctx, cachedStatus); err != nil {
-		return minInspectionInterval, errors.Wrapf(err, "Labels update failed")
-	}
-
-	return lastInterval, nil
-}
-
 // triggerInspection ensures that an inspection is run soon.
 func (d *Deployment) triggerInspection() {
 	d.inspectTrigger.Trigger()

pkg/deployment/resources/resources.go

@@ -20,7 +20,13 @@
 
 package resources
 
-import "github.com/arangodb/kube-arangodb/pkg/logging"
+import (
+	"context"
+
+	"github.com/arangodb/kube-arangodb/pkg/logging"
+	errors "github.com/arangodb/kube-arangodb/pkg/util/errors"
+	inspectorInterface "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector"
+)
 
 // Resources is a service that creates low level resources for members
 // and inspects low level resources, put the inspection result in members.
@@ -45,3 +51,19 @@ func NewResources(namespace, name string, context Context) *Resources {
 
 	return r
 }
+
+func (r *Resources) EnsureCoreResources(ctx context.Context, cachedStatus inspectorInterface.Inspector) error {
+	return errors.Errors(errors.Section(r.EnsureLeader(ctx, cachedStatus), "EnsureLeader"),
+		errors.Section(r.EnsureArangoMembers(ctx, cachedStatus), "EnsureArangoMembers"),
+		errors.Section(r.EnsureServices(ctx, cachedStatus), "EnsureServices"),
+		errors.Section(r.EnsureSecrets(ctx, cachedStatus), "EnsureSecrets"))
+}
+
+func (r *Resources) EnsureResources(ctx context.Context, serviceMonitorEnabled bool, cachedStatus inspectorInterface.Inspector) error {
+	return errors.Errors(errors.Section(r.EnsureServiceMonitor(ctx, serviceMonitorEnabled), "EnsureServiceMonitor"),
+		errors.Section(r.EnsurePVCs(ctx, cachedStatus), "EnsurePVCs"),
+		errors.Section(r.EnsurePods(ctx, cachedStatus), "EnsurePods"),
+		errors.Section(r.EnsurePDBs(ctx), "EnsurePDBs"),
+		errors.Section(r.EnsureAnnotations(ctx, cachedStatus), "EnsureAnnotations"),
+		errors.Section(r.EnsureLabels(ctx, cachedStatus), "EnsureLabels"))
+}

pkg/deployment/resources/secrets.go

@@ -88,13 +88,13 @@ func (r *Resources) EnsureSecrets(ctx context.Context, cachedStatus inspectorInt
 	if spec.IsAuthenticated() {
 		counterMetric.Inc()
 		if err := reconcileRequired.WithError(r.ensureTokenSecret(ctx, cachedStatus, secrets, spec.Authentication.GetJWTSecretName())); err != nil {
-			return errors.WithStack(err)
+			return errors.Section(err, "JWT Secret")
 		}
 	}
 	if spec.IsSecure() {
 		counterMetric.Inc()
 		if err := reconcileRequired.WithError(r.ensureTLSCACertificateSecret(ctx, cachedStatus, secrets, spec.TLS)); err != nil {
-			return errors.WithStack(err)
+			return errors.Section(err, "TLS CA")
 		}
 	}
 
@@ -106,7 +106,7 @@ func (r *Resources) EnsureSecrets(ctx context.Context, cachedStatus inspectorInt
 		if imageFound {
 			if pod.VersionHasJWTSecretKeyfolder(image.ArangoDBVersion, image.Enterprise) {
 				if err := r.ensureTokenSecretFolder(ctx, cachedStatus, secrets, spec.Authentication.GetJWTSecretName(), pod.JWTSecretFolder(deploymentName)); err != nil {
-					return errors.WithStack(err)
+					return errors.Section(err, "JWT Folder")
 				}
 			}
 		}
@@ -114,18 +114,18 @@ func (r *Resources) EnsureSecrets(ctx context.Context, cachedStatus inspectorInt
 		if spec.Metrics.IsEnabled() {
 			if imageFound && pod.VersionHasJWTSecretKeyfolder(image.ArangoDBVersion, image.Enterprise) {
 				if err := reconcileRequired.WithError(r.ensureExporterTokenSecret(ctx, cachedStatus, secrets, spec.Metrics.GetJWTTokenSecretName(), pod.JWTSecretFolder(deploymentName))); err != nil {
-					return errors.WithStack(err)
+					return errors.Section(err, "Metrics JWT")
 				}
 			} else {
 				if err := reconcileRequired.WithError(r.ensureExporterTokenSecret(ctx, cachedStatus, secrets, spec.Metrics.GetJWTTokenSecretName(), spec.Authentication.GetJWTSecretName())); err != nil {
-					return errors.WithStack(err)
+					return errors.Section(err, "Metrics JWT")
 				}
 			}
 		}
 	}
 	if spec.IsSecure() {
 		if err := reconcileRequired.WithError(r.ensureSecretWithEmptyKey(ctx, cachedStatus, secrets, GetCASecretName(r.context.GetAPIObject()), "empty")); err != nil {
-			return errors.WithStack(err)
+			return errors.Section(err, "TLS TrustStore")
 		}
 
 		if err := reconcileRequired.ParallelAll(len(members), func(id int) error {
@@ -160,32 +160,32 @@ func (r *Resources) EnsureSecrets(ctx context.Context, cachedStatus inspectorInt
 			}
 			return nil
 		}); err != nil {
-			return errors.WithStack(err)
+			return errors.Section(err, "TLS TrustStore")
 		}
 	}
 	if spec.RocksDB.IsEncrypted() {
 		if i := status.CurrentImage; i != nil && features.EncryptionRotation().Supported(i.ArangoDBVersion, i.Enterprise) {
 			if err := reconcileRequired.WithError(r.ensureEncryptionKeyfolderSecret(ctx, cachedStatus, secrets, spec.RocksDB.Encryption.GetKeySecretName(), pod.GetEncryptionFolderSecretName(deploymentName))); err != nil {
-				return errors.WithStack(err)
+				return errors.Section(err, "Encryption")
 			}
 		}
 	}
 	if spec.Sync.IsEnabled() {
 		counterMetric.Inc()
 		if err := reconcileRequired.WithError(r.ensureTokenSecret(ctx, cachedStatus, secrets, spec.Sync.Authentication.GetJWTSecretName())); err != nil {
-			return errors.WithStack(err)
+			return errors.Section(err, "Sync Auth")
 		}
 		counterMetric.Inc()
 		if err := reconcileRequired.WithError(r.ensureTokenSecret(ctx, cachedStatus, secrets, spec.Sync.Monitoring.GetTokenSecretName())); err != nil {
-			return errors.WithStack(err)
+			return errors.Section(err, "Sync Monitoring Auth")
 		}
 		counterMetric.Inc()
 		if err := reconcileRequired.WithError(r.ensureTLSCACertificateSecret(ctx, cachedStatus, secrets, spec.Sync.TLS)); err != nil {
-			return errors.WithStack(err)
+			return errors.Section(err, "Sync TLS CA")
 		}
 		counterMetric.Inc()
 		if err := reconcileRequired.WithError(r.ensureClientAuthCACertificateSecret(ctx, cachedStatus, secrets, spec.Sync.Authentication)); err != nil {
-			return errors.WithStack(err)
+			return errors.Section(err, "Sync TLS Client CA")
 		}
 	}
 	return reconcileRequired.Reconcile(ctx)
@@ -321,7 +321,7 @@ func (r *Resources) createSecretWithMod(ctx context.Context, secrets secretv1.Mo
 
 	err := globals.GetGlobalTimeouts().Kubernetes().RunWithTimeout(ctx, func(ctxChild context.Context) error {
 		_, err := secrets.Create(ctxChild, secret, meta.CreateOptions{})
-		return err
+		return kerrors.NewResourceError(err, secret)
 	})
 	if err != nil {
 		// Failed to create secret
@@ -412,7 +412,7 @@ func AppendKeyfileToKeyfolder(ctx context.Context, cachedStatus inspectorInterfa
 		k8sutil.AddOwnerRefToObject(secret, ownerRef)
 		if _, err := secrets.Create(ctx, secret, meta.CreateOptions{}); err != nil {
 			// Failed to create secret
-			return errors.WithStack(err)
+			return kerrors.NewResourceError(err, secret)
 		}
 
 		return errors.Reconcile()

pkg/deployment/resources/servicemonitor.go

@@ -122,7 +122,11 @@ func (r *Resources) serviceMonitorSpec() (coreosv1.ServiceMonitorSpec, error) {
 }
 
 // EnsureServiceMonitor creates or updates a ServiceMonitor.
-func (r *Resources) EnsureServiceMonitor(ctx context.Context) error {
+func (r *Resources) EnsureServiceMonitor(ctx context.Context, enabled bool) error {
+	if !enabled {
+		return nil
+	}
+
 	// Some preparations:
 	log := r.log.Str("section", "service-monitor")
 	apiObject := r.context.GetAPIObject()

pkg/util/errors/section.go

@@ -0,0 +1,27 @@
+//
+// DISCLAIMER
+//
+// Copyright 2016-2022 ArangoDB GmbH, Cologne, Germany
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Copyright holder is ArangoDB GmbH, Cologne, Germany
+//
+
+package errors
+
+import "github.com/pkg/errors"
+
+func Section(cause error, format string, args ...interface{}) error {
+	return errors.Wrapf(cause, format, args...)
+}

pkg/util/k8sutil/secrets.go

@@ -31,6 +31,7 @@ import (
 	"github.com/arangodb/kube-arangodb/pkg/util/errors"
 	"github.com/arangodb/kube-arangodb/pkg/util/globals"
 	secretv1 "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector/secret/v1"
+	"github.com/arangodb/kube-arangodb/pkg/util/k8sutil/kerrors"
 )
 
 // ValidateEncryptionKeySecret checks that a secret with given name in given namespace
@@ -71,7 +72,7 @@ func CreateEncryptionKeySecret(secrets secretv1.ModInterface, secretName string,
 	}
 	if _, err := secrets.Create(context.Background(), secret, meta.CreateOptions{}); err != nil {
 		// Failed to create secret
-		return errors.WithStack(err)
+		return kerrors.NewResourceError(err, secret)
 	}
 	return nil
 }
@@ -165,7 +166,7 @@ func CreateCASecret(ctx context.Context, secrets secretv1.ModInterface, secretNa
 	AddOwnerRefToObject(secret, ownerRef)
 	if _, err := secrets.Create(ctx, secret, meta.CreateOptions{}); err != nil {
 		// Failed to create secret
-		return errors.WithStack(err)
+		return kerrors.NewResourceError(err, secret)
 	}
 	return nil
 }
@@ -207,7 +208,7 @@ func CreateTLSKeyfileSecret(ctx context.Context, secrets secretv1.ModInterface,
 	AddOwnerRefToObject(secret, ownerRef)
 	if _, err := secrets.Create(ctx, secret, meta.CreateOptions{}); err != nil {
 		// Failed to create secret
-		return errors.WithStack(err)
+		return kerrors.NewResourceError(err, secret)
 	}
 	return nil
 }
@@ -267,7 +268,7 @@ func CreateTokenSecret(ctx context.Context, secrets secretv1.ModInterface, secre
 	AddOwnerRefToObject(secret, ownerRef)
 	if _, err := secrets.Create(ctx, secret, meta.CreateOptions{}); err != nil {
 		// Failed to create secret
-		return errors.WithStack(err)
+		return kerrors.NewResourceError(err, secret)
 	}
 	return nil
 }
@@ -327,7 +328,7 @@ func CreateBasicAuthSecret(ctx context.Context, secrets secretv1.ModInterface, s
 	AddOwnerRefToObject(secret, ownerRef)
 	err := globals.GetGlobalTimeouts().Kubernetes().RunWithTimeout(ctx, func(ctxChild context.Context) error {
 		_, err := secrets.Create(ctxChild, secret, meta.CreateOptions{})
-		return err
+		return kerrors.NewResourceError(err, secret)
 	})
 	if err != nil {
 		// Failed to create secret