CRD E2E Ginkgo Suite

.github/workflows/porch-e2e-ci-jobs.yaml

@@ -140,6 +140,11 @@ jobs:
             test_path: "${GITHUB_WORKSPACE}/test/e2e/cli"
             test_env: "E2E=1"
             log_prefix: "porch-cli-e2e-dbcache"
+          - name: "Porch CRD E2E Tests (v1alpha2)"
+            make_target: "run-in-kind-v1alpha2"
+            test_path: "${GITHUB_WORKSPACE}/test/e2e/crd -ginkgo.v"
+            test_env: "E2E=1"
+            log_prefix: "porch-crd-e2e"
 
     steps:
       - name: Checkout Porch
@@ -171,7 +176,25 @@ jobs:
           kind load docker-image ${IMAGE_REPO}/${PORCH_FUNCTION_RUNNER_IMAGE}:${{ needs.build.outputs.image-tag }} -n ${KIND_CONTEXT_NAME}
           kind load docker-image ${IMAGE_REPO}/${PORCH_WRAPPER_SERVER_IMAGE}:${{ needs.build.outputs.image-tag }} -n ${KIND_CONTEXT_NAME}
       - name: Deploy porch kpt pkg
+        timeout-minutes: 10
         run: IMAGE_TAG=${{ needs.build.outputs.image-tag }} SKIP_IMG_BUILD=true make ${{ matrix.make_target }}
+      - name: Dump cluster state on deploy failure
+        if: failure()
+        run: |
+          echo "=== Pods (all namespaces) ==="
+          kubectl get pods -A
+          echo "=== Events (porch-system) ==="
+          kubectl get events -n porch-system --sort-by='.lastTimestamp' | tail -40
+          echo "=== Non-running pods details ==="
+          kubectl get pods -A -o json | jq -r '.items[] | select(.status.phase != "Running" and .status.phase != "Succeeded") | "\n--- \(.metadata.namespace)/\(.metadata.name) (\(.status.phase)) ---"' 
+          for pod in $(kubectl get pods -A -o json | jq -r '.items[] | select(.status.phase != "Running" and .status.phase != "Succeeded") | "\(.metadata.namespace)/\(.metadata.name)"'); do
+            ns=$(echo $pod | cut -d/ -f1)
+            name=$(echo $pod | cut -d/ -f2)
+            echo "--- Describe $ns/$name ---"
+            kubectl describe pod -n $ns $name | tail -20
+            echo "--- Logs $ns/$name ---"
+            kubectl logs -n $ns $name --all-containers --tail=30 2>/dev/null || true
+          done
       - name: Run E2E tests
         run: ${{ matrix.test_env }} go test -v -timeout 20m ${{ matrix.test_path }}
       - name: Export porch server logs

controllers/functionconfigs/reconciler/functionconfigreconciler.go

@@ -125,6 +125,9 @@ func (s *FunctionConfigStore) UpdateBinaryCache(_ string, obj *configapi.Functio
 }
 
 func (s *FunctionConfigStore) UpdateExecCache(name string, functionConfig *configapi.FunctionConfig) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
 	functionAliases := map[string][]string{}
 
 	id := name
@@ -201,6 +204,14 @@ func (s *FunctionConfigStore) GetExecCache() map[string]fnsdk.ResourceListProces
 	return s.builtInExecutorCache
 }
 
