fix: delegate resources Condition update to the handler

Author: fra98

README.md

@@ -6,7 +6,14 @@ A Kubernetes controller for managing ResourceSlice resources with customizable b
 
 The Resource Slice Classes controller is designed to manage ResourceSlice resources in a Kubernetes cluster. Each controller instance handles ResourceSlices of a specific class, allowing you to run multiple controllers with different behaviors for different classes.
 
-The controller manages the ResourceSlice status updates and conditions, while the handler is responsible for implementing the resource allocation strategy.
+The controller manages the ResourceSlice status updates, while the handler is responsible for implementing the resource allocation strategy.
+In particular the handler should:
+
+- set the list of resources allocated for that resourceslice in the status
+- set a Condition of type `Resources` to indicate if the resources have been accepted or denied.
+- return an error if it fails to process the ResourceSlice, thus the reconciliation should be retried.
+Note: it should not return an error if the resourceslice has been correctly processed,
+but the resources have been denied.
 
 ## Features
 
@@ -76,6 +83,7 @@ To implement a custom handler for your ResourceSlice class:
     func (h *MyHandler) Handle(ctx context.Context, resourceSlice *authv1beta1.ResourceSlice) (ctrl.Result, error) {
         // Implement your custom resource allocation logic here
         // Update resourceSlice.Status.Resources with your allocated resources
+        // and set the status Condition of type "Resources" accordingly
 
         return ctrl.Result{}, nil
     }
@@ -122,12 +130,12 @@ Your handler implementation should:
 
 1. Implement your resource allocation strategy
 2. Set the allocated resources in `resourceSlice.Status.Resources`
-3. Return appropriate reconciliation results and errors
+3. Set the `Resources` Condition to accept or deny the resources requested
+4. Return appropriate reconciliation results and errors
 
 Note: The controller, not the handler, is responsible for:
 
 - Updating the ResourceSlice status in the API server
-- Managing ResourceSlice conditions
 - Recording events
 - Error handling and logging
 
@@ -140,6 +148,7 @@ Note: The controller, not the handler, is responsible for:
 
 2. **Handler Implementation**:
    - Keep handlers focused on resource calculation logic
+   - Explicitly accept or deny the resources through the appropriate Condition
    - Return meaningful errors for proper event recording
    - Use logging for debugging purposes
 

examples/cappedresources/handler.go

@@ -4,9 +4,11 @@ import (
 	"context"
 
 	authv1beta1 "github.com/liqotech/liqo/apis/authentication/v1beta1"
+	"github.com/liqotech/liqo/pkg/liqo-controller-manager/authentication"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	rshandler "github.com/liqotech/resource-slice-class-controller-template/pkg/resourceslice/handler"
 )
@@ -27,15 +29,25 @@ func NewHandler(maxResources corev1.ResourceList) rshandler.Handler {
 func (h *Handler) Handle(_ context.Context, resourceSlice *authv1beta1.ResourceSlice) (ctrl.Result, error) {
 	// Generate and update resources in status
 	resources := h.getCappedResources(resourceSlice.Spec.Resources)
-
 	resourceSlice.Status.Resources = resources
 
-	klog.InfoS("Updated ResourceSlice status",
+	klog.InfoS("Processed ResourceSlice resources",
 		"name", resourceSlice.Name,
 		"namespace", resourceSlice.Namespace,
 		"cpu", resources.Cpu().String(),
 		"memory", resources.Memory().String(),
-		"pods", resources.Pods().String())
+		"pods", resources.Pods().String(),
+		"ephemeral-storage", resources.StorageEphemeral().String())
+
+	// Ensure the "Resources" condition is set.
+	authentication.EnsureCondition(
+		resourceSlice,
+		authv1beta1.ResourceSliceConditionTypeResources,
+		authv1beta1.ResourceSliceConditionAccepted,
+		"ResourceSliceResourcesAccepted",
+		"ResourceSlice resources accepted",
+	)
+	klog.Infof("ResourceSlice %q resources condition accepted", client.ObjectKeyFromObject(resourceSlice))
 
 	return ctrl.Result{}, nil
 }

pkg/controller/resource_slice_controller.go

@@ -84,7 +84,13 @@ func (r *ResourceSliceReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	res, err = r.handler.Handle(ctx, &resourceSlice)
 	if err != nil {
 		r.recorder.Eventf(&resourceSlice, "Warning", "Failed", "Failed to handle ResourceSlice: %v", err)
-		return ctrl.Result{}, err
+		return ctrl.Result{}, fmt.Errorf("failed to handle ResourceSlice %q: %w", req.NamespacedName, err)
+	}
+
+	// check the "Resources" condition is set.
+	resCond := authentication.GetCondition(&resourceSlice, authv1beta1.ResourceSliceConditionTypeResources)
+	if resCond == nil {
+		return ctrl.Result{}, fmt.Errorf("failed to handle ResourceSlice %q: missing \"Resources\" condition", req.NamespacedName)
 	}
 
 	defer func() {
@@ -94,22 +100,12 @@ func (r *ResourceSliceReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 				klog.Error(err)
 			}
 			r.recorder.Eventf(&resourceSlice, "Warning", "Failed", "Failed to update ResourceSlice status: %v", err)
-			err = fmt.Errorf("failed to update ResourceSlice status: %w", newErr)
+			err = fmt.Errorf("failed to update ResourceSlice %q status: %w", req.NamespacedName, newErr)
+			return
 		}
+		klog.Infof("ResourceSlice %q status correctly updated", req.NamespacedName)
 	}()
 
-	// Update the conditions
-	if resourceSlice.Status.Conditions == nil {
-		resourceSlice.Status.Conditions = []authv1beta1.ResourceSliceCondition{}
-	}
-	authentication.EnsureCondition(
-		&resourceSlice,
-		authv1beta1.ResourceSliceConditionTypeResources,
-		authv1beta1.ResourceSliceConditionAccepted,
-		"ResourceSliceResourcesAccepted",
-		"ResourceSlice resources accepted",
-	)
-
 	// Return the reconciliation result
 	return res, nil
 }

pkg/resourceslice/handler/interface.go

@@ -10,6 +10,14 @@ import (
 
 // Handler defines the interface for handling ResourceSlice operations.
 type Handler interface {
-	// Handle processes a ResourceSlice and returns a reconciliation result
+	// Handle processes a ResourceSlice and returns a reconciliation result.
+	// The handler should also update the ResourceSlice status. In particular, it should:
+	// - set the list of resources that have been allocated in `status.resources`
+	// - set a Condition of type "Resources"  in `status.conditions` to indicate
+	//   if the resources have been accepted or denied.
+	// An error should be returned if the handler fails to process the ResourceSlice
+	// and the reconciliation should be retried.
+	// Note: it should not return an error if the resourceslice has been correctly processed,
+	// but the resources have been denied.
 	Handle(ctx context.Context, resourceSlice *authv1beta1.ResourceSlice) (ctrl.Result, error)
 }