Trigger full snapshot when spinning up a compaction job is unviable (#1130)

* trigger full snapshot when spinning up a compaction job is unviable

* Add unit tests for mocking httpClient

* modify the CompactionSpec object name to SnapshotCompactionSpec to be more clear about the type of compaction

* etcd status changes & integration test adaption

* poll k8s resources to make sure of the state before proceeding with next reconciliations

* initialize logger with etcdName & etcdNamespace rather than passing them everytime

* get etcd full snapshot timeout from backup spec

Author: anveshreddy18

api/config/v1alpha1/defaults.go

@@ -129,6 +129,8 @@ const (
 	DefaultCompactionConcurrentSyncs = 3
 	// DefaultCompactionEventsThreshold is the default number of events that triggers compaction.
 	DefaultCompactionEventsThreshold = 1000000
+	// DefaultCompactionTriggerFullSnapshotThreshold is the default upper threshold for the number of etcd events before giving up on compaction job and triggering a full snapshot.
+	DefaultCompactionTriggerFullSnapshotThreshold = 3000000
 	// DefaultCompactionActiveDeadlineDuration is the default active deadline duration for compaction.
 	DefaultCompactionActiveDeadlineDuration = 3 * time.Hour
 )
@@ -144,6 +146,9 @@ func SetDefaults_CompactionControllerConfiguration(compactionCtrlConfig *Compact
 	if compactionCtrlConfig.EventsThreshold == 0 {
 		compactionCtrlConfig.EventsThreshold = DefaultCompactionEventsThreshold
 	}
+	if compactionCtrlConfig.TriggerFullSnapshotThreshold == 0 {
+		compactionCtrlConfig.TriggerFullSnapshotThreshold = DefaultCompactionTriggerFullSnapshotThreshold
+	}
 	if compactionCtrlConfig.ActiveDeadlineDuration == zeroDuration {
 		compactionCtrlConfig.ActiveDeadlineDuration = metav1.Duration{Duration: DefaultCompactionActiveDeadlineDuration}
 	}

api/config/v1alpha1/defaults_test.go

@@ -267,24 +267,27 @@ func TestSetDefaults_CompactionControllerConfiguration(t *testing.T) {
 				Enabled: true,
 			},
 			expected: &CompactionControllerConfiguration{
-				Enabled:                true,
-				ConcurrentSyncs:        ptr.To(3),
-				EventsThreshold:        1000000,
-				ActiveDeadlineDuration: metav1.Duration{Duration: 3 * time.Hour},
+				Enabled:                      true,
+				ConcurrentSyncs:              ptr.To(3),
+				EventsThreshold:              1000000,
+				TriggerFullSnapshotThreshold: 3000000,
+				ActiveDeadlineDuration:       metav1.Duration{Duration: 3 * time.Hour},
 			},
 		},
 		{
 			name: "should not overwrite already set values",
 			config: &CompactionControllerConfiguration{
-				Enabled:                true,
-				ConcurrentSyncs:        ptr.To(5),
-				ActiveDeadlineDuration: metav1.Duration{Duration: 1 * time.Hour},
+				Enabled:                      true,
+				ConcurrentSyncs:              ptr.To(5),
+				TriggerFullSnapshotThreshold: 2000000,
+				ActiveDeadlineDuration:       metav1.Duration{Duration: 1 * time.Hour},
 			},
 			expected: &CompactionControllerConfiguration{
-				Enabled:                true,
-				ConcurrentSyncs:        ptr.To(5),
-				EventsThreshold:        1000000,
-				ActiveDeadlineDuration: metav1.Duration{Duration: 1 * time.Hour},
+				Enabled:                      true,
+				ConcurrentSyncs:              ptr.To(5),
+				EventsThreshold:              1000000,
+				TriggerFullSnapshotThreshold: 2000000,
+				ActiveDeadlineDuration:       metav1.Duration{Duration: 1 * time.Hour},
 			},
 		},
 	}

api/config/v1alpha1/types.go