+// GetProcessorFromCache looks up a function processor by image, holding the read lock for the duration of the lookup.
+func (s *FunctionConfigStore) GetProcessorFromCache(image string) (fnsdk.ResourceListProcessor, bool) {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	processor, found := s.builtInExecutorCache[image]
+	return processor, found
+}
+
 func (s *FunctionConfigStore) List() []*configapi.FunctionConfig {
 	s.mu.Lock()
 	defer s.mu.Unlock()
@@ -268,6 +279,9 @@ func (r *FunctionConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 
 		if err := r.Client.Status().Patch(ctx, obj, patch); err != nil {
 			klog.Errorf("Failed to update status of FunctionConfig %q: %v", obj.Name, err)
+			if finalErr == nil {
+				finalErr = err
+			}
 		}
 	}()
 

controllers/functionconfigs/reconciler/functionconfigreconciler_test.go

@@ -194,7 +194,7 @@ func TestFunctionConfigReconciler(t *testing.T) {
 	for _, tt := range tests {
 		tt := tt // pin for closure
 		t.Run(tt.name, func(t *testing.T) {
-			c := fake.NewClientBuilder().WithObjects(tt.objs...).WithScheme(scheme).Build()
+			c := fake.NewClientBuilder().WithObjects(tt.objs...).WithScheme(scheme).WithStatusSubresource(&configapi.FunctionConfig{}).Build()
 
 			functionConfigStore := NewFunctionConfigStore(defaultImagePrefix, functionCacheDir)
 			reconciler := &FunctionConfigReconciler{
@@ -259,7 +259,7 @@ func TestFinalizersAdded(t *testing.T) {
 				},
 			}
 
-			c := fake.NewClientBuilder().WithScheme(schemeWithFunctionConfig(t)).WithObjects(obj).Build()
+			c := fake.NewClientBuilder().WithScheme(schemeWithFunctionConfig(t)).WithObjects(obj).WithStatusSubresource(&configapi.FunctionConfig{}).Build()
 			r := &FunctionConfigReconciler{
 				Client:              c,
 				FunctionConfigStore: NewFunctionConfigStore(defaultImagePrefix, functionCacheDir),
@@ -279,6 +279,141 @@ func TestFinalizersAdded(t *testing.T) {
 	}
 }
 
+func TestGetProcessorFromCache(t *testing.T) {
+	store := NewFunctionConfigStore(defaultImagePrefix, functionCacheDir)
+
+	// Populate via UpdateExecCache (same path as reconciler)
+	obj := &configapi.FunctionConfig{
+		ObjectMeta: metav1.ObjectMeta{Name: "set-namespace", Namespace: testNamespace},
+		Spec: configapi.FunctionConfigSpec{
+			Image:    "set-namespace",
+			Prefixes: []string{""},
+			GoExecutor: &configapi.GoExecutorConfig{
+				Tags: []string{"v0.4.1"},
+			},
+		},
+	}
+	store.UpdateExecCache(obj.Name, obj)
+
+	// Found with full prefix
+	processor, found := store.GetProcessorFromCache("ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1")
+	assert.True(t, found)
+	assert.NotNil(t, processor)
+
+	// Found without prefix (short form)
+	processor, found = store.GetProcessorFromCache("set-namespace:v0.4.1")
+	assert.True(t, found)
+	assert.NotNil(t, processor)
+
+	// Not found for unknown tag
+	_, found = store.GetProcessorFromCache("set-namespace:v9.9.9")
+	assert.False(t, found)
+
+	// Not found for unknown image
+	_, found = store.GetProcessorFromCache("nonexistent:v1.0.0")
+	assert.False(t, found)
+}
+
+func TestPrePopulationPattern(t *testing.T) {
+	// Simulates what setupFunctionConfigReconciler does on cold start:
+	// list all FunctionConfigs and populate the store synchronously
+	// without going through the reconcile loop.
+	store := NewFunctionConfigStore(defaultImagePrefix, functionCacheDir)
+
+	configs := []configapi.FunctionConfig{
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: "set-namespace", Namespace: testNamespace},
+			Spec: configapi.FunctionConfigSpec{
+				Image:      "set-namespace",
+				Prefixes:   []string{""},
+				GoExecutor: &configapi.GoExecutorConfig{Tags: []string{"v0.4.1"}},
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: "apply-replacements", Namespace: testNamespace},
+			Spec: configapi.FunctionConfigSpec{
+				Image:      "apply-replacements",
+				Prefixes:   []string{""},
+				GoExecutor: &configapi.GoExecutorConfig{Tags: []string{"v0.1.1"}},
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: "set-image", Namespace: testNamespace},
+			Spec: configapi.FunctionConfigSpec{
+				Image:    "set-image",
+				Prefixes: []string{""},
+				BinaryExecutor: &configapi.BinaryExecutorConfig{
+					Tags: []string{"v0.1.4"},
+					Path: "set-image",
+				},
+			},
+		},
+	}
+
+	// Pre-populate (mirrors the code in setupFunctionConfigReconciler)
+	for i := range configs {
+		obj := &configs[i]
+		store.UpsertFunctionConfig(obj.Name, obj)
+		if obj.Spec.GoExecutor != nil {
+			store.UpdateExecCache(obj.Name, obj)
+		}
+		if obj.Spec.BinaryExecutor != nil {
+			store.UpdateBinaryCache(obj.Name, obj)
+		}
+	}
+
+	// Verify exec cache is populated
+	_, found := store.GetProcessorFromCache("ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1")
+	assert.True(t, found, "set-namespace should be in exec cache after pre-population")
+
+	_, found = store.GetProcessorFromCache("ghcr.io/kptdev/krm-functions-catalog/apply-replacements:v0.1.1")
+	assert.True(t, found, "apply-replacements should be in exec cache after pre-population")
+
+	// Verify binary cache is populated
+	path, found := store.GetBinaryFromCache("ghcr.io/kptdev/krm-functions-catalog/set-image:v0.1.4")
+	assert.True(t, found, "set-image should be in binary cache after pre-population")
+	assert.Equal(t, "/functions/set-image", path)
+
+	// Verify function configs are stored
+	assert.Len(t, store.List(), 3)
+}
+
+func TestConcurrentAccessSafety(t *testing.T) {
+	// Verifies no data race when UpdateExecCache and GetProcessorFromCache
+	// are called concurrently (the fix for the data race bug).
+	store := NewFunctionConfigStore(defaultImagePrefix, functionCacheDir)
+
+	obj := &configapi.FunctionConfig{
+		ObjectMeta: metav1.ObjectMeta{Name: "set-namespace", Namespace: testNamespace},
+		Spec: configapi.FunctionConfigSpec{
+			Image:      "set-namespace",
+			Prefixes:   []string{""},
+			GoExecutor: &configapi.GoExecutorConfig{Tags: []string{"v0.4.1"}},
+		},
+	}
+
+	done := make(chan struct{})
+
+	// Writer goroutine
+	go func() {
+		defer close(done)
+		for i := 0; i < 100; i++ {
+			store.UpdateExecCache(obj.Name, obj)
+		}
+	}()
+
+	// Reader goroutine (concurrent with writer)
+	for i := 0; i < 100; i++ {
+		store.GetProcessorFromCache("ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1")
+	}
+
+	<-done
+
+	// After all writes complete, the entry should be present
+	_, found := store.GetProcessorFromCache("ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1")
+	assert.True(t, found)
+}
+
 func TestFinalizersRemoved(t *testing.T) {
 	now := metav1.Now()
 	const testFinalizer = "config.porch.kpt.dev/test-hold"
@@ -318,7 +453,7 @@ func TestFinalizersRemoved(t *testing.T) {
 				},
 			}
 
-			c := fake.NewClientBuilder().WithScheme(schemeWithFunctionConfig(t)).WithObjects(obj).Build()
+			c := fake.NewClientBuilder().WithScheme(schemeWithFunctionConfig(t)).WithObjects(obj).WithStatusSubresource(&configapi.FunctionConfig{}).Build()
 			store := NewFunctionConfigStore(defaultImagePrefix, functionCacheDir)
 			store.UpsertFunctionConfig(objName, obj)
 

controllers/main.go

@@ -319,10 +319,35 @@ func setupFunctionConfigReconciler(mgr ctrl.Manager) (*reconciler.FunctionConfig
 		return nil, fmt.Errorf("error creating FunctionConfig controller: %w", err)
 	}
 
+	prePopulateFunctionConfigStore(mgr.GetAPIReader(), functionConfigStore)
+
 	klog.Infof("FunctionConfig reconciler registered (for: %s)", reconciler.ReconcilerForController)
 	return functionConfigStore, nil
 }
 
+// prePopulateFunctionConfigStore loads all FunctionConfigs into the store
+// synchronously so the exec cache is ready before the PR controller starts.
+// Without this, a pod restart leaves the cache empty until the async
+// informer triggers reconciliation.
+func prePopulateFunctionConfigStore(reader client.Reader, store *reconciler.FunctionConfigStore) {
+	var fcList configapi.FunctionConfigList
+	if err := reader.List(context.Background(), &fcList); err != nil {
+		klog.Warningf("FunctionConfig pre-population failed (non-fatal): %v", err)
+		return
+	}
+	for i := range fcList.Items {
+		obj := &fcList.Items[i]
+		store.UpsertFunctionConfig(obj.Name, obj)
+		if obj.Spec.GoExecutor != nil {
+			store.UpdateExecCache(obj.Name, obj)
+		}
+		if obj.Spec.BinaryExecutor != nil {
+			store.UpdateBinaryCache(obj.Name, obj)
+		}
+	}
+	klog.Infof("FunctionConfig store pre-populated with %d configs", len(fcList.Items))
+}
+
 
 // --- Helpers ---
 

controllers/main_test.go

@@ -19,10 +19,15 @@ import (
 	"flag"
 	"testing"
 
+	configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1"
+	"github.com/nephio-project/porch/controllers/functionconfigs/reconciler"
+	mockclient "github.com/nephio-project/porch/test/mockery/mocks/external/sigs.k8s.io/controller-runtime/pkg/client"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
@@ -132,3 +137,69 @@ func TestReconcilersMapContainsAllReconcilers(t *testing.T) {
 	}
 	assert.Len(t, reconcilers, len(expected))
 }
+
+// --- prePopulateFunctionConfigStore ---
+
+func TestPrePopulateFunctionConfigStore_Success(t *testing.T) {
+	items := []configapi.FunctionConfig{
+		{
+			Spec: configapi.FunctionConfigSpec{
+				GoExecutor: &configapi.GoExecutorConfig{
+					Tags: []string{"v0.4.1"},
+				},
+			},
+		},
+		{
+			Spec: configapi.FunctionConfigSpec{
+				BinaryExecutor: &configapi.BinaryExecutorConfig{
+					Tags: []string{"v1.0.0"},
+					Path: "/usr/local/bin/starlark",
+				},
+			},
+		},
+		{
+			Spec: configapi.FunctionConfigSpec{},
+		},
+	}
+	items[0].Name = "set-namespace"
+	items[1].Name = "starlark"
+	items[2].Name = "no-executor"
+
+	mockReader := mockclient.NewMockReader(t)
+	mockReader.EXPECT().List(mock.Anything, mock.AnythingOfType("*v1alpha1.FunctionConfigList"), mock.Anything).
+		Run(func(_ context.Context, list client.ObjectList, _ ...client.ListOption) {
+			list.(*configapi.FunctionConfigList).Items = items
+		}).Return(nil)
+
+	store := reconciler.NewFunctionConfigStore("ghcr.io/kptdev", "/tmp/bins")
+	prePopulateFunctionConfigStore(mockReader, store)
+
+	_, ok := store.GetFunctionConfig("set-namespace")
+	assert.True(t, ok, "set-namespace should be in store")
+	_, ok = store.GetFunctionConfig("starlark")
+	assert.True(t, ok, "starlark should be in store")
+	_, ok = store.GetFunctionConfig("no-executor")
+	assert.True(t, ok, "no-executor should be in store")
+}
+
+func TestPrePopulateFunctionConfigStore_ListError(t *testing.T) {
+	mockReader := mockclient.NewMockReader(t)
+	mockReader.EXPECT().List(mock.Anything, mock.Anything, mock.Anything).Return(assert.AnError)
+
+	store := reconciler.NewFunctionConfigStore("ghcr.io/kptdev", "/tmp/bins")
+	prePopulateFunctionConfigStore(mockReader, store)
+
+	_, ok := store.GetFunctionConfig("anything")
+	assert.False(t, ok, "store should be empty after list error")
+}
+
+func TestPrePopulateFunctionConfigStore_EmptyList(t *testing.T) {
+	mockReader := mockclient.NewMockReader(t)
+	mockReader.EXPECT().List(mock.Anything, mock.AnythingOfType("*v1alpha1.FunctionConfigList"), mock.Anything).
+		Return(nil)
+
+	store := reconciler.NewFunctionConfigStore("ghcr.io/kptdev", "/tmp/bins")
+	prePopulateFunctionConfigStore(mockReader, store)
+
+	assert.Equal(t, 0, len(store.List()))
+}

controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller.go

@@ -94,6 +94,14 @@ func (r *PackageRevisionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return resultOrDefault(result), nil
 	}
 
+	// Re-read to pick up spec changes (e.g. lifecycle transitions) that
+	// occurred while render was in-flight. A validating webhook should
+	// eventually block lifecycle transitions during render, making this
+	// re-read unnecessary.
+	if err := r.Get(ctx, req.NamespacedName, &pr); err != nil {
+		return ctrl.Result{}, client.IgnoreNotFound(err)
+	}
+
 	return r.reconcileLifecycle(ctx, &pr, repoKey)
 }
 
@@ -135,7 +143,7 @@ func (r *PackageRevisionReconciler) reconcileLifecycle(ctx context.Context, pr *
 	if err != nil {
 		log.Error(err, "lifecycle transition failed")
 		r.updateStatus(ctx, pr, nil, "", readyCondition(pr.Generation, metav1.ConditionFalse, porchv1alpha2.ReasonFailed, err.Error()))
-		return ctrl.Result{}, nil
+		return ctrl.Result{Requeue: true}, nil
 	}
 
 	r.updateStatus(ctx, pr, updated, "", readyCondition(pr.Generation, metav1.ConditionTrue, porchv1alpha2.ReasonReady, ""))