fix: operator retry bug fixes

src/istio/common/manifests/pepr-istio-config.yaml

@@ -1,6 +1,7 @@
 # Have to pre-create the namespace and also patch it with the istio-injection label later because
 # Helm is kind of dumb: https://github.com/helm/helm/issues/350
 kind: Namespace
+apiVersion: v1
 metadata:
   name: pepr-system
   labels:

src/pepr/operator/controllers/keycloak/client-sync.ts

@@ -134,7 +134,7 @@ async function syncClient(
 
     if (isRetry) {
       Log.error(`${msg}, retry failed, aborting`);
-      throw new Error(`${msg}. RETRY FAILED, aborting: ${JSON.stringify(err)}`);
+      throw new Error(`${msg}. RETRY FAILED, aborting: ${err.message}`);
     }
 
     // Retry the request

src/pepr/operator/crd/generated/package-v1alpha1.ts

@@ -536,7 +536,7 @@ export enum ClientAuthenticatorType {
  */
 export interface Groups {
   /**
-   * List of group allowed to access to client
+   * List of groups allowed to access to client
    */
   anyOf?: string[];
 }
@@ -564,6 +564,7 @@ export enum Phase {
   Failed = "Failed",
   Pending = "Pending",
   Ready = "Ready",
+  Retrying = "Retrying",
 }
 
 RegisterKind(Package, {

src/pepr/operator/crd/sources/package/v1alpha1.ts

@@ -385,7 +385,7 @@ export const v1alpha1: V1CustomResourceDefinitionVersion = {
               type: "integer",
             },
             phase: {
-              enum: ["Pending", "Ready", "Failed"],
+              enum: ["Pending", "Ready", "Failed", "Retrying"],
               type: "string",
             },
             ssoClients: {

src/pepr/operator/reconcilers/index.spec.ts

@@ -180,6 +180,7 @@ describe("handleFailure", () => {
     expect(PatchStatus).toHaveBeenCalledWith({
       metadata: { namespace: "default", name: "test" },
       status: {
+        phase: Phase.Retrying,
         retryAttempt: 1,
       },
     });

src/pepr/operator/reconcilers/index.ts

@@ -12,6 +12,7 @@ export const uidSeen = new Set<string>();
  * @returns true if the CRD is pending or the current generation has been processed
  */
 export function shouldSkip(cr: UDSPackage) {
+  const isRetrying = cr.status?.phase === Phase.Retrying;
   const isPending = cr.status?.phase === Phase.Pending;
   const isCurrentGeneration = cr.metadata?.generation === cr.status?.observedGeneration;
 
@@ -22,6 +23,12 @@ export function shouldSkip(cr: UDSPackage) {
     return false;
   }
 
+  // If the CR is retrying, it should not be skipped
+  if (isRetrying) {
+    Log.debug(cr, `Should skip? No, retrying`);
+    return false;
+  }
+
   // This is the second time the CR has been seen, so check if it is pending or the current generation
   if (isPending || isCurrentGeneration) {
     Log.debug(cr, `Should skip? Yes, pending or current generation and not first time seen`);
@@ -50,6 +57,9 @@ export async function updateStatus(cr: UDSPackage, status: PkgStatus) {
     },
     status,
   });
+
+  // Track the UID of the CRD to know if it has been seen before
+  uidSeen.add(cr.metadata!.uid!);
 }
 
 /**
@@ -95,7 +105,7 @@ export async function handleFailure(err: { status: number; message: string }, cr
   const identifier = `${metadata.namespace}/${metadata.name}`;
   let status: Status;
 
-  // todo: identify exact 404 we are targetting, possibly in `updateStatus`
+  // todo: identify exact 404 we are targeting, possibly in `updateStatus`
   if (err.status === 404) {
     Log.warn({ err }, `Package metadata seems to have been deleted`);
     return;
@@ -108,6 +118,7 @@ export async function handleFailure(err: { status: number; message: string }, cr
     Log.error({ err }, `Reconciliation attempt ${currRetry} failed for ${identifier}, retrying...`);
 
     status = {
+      phase: Phase.Retrying,
       retryAttempt: currRetry,
     };
   } else {

src/pepr/operator/reconcilers/package-reconciler.ts

@@ -1,6 +1,6 @@
 import { Log } from "pepr";
 
-import { handleFailure, shouldSkip, uidSeen, updateStatus } from ".";
+import { handleFailure, shouldSkip, updateStatus } from ".";
 import { UDSConfig } from "../../config";
 import { enableInjection } from "../controllers/istio/injection";
 import { istioResources } from "../controllers/istio/istio-resources";
@@ -67,9 +67,6 @@ export async function packageReconciler(pkg: UDSPackage) {
       observedGeneration: metadata.generation,
       retryAttempt: 0, // todo: make this nullable when kfc generates the type
     });
-
-    // Update to indicate this version of pepr-core has reconciled the package successfully once
-    uidSeen.add(pkg.metadata!.uid!);
   } catch (err) {
     void handleFailure(err, pkg);
   }