@@ -166,6 +166,8 @@ type CompactionControllerConfiguration struct {
 	ConcurrentSyncs *int `json:"concurrentSyncs,omitempty"`
 	// EventsThreshold denotes total number of etcd events to be reached upon which a backup compaction job is triggered.
 	EventsThreshold int64 `json:"eventsThreshold"`
+	// TriggerFullSnapshotThreshold denotes the upper threshold for the number of etcd events before giving up on compaction job and triggering a full snapshot.
+	TriggerFullSnapshotThreshold int64 `json:"triggerFullSnapshotThreshold"`
 	// ActiveDeadlineDuration is the duration after which a running compaction job will be killed.
 	ActiveDeadlineDuration metav1.Duration `json:"activeDeadlineDuration"`
 	// MetricsScrapeWaitDuration is the duration to wait for after compaction job is completed, to allow Prometheus metrics to be scraped

api/config/v1alpha1/validation/validation.go

@@ -97,6 +97,9 @@ func validateCompactionControllerConfiguration(compactionControllerConfig druidc
 	if compactionControllerConfig.EventsThreshold <= 0 {
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("eventsThreshold"), compactionControllerConfig.EventsThreshold, "must be greater than 0"))
 	}
+	if compactionControllerConfig.TriggerFullSnapshotThreshold <= 0 {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("triggerFullSnapshotThreshold"), compactionControllerConfig.TriggerFullSnapshotThreshold, "must be greater than 0"))
+	}
 	return allErrs
 }
 

api/config/v1alpha1/validation/validation_test.go

