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);
  }