Updating EndpointSlice controllers to return if error encountered

Previously the controllers would proceed with additional creates,
updates, or deletes if 1 failed. That could potentially result in
scenarios where an EndpointSlice create or update failing while a delete
worked. This updates the logic so that removals will not happen if
additions fail.

pkg/controller/endpointslice/BUILD

@@ -26,7 +26,6 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
-        "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",

pkg/controller/endpointslice/reconciler.go

@@ -27,7 +27,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
-	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
 	clientset "k8s.io/client-go/kubernetes"
 	corelisters "k8s.io/client-go/listers/core/v1"
@@ -180,8 +179,6 @@ func (r *reconciler) finalize(
 	slicesToDelete []*discovery.EndpointSlice,
 	triggerTime time.Time,
 ) error {
-	errs := []error{}
-
 	// If there are slices to create and delete, change the creates to updates
 	// of the slices that would otherwise be deleted.
 	for i := 0; i < len(slicesToDelete); {
@@ -218,11 +215,10 @@ func (r *reconciler) finalize(
 				if errors.HasStatusCause(err, corev1.NamespaceTerminatingCause) {
 					return nil
 				}
-				errs = append(errs, fmt.Errorf("Error creating EndpointSlice for Service %s/%s: %v", service.Namespace, service.Name, err))
+				return fmt.Errorf("failed to create EndpointSlice for Service %s/%s: %v", service.Namespace, service.Name, err)
-			} else {
-				r.endpointSliceTracker.Update(createdSlice)
-				metrics.EndpointSliceChanges.WithLabelValues("create").Inc()
 			}
+			r.endpointSliceTracker.Update(createdSlice)
+			metrics.EndpointSliceChanges.WithLabelValues("create").Inc()
 		}
 	}
 
@@ -230,24 +226,22 @@ func (r *reconciler) finalize(
 		addTriggerTimeAnnotation(endpointSlice, triggerTime)
 		updatedSlice, err := r.client.DiscoveryV1beta1().EndpointSlices(service.Namespace).Update(context.TODO(), endpointSlice, metav1.UpdateOptions{})
 		if err != nil {
-			errs = append(errs, fmt.Errorf("Error updating %s EndpointSlice for Service %s/%s: %v", endpointSlice.Name, service.Namespace, service.Name, err))
+			return fmt.Errorf("failed to update %s EndpointSlice for Service %s/%s: %v", endpointSlice.Name, service.Namespace, service.Name, err)
-		} else {
-			r.endpointSliceTracker.Update(updatedSlice)
-			metrics.EndpointSliceChanges.WithLabelValues("update").Inc()
 		}
+		r.endpointSliceTracker.Update(updatedSlice)
+		metrics.EndpointSliceChanges.WithLabelValues("update").Inc()
 	}
 
 	for _, endpointSlice := range slicesToDelete {
 		err := r.client.DiscoveryV1beta1().EndpointSlices(service.Namespace).Delete(context.TODO(), endpointSlice.Name, metav1.DeleteOptions{})
 		if err != nil {
-			errs = append(errs, fmt.Errorf("Error deleting %s EndpointSlice for Service %s/%s: %v", endpointSlice.Name, service.Namespace, service.Name, err))
+			return fmt.Errorf("failed to delete %s EndpointSlice for Service %s/%s: %v", endpointSlice.Name, service.Namespace, service.Name, err)
-		} else {
-			r.endpointSliceTracker.Delete(endpointSlice)
-			metrics.EndpointSliceChanges.WithLabelValues("delete").Inc()
 		}
+		r.endpointSliceTracker.Delete(endpointSlice)
+		metrics.EndpointSliceChanges.WithLabelValues("delete").Inc()
 	}
 
-	return utilerrors.NewAggregate(errs)
+	return nil
 }
 
 // reconcileByPortMapping compares the endpoints found in existing slices with

pkg/controller/endpointslicemirroring/BUILD

@@ -26,7 +26,6 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
-        "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//staging/src/k8s.io/client-go/informers/core/v1:go_default_library",

pkg/controller/endpointslicemirroring/reconciler.go

@@ -25,7 +25,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
-	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/klog/v2"
@@ -231,7 +230,6 @@ func (r *reconciler) finalize(endpoints *corev1.Endpoints, slices slicesByAction
 	// be deleted.
 	recycleSlices(&slices)
 
-	var errs []error
 	epsClient := r.client.DiscoveryV1beta1().EndpointSlices(endpoints.Namespace)
 
 	// Don't create more EndpointSlices if corresponding Endpoints resource is
@@ -244,35 +242,32 @@ func (r *reconciler) finalize(endpoints *corev1.Endpoints, slices slicesByAction
 				if errors.HasStatusCause(err, corev1.NamespaceTerminatingCause) {
 					return nil
 				}
-				errs = append(errs, fmt.Errorf("Error creating EndpointSlice for Endpoints %s/%s: %v", endpoints.Namespace, endpoints.Name, err))
+				return fmt.Errorf("failed to create EndpointSlice for Endpoints %s/%s: %v", endpoints.Namespace, endpoints.Name, err)
-			} else {
-				r.endpointSliceTracker.Update(createdSlice)
-				metrics.EndpointSliceChanges.WithLabelValues("create").Inc()
 			}
+			r.endpointSliceTracker.Update(createdSlice)
+			metrics.EndpointSliceChanges.WithLabelValues("create").Inc()
 		}
 	}
 
 	for _, endpointSlice := range slices.toUpdate {
 		updatedSlice, err := epsClient.Update(context.TODO(), endpointSlice, metav1.UpdateOptions{})
 		if err != nil {
-			errs = append(errs, fmt.Errorf("Error updating %s EndpointSlice for Endpoints %s/%s: %v", endpointSlice.Name, endpoints.Namespace, endpoints.Name, err))
+			return fmt.Errorf("failed to update %s EndpointSlice for Endpoints %s/%s: %v", endpointSlice.Name, endpoints.Namespace, endpoints.Name, err)
-		} else {
-			r.endpointSliceTracker.Update(updatedSlice)
-			metrics.EndpointSliceChanges.WithLabelValues("update").Inc()
 		}
+		r.endpointSliceTracker.Update(updatedSlice)
+		metrics.EndpointSliceChanges.WithLabelValues("update").Inc()
 	}
 
 	for _, endpointSlice := range slices.toDelete {
 		err := epsClient.Delete(context.TODO(), endpointSlice.Name, metav1.DeleteOptions{})
 		if err != nil {
-			errs = append(errs, fmt.Errorf("Error deleting %s EndpointSlice for Endpoints %s/%s: %v", endpointSlice.Name, endpoints.Namespace, endpoints.Name, err))
+			return fmt.Errorf("failed to delete %s EndpointSlice for Endpoints %s/%s: %v", endpointSlice.Name, endpoints.Namespace, endpoints.Name, err)
-		} else {
-			r.endpointSliceTracker.Delete(endpointSlice)
-			metrics.EndpointSliceChanges.WithLabelValues("delete").Inc()
 		}
+		r.endpointSliceTracker.Delete(endpointSlice)
+		metrics.EndpointSliceChanges.WithLabelValues("delete").Inc()
 	}
 
-	return utilerrors.NewAggregate(errs)
+	return nil
 }
 
 // deleteEndpoints deletes any associated EndpointSlices and cleans up any