Record an event when provisioning request fails (kubernetes-sigs#3056)

* Record an event when provisioning request fails * Update event reason * Expose ProvReq error as a status * Include error details into status message * Use interceptor funcs for testing * Avoid code duplication for error message
KunWuLuan · Oct 9, 2024 · 355c447 · 355c447
1 parent f6f80a5
commit 355c447
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 2 deletions.
diff --git a/pkg/controller/admissionchecks/provisioning/controller.go b/pkg/controller/admissionchecks/provisioning/controller.go
@@ -237,7 +237,8 @@ func (c *Controller) syncOwnedProvisionRequest(ctx context.Context, wl *kueue.Wo
 		if !c.reqIsNeeded(wl, prc) {
 			continue
 		}
-		if ac := workload.FindAdmissionCheck(wl.Status.AdmissionChecks, checkName); ac != nil && ac.State == kueue.CheckStateReady {
+		ac := workload.FindAdmissionCheck(wl.Status.AdmissionChecks, checkName)
+		if ac != nil && ac.State == kueue.CheckStateReady {
 			log.V(2).Info("Skip syncing of the ProvReq for admission check which is Ready", "workload", klog.KObj(wl), "admissionCheck", checkName)
 			continue
 		}
@@ -302,6 +303,10 @@ func (c *Controller) syncOwnedProvisionRequest(ctx context.Context, wl *kueue.Wo
 			}
 
 			if err := c.client.Create(ctx, req); err != nil {
+				ac.Message = fmt.Sprintf("Error creating ProvisioningRequest %q: %v", requestName, err)
+				workload.SetAdmissionCheckState(&wl.Status.AdmissionChecks, *ac)
+
+				c.record.Eventf(wl, corev1.EventTypeWarning, "FailedCreate", ac.Message)
 				return nil, err
 			}
 			c.record.Eventf(wl, corev1.EventTypeNormal, "ProvisioningRequestCreated", "Created ProvisioningRequest: %q", req.Name)

diff --git a/pkg/controller/admissionchecks/provisioning/controller_test.go b/pkg/controller/admissionchecks/provisioning/controller_test.go
@@ -17,6 +17,8 @@ limitations under the License.
 package provisioning
 
 import (
+	"context"
+	"errors"
 	"testing"
 	"time"
 
@@ -40,6 +42,8 @@ import (
 	"sigs.k8s.io/kueue/pkg/workload"
 )
 
+var errInvalidProvisioningRequest = errors.New("invalid ProvisioningRequest error")
+
 var (
 	wlCmpOptions = []cmp.Option{
 		cmpopts.EquateEmpty(),
@@ -1171,13 +1175,44 @@ func TestReconcile(t *testing.T) {
 					Obj(),
 			},
 		},
+		"when invalid provisioning request": {
+			workload: utiltesting.MakeWorkload("wl", TestNamespace).
+				Annotations(map[string]string{
+					"provreq.kueue.x-k8s.io/ValidUntilSeconds": "0",
+					"invalid-provreq-prefix/Foo1":              "Bar1",
+					"another-invalid-provreq-prefix/Foo2":      "Bar2"}).
+				AdmissionChecks(kueue.AdmissionCheckState{
+					Name:  "check1",
+					State: kueue.CheckStatePending}).
+				ReserveQuota(utiltesting.MakeAdmission("q1").Obj()).
+				Obj(),
+			checks:             []kueue.AdmissionCheck{*baseCheck.DeepCopy()},
+			configs:            []kueue.ProvisioningRequestConfig{{ObjectMeta: metav1.ObjectMeta{Name: "config1"}}},
+			wantReconcileError: errInvalidProvisioningRequest,
+			wantEvents: []utiltesting.EventRecord{
+				{
+					Key:       client.ObjectKeyFromObject(baseWorkload),
+					EventType: corev1.EventTypeWarning,
+					Reason:    "FailedCreate",
+					Message:   `Error creating ProvisioningRequest "wl-check1-1": invalid ProvisioningRequest error`,
+				},
+			},
+		},
 	}
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
 			builder, ctx := getClientBuilder()
 			builder = builder.WithInterceptorFuncs(interceptor.Funcs{SubResourcePatch: utiltesting.TreatSSAAsStrategicMerge})
 
+			if tc.wantReconcileError != nil {
+				builder = builder.WithInterceptorFuncs(
+					interceptor.Funcs{
+						Create: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
+							return tc.wantReconcileError
+						}})
+			}
+
 			builder = builder.WithObjects(tc.workload)
 			builder = builder.WithStatusSubresource(tc.workload)
 
@@ -1207,7 +1242,7 @@ func TestReconcile(t *testing.T) {
 				},
 			}
 			_, gotReconcileError := controller.Reconcile(ctx, req)
-			if diff := cmp.Diff(tc.wantReconcileError, gotReconcileError); diff != "" {
+			if diff := cmp.Diff(tc.wantReconcileError, gotReconcileError, cmpopts.EquateErrors()); diff != "" {
 				t.Errorf("unexpected reconcile error (-want/+got):\n%s", diff)
 			}