@@ -313,14 +313,15 @@ func TestValidateEtcdControllerConfiguration(t *testing.T) {
 
 func TestValidateCompactionControllerConfiguration(t *testing.T) {
 	tests := []struct {
-		name                      string
-		enabled                   bool
-		concurrentSync            *int
-		eventThreshold            *int64
-		activeDeadlineDuration    *metav1.Duration
-		metricsScrapeWaitDuration *metav1.Duration
-		expectedErrors            int
-		matcher                   gomegatypes.GomegaMatcher
+		name                         string
+		enabled                      bool
+		concurrentSync               *int
+		eventsThreshold              *int64
+		triggerFullSnapshotThreshold *int64
+		activeDeadlineDuration       *metav1.Duration
+		metricsScrapeWaitDuration    *metav1.Duration
+		expectedErrors               int
+		matcher                      gomegatypes.GomegaMatcher
 	}{
 		{
 			name:           "should allow default compaction controller configuration when it is enabled",
@@ -355,12 +356,32 @@ func TestValidateCompactionControllerConfiguration(t *testing.T) {
 			matcher:        ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{"Type": Equal(field.ErrorTypeInvalid), "Field": Equal("controllers.compaction.concurrentSyncs")}))),
 		},
 		{
-			name:           "should forbid event threshold less than zero",
-			enabled:        true,
-			eventThreshold: ptr.To(int64(-1)),
-			expectedErrors: 1,
-
-			matcher: ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{"Type": Equal(field.ErrorTypeInvalid), "Field": Equal("controllers.compaction.eventsThreshold")}))),
+			name:            "should forbid events threshold equal to zero",
+			enabled:         true,
+			eventsThreshold: ptr.To(int64(0)),
+			expectedErrors:  1,
+			matcher:         ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{"Type": Equal(field.ErrorTypeInvalid), "Field": Equal("controllers.compaction.eventsThreshold")}))),
+		},
+		{
+			name:            "should forbid events threshold less than zero",
+			enabled:         true,
+			eventsThreshold: ptr.To(int64(-1)),
+			expectedErrors:  1,
+			matcher:         ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{"Type": Equal(field.ErrorTypeInvalid), "Field": Equal("controllers.compaction.eventsThreshold")}))),
+		},
+		{
+			name:                         "should forbid trigger full snapshot threshold equal to zero",
+			enabled:                      true,
+			triggerFullSnapshotThreshold: ptr.To(int64(0)),
+			expectedErrors:               1,
+			matcher:                      ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{"Type": Equal(field.ErrorTypeInvalid), "Field": Equal("controllers.compaction.triggerFullSnapshotThreshold")}))),
+		},
+		{
+			name:                         "should forbid trigger full snapshot threshold less than zero",
+			enabled:                      true,
+			triggerFullSnapshotThreshold: ptr.To(int64(-1)),
+			expectedErrors:               1,
+			matcher:                      ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{"Type": Equal(field.ErrorTypeInvalid), "Field": Equal("controllers.compaction.triggerFullSnapshotThreshold")}))),
 		},
 		{
 			name:                   "should forbid active deadline duration less than zero",
@@ -390,8 +411,11 @@ func TestValidateCompactionControllerConfiguration(t *testing.T) {
 			if test.concurrentSync != nil {
 				controllerConfig.ConcurrentSyncs = test.concurrentSync
 			}
-			if test.eventThreshold != nil {
-				controllerConfig.EventsThreshold = *test.eventThreshold
+			if test.eventsThreshold != nil {
+				controllerConfig.EventsThreshold = *test.eventsThreshold
+			}
+			if test.triggerFullSnapshotThreshold != nil {
+				controllerConfig.TriggerFullSnapshotThreshold = *test.triggerFullSnapshotThreshold
 			}
 			if test.activeDeadlineDuration != nil {
 				controllerConfig.ActiveDeadlineDuration = *test.activeDeadlineDuration

api/core/v1alpha1/constants.go

@@ -4,6 +4,11 @@
 
 package v1alpha1
 
+import (
+	batchv1 "k8s.io/api/batch/v1"
+	v1 "k8s.io/api/core/v1"
+)
+
 // Common label keys to be placed on all druid-managed resources
 const (
 	// LabelAppNameKey is a label which sets the name of the resource provisioned for an etcd cluster.
@@ -38,3 +43,32 @@ const (
 	// runtime components of the etcd cluster such as pods, PVCs, leases, RBAC resources, PDBs, services, etc.
 	DisableEtcdRuntimeComponentCreationAnnotation = "druid.gardener.cloud/disable-etcd-runtime-component-creation"
 )
+
+// Compaction Job/Pod reasons that are used to set the reason for a pod condition in the status of an Etcd resource.
+const (
+	// PodFailureReasonPreemptionByScheduler is a reason for a pod failure that indicates that the pod was preempted by the scheduler.
+	PodFailureReasonPreemptionByScheduler = v1.PodReasonPreemptionByScheduler
+	// PodFailureReasonDeletionByTaintManager is a reason for a pod failure that indicates that the pod was deleted by the taint manager.
+	PodFailureReasonDeletionByTaintManager = "DeletionByTaintManager"
+	// PodFailureReasonEvictionByEvictionAPI is a reason for a pod failure that indicates that the pod was evicted by the eviction API.
+	PodFailureReasonEvictionByEvictionAPI = "EvictionByEvictionAPI"
+	// PodFailureReasonTerminationByKubelet is a reason for a pod failure that indicates that the pod was terminated by the kubelet.
+	PodFailureReasonTerminationByKubelet = v1.PodReasonTerminationByKubelet
+	// PodFailureReasonProcessFailure is a reason for a pod failure that indicates that the pod process failed.
+	PodFailureReasonProcessFailure = "ProcessFailure"
+	// PodFailureReasonUnknown is a reason for a pod failure that indicates that the reason for the pod failure is unknown.
+	PodFailureReasonUnknown = "Unknown"
+
+	// PodSuccessReasonNone is a reason for a pod success that indicates that the pod has not failed.
+	PodSuccessReasonNone = "None"
+
+	// JobFailureReasonDeadlineExceeded is a reason for a job failure that indicates that the job has exceeded its deadline.
+	JobFailureReasonDeadlineExceeded = batchv1.JobReasonDeadlineExceeded
+	// JobFailureReasonBackoffLimitExceeded is a reason for a job failure that indicates that the job has exceeded its backoff limit.
+	JobFailureReasonBackoffLimitExceeded = batchv1.JobReasonBackoffLimitExceeded
+
+	// FullSnapshotSuccessReason is the reason for a successful full snapshot.
+	FullSnapshotSuccessReason string = "FullSnapshotTakenSuccessfully"
+	// FullSnapshotFailureReason is the reason for a failed full snapshot.
+	FullSnapshotFailureReason string = "ErrorTriggeringFullSnapshot"
+)

api/core/v1alpha1/crds/druid.gardener.cloud_etcds.yaml

@@ -75,67 +75,6 @@ spec:
                 description: BackupSpec defines parameters associated with the full
                   and delta snapshots of etcd.
                 properties:
-                  compactionResources:
-                    description: |-
-                      CompactionResources defines compute Resources required by compaction job.
-                      More info: https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/
-                    properties:
-                      claims:
-                        description: |-
-                          Claims lists the names of resources, defined in spec.resourceClaims,
-                          that are used by this container.
-
-                          This is an alpha field and requires enabling the
-                          DynamicResourceAllocation feature gate.
-
-                          This field is immutable. It can only be set for containers.
-                        items:
-                          description: ResourceClaim references one entry in PodSpec.ResourceClaims.
-                          properties:
-                            name:
-                              description: |-
-                                Name must match the name of one entry in pod.spec.resourceClaims of
-                                the Pod where this field is used. It makes that resource available
-                                inside a container.
-                              type: string
-                            request:
-                              description: |-
-                                Request is the name chosen for a request in the referenced claim.
-                                If empty, everything from the claim is made available, otherwise
-                                only the result of this request.
-                              type: string
-                          required:
-                          - name
-                          type: object
-                        type: array
-                        x-kubernetes-list-map-keys:
-                        - name
-                        x-kubernetes-list-type: map
-                      limits:
-                        additionalProperties:
-                          anyOf:
-                          - type: integer
-                          - type: string
-                          pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
-                          x-kubernetes-int-or-string: true
-                        description: |-
-                          Limits describes the maximum amount of compute resources allowed.
-                          More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
-                        type: object
-                      requests:
-                        additionalProperties:
-                          anyOf:
-                          - type: integer
-                          - type: string
-                          pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
-                          x-kubernetes-int-or-string: true
-                        description: |-
-                          Requests describes the minimum amount of compute resources required.
-                          If Requests is omitted for a container, it defaults to Limits if that is explicitly specified,
-                          otherwise to an implementation-defined value. Requests cannot exceed Limits.
-                          More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
-                        type: object
-                    type: object
                   compression:
                     description: SnapshotCompression defines the specification for
                       compression of Snapshots.
@@ -286,6 +225,83 @@ spec:
                           More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
                         type: object
                     type: object
+                  snapshotCompaction:
+                    description: SnapshotCompaction defines the specification for
+                      compaction of backups.
+                    properties:
+                      eventsThreshold:
+                        description: EventsThreshold defines the threshold for the
+                          number of etcd events before triggering a compaction job
+                        format: int64
+                        type: integer
+                      resources:
+                        description: |-
+                          Resources defines compute Resources required by compaction job.
+                          More info: https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/
+                        properties:
+                          claims:
+                            description: |-
+                              Claims lists the names of resources, defined in spec.resourceClaims,
+                              that are used by this container.
+
+                              This is an alpha field and requires enabling the
+                              DynamicResourceAllocation feature gate.
+
+                              This field is immutable. It can only be set for containers.
+                            items:
+                              description: ResourceClaim references one entry in PodSpec.ResourceClaims.
+                              properties:
+                                name:
+                                  description: |-
+                                    Name must match the name of one entry in pod.spec.resourceClaims of
+                                    the Pod where this field is used. It makes that resource available
+                                    inside a container.
+                                  type: string
+                                request:
+                                  description: |-
+                                    Request is the name chosen for a request in the referenced claim.
+                                    If empty, everything from the claim is made available, otherwise
+                                    only the result of this request.
+                                  type: string
+                              required:
+                              - name
+                              type: object
+                            type: array
+                            x-kubernetes-list-map-keys:
+                            - name
+                            x-kubernetes-list-type: map
+                          limits:
+                            additionalProperties:
+                              anyOf:
+                              - type: integer
+                              - type: string
+                              pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                              x-kubernetes-int-or-string: true
+                            description: |-
+                              Limits describes the maximum amount of compute resources allowed.
+                              More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
+                            type: object
+                          requests:
+                            additionalProperties:
+                              anyOf:
+                              - type: integer
+                              - type: string
+                              pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                              x-kubernetes-int-or-string: true
+                            description: |-
+                              Requests describes the minimum amount of compute resources required.
+                              If Requests is omitted for a container, it defaults to Limits if that is explicitly specified,
+                              otherwise to an implementation-defined value. Requests cannot exceed Limits.
+                              More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
+                            type: object
+                        type: object
+                      triggerFullSnapshotThreshold:
+                        description: TriggerFullSnapshotThreshold defines the upper
+                          threshold for the number of etcd events before giving up
+                          on compaction job and triggering a full snapshot.
+                        format: int64
+                        type: integer
+                    type: object
                   store:
                     description: Store defines the specification of object store provider
                       for storing backups.