Terminal reconciliation errors

Azure · Dec 29, 2023 · d60dea8 · d60dea8
1 parent 49beafa
commit d60dea8
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 6 deletions.
diff --git a/internal/controllers/reconciliation/controller.go b/internal/controllers/reconciliation/controller.go
@@ -16,6 +16,7 @@ import (
 	"k8s.io/client-go/rest"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	apiv1 "github.com/Azure/eno/api/v1"
 	"github.com/Azure/eno/internal/reconstitution"
@@ -25,8 +26,6 @@ import (
 // TODO: Block ResourceSlice deletion until resources have been cleaned up
 // TODO: Clean up unused resource slices older than a duration
 
-// TODO: Handle 400s better
-
 type Controller struct {
 	client         client.Client
 	resourceClient reconstitution.Client
@@ -168,7 +167,7 @@ func (c *Controller) buildPatch(ctx context.Context, prev, resource *reconstitut
 
 	currentJS, err := current.MarshalJSON()
 	if err != nil {
-		return nil, "", fmt.Errorf("building json representation of desired state: %w", err)
+		return nil, "", reconcile.TerminalError(fmt.Errorf("building json representation of desired state: %w", err))
 	}
 
 	model, err := c.discovery.Get(ctx, resource.Object.GroupVersionKind())
@@ -177,24 +176,30 @@ func (c *Controller) buildPatch(ctx context.Context, prev, resource *reconstitut
 	}
 	if model == nil {
 		patch, err := jsonmergepatch.CreateThreeWayJSONMergePatch(prevManifest, []byte(resource.Manifest), currentJS)
+		if err != nil {
+			return nil, "", reconcile.TerminalError(err)
+		}
 		return patch, types.MergePatchType, err
 	}
 
 	patchmeta := strategicpatch.NewPatchMetaFromOpenAPI(model)
 	patch, err := strategicpatch.CreateThreeWayMergePatch(prevManifest, []byte(resource.Manifest), currentJS, patchmeta, true)
+	if err != nil {
+		return nil, "", reconcile.TerminalError(err)
+	}
 	return patch, types.StrategicMergePatchType, err
 }
 
 func mungePatch(patch []byte, rv string) ([]byte, error) {
 	var patchMap map[string]interface{}
 	err := json.Unmarshal(patch, &patchMap)
 	if err != nil {
-		return nil, err
+		return nil, reconcile.TerminalError(err)
 	}
 	u := unstructured.Unstructured{Object: patchMap}
 	a, err := meta.Accessor(&u)
 	if err != nil {
-		return nil, err
+		return nil, reconcile.TerminalError(err)
 	}
 	a.SetResourceVersion(rv)
 	a.SetCreationTimestamp(metav1.Time{})

diff --git a/internal/controllers/synthesis/rollout.go b/internal/controllers/synthesis/rollout.go
@@ -60,7 +60,6 @@ func (c *rolloutController) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	}
 
 	// randomize list to avoid always rolling out changes in the same order
-	// TODO: Consider a more efficient approach here
 	rand.Shuffle(len(compList.Items), func(i, j int) { compList.Items[i] = compList.Items[j] })
 
 	for _, comp := range compList.Items {