Skip to content

Commit ed230dc

Browse files
mozesl-nokiaCopilot
mozesl-nokia
and
Copilot
authored
Make porch-server manager-runnable (#1111)
* make PorchServer manager runnable Assisted-by: Cursor:composer-2.5 Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com> * go mod tidy Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com> * fix potentially flaky e2e test Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com> * Apply suggestions from code review Signed-off-by: mozesl-nokia <laszlo.mozes@nokia.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: mozesl-nokia <laszlo.mozes@nokia.com> * improve coverage Assisted-by: Cursor:composer-2.5 Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com> * extract consts in apiserver_test.go Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com> * try improve coverage Assisted-by: Cursor:grok-4.5 Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com> * remove false positive security issue Assisted-by: Cursor:grok-4.5 Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com> * fix unit test Assisted-by: Cursor:grok-4.5 Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com> * try fix race condition causing unexpected mock call Assisted-by: Cursor:grok-4.5 Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com> * restructure after merge Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com> * fix comment Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com> --------- Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com> Signed-off-by: mozesl-nokia <laszlo.mozes@nokia.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent cad1567 commit ed230dc

31 files changed

Lines changed: 1277 additions & 411 deletions

controllers/functionconfigs/reconciler/functionconfigreconciler.go renamed to controllers/functionconfigs/functionconfigreconciler.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
package reconciler
15+
package functionconfigs
1616

1717
import (
1818
"context"
@@ -35,6 +35,7 @@ import (
3535
ctrl "sigs.k8s.io/controller-runtime"
3636
"sigs.k8s.io/controller-runtime/pkg/client"
3737
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
38+
"sigs.k8s.io/controller-runtime/pkg/predicate"
3839
)
3940

4041
const BaseFinalizer = "config.porch.kpt.dev/functionconfig"
@@ -250,15 +251,22 @@ const (
250251
ReconcilerForController ReconcilerFor = "controller"
251252
)
252253

253-
type FunctionConfigReconciler struct {
254+
type Reconciler struct {
254255
Client client.Client
255256
FunctionConfigStore *FunctionConfigStore
256257
// For indicates which component the reconciler is collecting the configs for
257258
// TODO: remove after merging of function-runner into server
258259
For ReconcilerFor
259260
}
260261

261-
func (r *FunctionConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.Result, finalErr error) {
262+
func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
263+
return ctrl.NewControllerManagedBy(mgr).
264+
For(&configapi.FunctionConfig{}).
265+
WithEventFilter(predicate.GenerationChangedPredicate{}).
266+
Complete(r)
267+
}
268+
269+
func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.Result, finalErr error) {
262270
klog.Infof("FunctionConfig %q changed", req.NamespacedName)
263271
obj := &configapi.FunctionConfig{}
264272
err := r.Client.Get(ctx, req.NamespacedName, obj)
@@ -330,7 +338,7 @@ func (r *FunctionConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reque
330338
return ctrl.Result{}, nil
331339
}
332340

333-
func (r *FunctionConfigReconciler) removeFinalizer(ctx context.Context, obj *configapi.FunctionConfig) error {
341+
func (r *Reconciler) removeFinalizer(ctx context.Context, obj *configapi.FunctionConfig) error {
334342
patch := client.MergeFrom(obj.DeepCopy())
335343

336344
switch r.For {
@@ -350,7 +358,7 @@ func (r *FunctionConfigReconciler) removeFinalizer(ctx context.Context, obj *con
350358
return nil
351359
}
352360

353-
func (r *FunctionConfigReconciler) addFinalizer(ctx context.Context, obj *configapi.FunctionConfig) error {
361+
func (r *Reconciler) addFinalizer(ctx context.Context, obj *configapi.FunctionConfig) error {
354362
patch := client.MergeFrom(obj.DeepCopy())
355363

356364
updated := false

controllers/functionconfigs/reconciler/functionconfigreconciler_test.go renamed to controllers/functionconfigs/functionconfigreconciler_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 The kpt Authors
1+
// Copyright 2026 The kpt Authors
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -12,7 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
package reconciler
15+
package functionconfigs
1616

1717
import (
1818
"context"
@@ -39,7 +39,7 @@ func TestFunctionConfigReconciler(t *testing.T) {
3939
type testcase struct {
4040
name string
4141
objs []client.Object // input: objects to seed the fake client
42-
check func(t *testing.T, reconciler *FunctionConfigReconciler)
42+
check func(t *testing.T, reconciler *Reconciler)
4343
requests []string // name set in the reconcile request
4444
expectErr bool
4545
}
@@ -137,7 +137,7 @@ func TestFunctionConfigReconciler(t *testing.T) {
137137
name: "FunctionConfig object is stored in FunctionStore after reconciliation",
138138
objs: []client.Object{sampleFunctionConfig},
139139
requests: []string{"set-image"},
140-
check: func(t *testing.T, r *FunctionConfigReconciler) {
140+
check: func(t *testing.T, r *Reconciler) {
141141
// Check existence of the functionConfig in cluster
142142
got, exists := r.FunctionConfigStore.GetFunctionConfig("set-image")
143143
expectedNumberOfFunctions := 1
@@ -152,7 +152,7 @@ func TestFunctionConfigReconciler(t *testing.T) {
152152
name: "FunctionConfig object is deleted from FunctionStore after reconciliation",
153153
objs: []client.Object{},
154154
requests: []string{"set-image"},
155-
check: func(t *testing.T, r *FunctionConfigReconciler) {
155+
check: func(t *testing.T, r *Reconciler) {
156156
// Check existence of the functionConfig in cluster
157157
_, exists := r.FunctionConfigStore.GetFunctionConfig("set-image")
158158
assert.False(t, exists, "FunctionConfig 'set-image' should not exist in the store")
@@ -162,7 +162,7 @@ func TestFunctionConfigReconciler(t *testing.T) {
162162
name: "BinaryExecutorCache is available with image",
163163
objs: []client.Object{sampleFunctionConfig},
164164
requests: []string{"set-image"},
165-
check: func(t *testing.T, r *FunctionConfigReconciler) {
165+
check: func(t *testing.T, r *Reconciler) {
166166
expectedKey := "ghcr.io/kptdev/krm-functions-catalog/set-image:v0.1.4"
167167
expectedPath := "/functions/set-image"
168168
binary, exists := r.FunctionConfigStore.GetBinaryFromCache(expectedKey)
@@ -174,7 +174,7 @@ func TestFunctionConfigReconciler(t *testing.T) {
174174
name: "BuiltInExecutorCache is available for starlark",
175175
objs: []client.Object{builtInSetNamespace, builtInApplyReplacements, builtInStarlarkWithId},
176176
requests: []string{"apply-replacements", "set-namespace", "starlark"},
177-
check: func(t *testing.T, r *FunctionConfigReconciler) {
177+
check: func(t *testing.T, r *Reconciler) {
178178
expectedStarlarkKey := "starlark-id"
179179
execFunctions := r.FunctionConfigStore.GetExecCache()
180180

@@ -197,7 +197,7 @@ func TestFunctionConfigReconciler(t *testing.T) {
197197
c := fake.NewClientBuilder().WithObjects(tt.objs...).WithScheme(scheme).WithStatusSubresource(&configapi.FunctionConfig{}).Build()
198198

199199
functionConfigStore := NewFunctionConfigStore(defaultImagePrefix, functionCacheDir)
200-
reconciler := &FunctionConfigReconciler{
200+
reconciler := &Reconciler{
201201
Client: c,
202202
FunctionConfigStore: functionConfigStore,
203203
}
@@ -260,7 +260,7 @@ func TestFinalizersAdded(t *testing.T) {
260260
}
261261

262262
c := fake.NewClientBuilder().WithScheme(schemeWithFunctionConfig(t)).WithObjects(obj).WithStatusSubresource(&configapi.FunctionConfig{}).Build()
263-
r := &FunctionConfigReconciler{
263+
r := &Reconciler{
264264
Client: c,
265265
FunctionConfigStore: NewFunctionConfigStore(defaultImagePrefix, functionCacheDir),
266266
For: tc.forValue,
@@ -528,7 +528,7 @@ func TestFinalizersRemoved(t *testing.T) {
528528
store := NewFunctionConfigStore(defaultImagePrefix, functionCacheDir)
529529
store.UpsertFunctionConfig(objName, obj)
530530

531-
r := &FunctionConfigReconciler{
531+
r := &Reconciler{
532532
Client: c,
533533
FunctionConfigStore: store,
534534
For: tc.forValue,

‎controllers/main.go‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727

2828
"slices"
2929

30+
"github.com/kptdev/porch/controllers/functionconfigs"
3031
// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
3132
// to ensure that exec-entrypoint and run can make use of them.
3233
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
@@ -40,7 +41,6 @@ import (
4041
"sigs.k8s.io/controller-runtime/pkg/webhook"
4142

4243
"github.com/kptdev/kpt/pkg/lib/runneroptions"
43-
"github.com/kptdev/porch/controllers/functionconfigs/reconciler"
4444
"github.com/kptdev/porch/controllers/packagerevisions/pkg/controllers/packagerevision"
4545
"github.com/kptdev/porch/controllers/packagevariants/pkg/controllers/packagevariant"
4646
"github.com/kptdev/porch/controllers/packagevariantsets/pkg/controllers/packagevariantset"
@@ -52,7 +52,6 @@ import (
5252
ctrl "sigs.k8s.io/controller-runtime"
5353
"sigs.k8s.io/controller-runtime/pkg/healthz"
5454
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
55-
"sigs.k8s.io/controller-runtime/pkg/predicate"
5655
"sigs.k8s.io/controller-runtime/pkg/reconcile"
5756

5857
porchapi "github.com/kptdev/porch/api/porch/v1alpha1"
@@ -305,37 +304,34 @@ func setupReconciler(mgr ctrl.Manager, enabled []string, r Reconciler, started [
305304
return append(started, name), nil
306305
}
307306

308-
func setupFunctionConfigReconciler(mgr ctrl.Manager) (*reconciler.FunctionConfigStore, error) {
307+
func setupFunctionConfigReconciler(mgr ctrl.Manager) (*functionconfigs.FunctionConfigStore, error) {
309308
prefix := os.Getenv("DEFAULT_IMAGE_PREFIX")
310309
if prefix == "" {
311310
prefix = runneroptions.GHCRImagePrefix
312311
}
313-
functionConfigStore := reconciler.NewFunctionConfigStore(prefix, "")
312+
functionConfigStore := functionconfigs.NewFunctionConfigStore(prefix, "")
314313

315-
rec := &reconciler.FunctionConfigReconciler{
314+
rec := &functionconfigs.Reconciler{
316315
Client: mgr.GetClient(),
317316
FunctionConfigStore: functionConfigStore,
318-
For: reconciler.ReconcilerForController,
317+
For: functionconfigs.ReconcilerForController,
319318
}
320319

321-
if err := ctrl.NewControllerManagedBy(mgr).
322-
For(&configapi.FunctionConfig{}).
323-
WithEventFilter(predicate.GenerationChangedPredicate{}).
324-
Complete(rec); err != nil {
320+
if err := rec.SetupWithManager(mgr); err != nil {
325321
return nil, fmt.Errorf("error creating FunctionConfig controller: %w", err)
326322
}
327323

328324
prePopulateFunctionConfigStore(mgr.GetAPIReader(), functionConfigStore)
329325

330-
klog.Infof("FunctionConfig reconciler registered (for: %s)", reconciler.ReconcilerForController)
326+
klog.Infof("FunctionConfig reconciler registered (for: %s)", functionconfigs.ReconcilerForController)
331327
return functionConfigStore, nil
332328
}
333329

334330
// prePopulateFunctionConfigStore loads all FunctionConfigs into the store
335331
// synchronously so the exec cache is ready before the PR controller starts.
336332
// Without this, a pod restart leaves the cache empty until the async
337333
// informer triggers reconciliation.
338-
func prePopulateFunctionConfigStore(reader client.Reader, store *reconciler.FunctionConfigStore) {
334+
func prePopulateFunctionConfigStore(reader client.Reader, store *functionconfigs.FunctionConfigStore) {
339335
var fcList configapi.FunctionConfigList
340336
if err := reader.List(context.Background(), &fcList); err != nil {
341337
klog.Warningf("FunctionConfig pre-population failed (non-fatal): %v", err)

‎controllers/main_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
"testing"
2121

2222
configapi "github.com/kptdev/porch/api/porchconfig/v1alpha1"
23-
"github.com/kptdev/porch/controllers/functionconfigs/reconciler"
23+
"github.com/kptdev/porch/controllers/functionconfigs"
2424
mockclient "github.com/kptdev/porch/test/mockery/mocks/external/sigs.k8s.io/controller-runtime/pkg/client"
2525
"github.com/stretchr/testify/assert"
2626
"github.com/stretchr/testify/mock"
@@ -171,7 +171,7 @@ func TestPrePopulateFunctionConfigStore_Success(t *testing.T) {
171171
list.(*configapi.FunctionConfigList).Items = items
172172
}).Return(nil)
173173

174-
store := reconciler.NewFunctionConfigStore("ghcr.io/kptdev", "/tmp/bins")
174+
store := functionconfigs.NewFunctionConfigStore("ghcr.io/kptdev", "/tmp/bins")
175175
prePopulateFunctionConfigStore(mockReader, store)
176176

177177
_, ok := store.GetFunctionConfig("set-namespace")
@@ -186,7 +186,7 @@ func TestPrePopulateFunctionConfigStore_ListError(t *testing.T) {
186186
mockReader := mockclient.NewMockReader(t)
187187
mockReader.EXPECT().List(mock.Anything, mock.Anything, mock.Anything).Return(assert.AnError)
188188

189-
store := reconciler.NewFunctionConfigStore("ghcr.io/kptdev", "/tmp/bins")
189+
store := functionconfigs.NewFunctionConfigStore("ghcr.io/kptdev", "/tmp/bins")
190190
prePopulateFunctionConfigStore(mockReader, store)
191191

192192
_, ok := store.GetFunctionConfig("anything")
@@ -198,7 +198,7 @@ func TestPrePopulateFunctionConfigStore_EmptyList(t *testing.T) {
198198
mockReader.EXPECT().List(mock.Anything, mock.AnythingOfType("*v1alpha1.FunctionConfigList"), mock.Anything).
199199
Return(nil)
200200

201-
store := reconciler.NewFunctionConfigStore("ghcr.io/kptdev", "/tmp/bins")
201+
store := functionconfigs.NewFunctionConfigStore("ghcr.io/kptdev", "/tmp/bins")
202202
prePopulateFunctionConfigStore(mockReader, store)
203203

204204
assert.Equal(t, 0, len(store.List()))

‎controllers/packagerevisions/pkg/controllers/packagerevision/config_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import (
44
"flag"
55
"testing"
66

7-
"github.com/kptdev/porch/controllers/functionconfigs/reconciler"
7+
"github.com/kptdev/porch/controllers/functionconfigs"
88
"github.com/stretchr/testify/assert"
99
"github.com/stretchr/testify/require"
1010
"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -62,7 +62,7 @@ func TestInit_NilCache(t *testing.T) {
6262
r := &PackageRevisionReconciler{
6363
RepoOperationRetryAttempts: 3,
6464
MaxGRPCMessageSize: defaultMaxGRPCMessageSize,
65-
FunctionConfigStore: reconciler.NewFunctionConfigStore("", ""),
65+
FunctionConfigStore: functionconfigs.NewFunctionConfigStore("", ""),
6666
}
6767
err := r.Init(mgr)
6868
require.NoError(t, err)
@@ -75,7 +75,7 @@ func TestInit_SetsCredResolverAndFetcher(t *testing.T) {
7575
r := &PackageRevisionReconciler{
7676
RepoOperationRetryAttempts: 3,
7777
MaxGRPCMessageSize: defaultMaxGRPCMessageSize,
78-
FunctionConfigStore: reconciler.NewFunctionConfigStore("", ""),
78+
FunctionConfigStore: functionconfigs.NewFunctionConfigStore("", ""),
7979
}
8080

8181
err := r.Init(mgr)
@@ -93,7 +93,7 @@ func TestInit_RendererEnabledWithFnRunner(t *testing.T) {
9393
r := &PackageRevisionReconciler{
9494
RepoOperationRetryAttempts: 3,
9595
MaxGRPCMessageSize: defaultMaxGRPCMessageSize,
96-
FunctionConfigStore: reconciler.NewFunctionConfigStore("", ""),
96+
FunctionConfigStore: functionconfigs.NewFunctionConfigStore("", ""),
9797
}
9898

9999
t.Setenv("FUNCTION_RUNNER_ADDRESS", "localhost:0")

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
"time"
2121

2222
porchv1alpha2 "github.com/kptdev/porch/api/porch/v1alpha2"
23-
"github.com/kptdev/porch/controllers/functionconfigs/reconciler"
23+
"github.com/kptdev/porch/controllers/functionconfigs"
2424
"github.com/kptdev/porch/pkg/repository"
2525
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2626
"k8s.io/apimachinery/pkg/runtime"
@@ -52,7 +52,7 @@ type PackageRevisionReconciler struct {
5252
Scheme *runtime.Scheme
5353
ContentCache repository.ContentCache
5454
ExternalPackageFetcher repository.ExternalPackageFetcher
55-
FunctionConfigStore *reconciler.FunctionConfigStore
55+
FunctionConfigStore *functionconfigs.FunctionConfigStore
5656
Renderer renderer // nil = skip rendering
5757

5858
MaxConcurrentReconciles int

0 commit comments

Comments
 (0)