refactor how the syncagent integrates with multicluster-runtime

Since the mcprovider now watches the APIExportEndpointSlice itself, it does not make sense anymore
to start it dynamically on demand. There is no point in having the syncmanager watch an APIExport or
EndpointSlice itself, since the mcprovider does not take any URLs as input values anymore, it relies
solely on watching the EndpointSlice itself.

To accomodate this, I moved the manager and its provider up into the main(). Since the syncmanager
controller however not just simply starts an mcmanager/provider, but also deals with dynamically
started controllers, I decided to move that behaviour into a new DynamicMultiClusterManager (DMCM).

The DMCM is capable of keeping track of all engaged clusters and allows you to add and start
controllers at any time. It will automatically pre-seed new controller instances with the known
clusters.

There is one downside: Right now, if the controller fails to start, the syncmanager controller will
not notice, re-reconcile and restore it. There's no back channel for that error from the sync
controller's goroutine yet.

---

Since the syncagent by design dynamically fills APIExports with resource schemas, when the DMCM is
started, there are most likely (at least in cold start situations) no resource schemas yet in the
APIExport. Even though the syncmanager waits for the apiresourceschema controller to be done, it
currently does not wait for the apiexport controller to be done (in adding that ResourceSchema to
the APIExport) (there is no condition or anything to tell when that controller has finished).

This means the syncmanager will start new sync controllers at the same time as the apiexport
controller is still filling the APIExport. However when the resources are not present yet, these
sync controllers fail to boot up, and since there is currently no channel to report this back to the
syncmanager, they do not get quickly restarted.

To handle this (it might have been simpler to wait for the APIExport to be ready, but alas... I
realized my oversight too late), I added an explicit API discovery step, where the sync controllers
are only started if and when the primary resource is actually available.

---

One major side effect of this whole refactoring is of course: the agent does not longer support
watching APIExports for their deprecated virtual workspace URLs. When this is merged, you must
provide an APIExportEndpointSlice.

On-behalf-of: @SAP [email protected]

Author: xrstf

cmd/api-syncagent/main.go

@@ -32,12 +32,12 @@ import (
 	"github.com/kcp-dev/api-syncagent/internal/controller/apiexport"
 	"github.com/kcp-dev/api-syncagent/internal/controller/apiresourceschema"
 	"github.com/kcp-dev/api-syncagent/internal/controller/syncmanager"
+	"github.com/kcp-dev/api-syncagent/internal/discovery"
+	"github.com/kcp-dev/api-syncagent/internal/kcp"
 	syncagentlog "github.com/kcp-dev/api-syncagent/internal/log"
 	"github.com/kcp-dev/api-syncagent/internal/version"
 	syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1"
 
-	kcpapisv1alpha1 "github.com/kcp-dev/sdk/apis/apis/v1alpha1"
-
 	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -147,6 +147,21 @@ func run(ctx context.Context, log *zap.SugaredLogger, opts *Options) error {
 		if err := mgr.Add(endpointKcpCluster); err != nil {
 			return fmt.Errorf("failed to add endpoint kcp cluster runnable: %w", err)
 		}
+
+		endpointSliceCluster = endpointKcpCluster
+	}
+
+	// Setup the magical dynamic multicluster manager. It's a dynamic version of the
+	// regular mcmanager, capable of starting new controllers at any later time and
+	// allowing them to be also stopped at any time. The syncmanager needs it to
+	// start/stop sync controllers for each PublishedResource.
+	dmcm, err := kcp.NewDynamicMultiClusterManager(endpoint.EndpointSlice.Config, endpoint.EndpointSlice.Name)
+	if err != nil {
+		return fmt.Errorf("failed to start dynamic multi cluster manager: %w", err)
+	}
+
+	if err := mgr.Add(dmcm); err != nil {
+		return fmt.Errorf("failed to add endpoint kcp cluster runnable: %w", err)
 	}
 
 	startController := func(name string, creator func() error) error {
@@ -177,20 +192,11 @@ func run(ctx context.Context, log *zap.SugaredLogger, opts *Options) error {
 	// This controller is called "sync" because it makes the most sense to the users, even though internally the relevant
 	// controller is the syncmanager (which in turn would start/stop the sync controllers).
 	if err := startController("sync", func() error {
-		cluster := endpointKcpCluster
-		if cluster == nil {
-			cluster = managedKcpCluster
-		}
-
-		var endpointSlice *kcpdevv1alpha1.APIExportEndpointSlice
-		if endpoint.EndpointSlice != nil {
-			endpointSlice = endpoint.EndpointSlice.APIExportEndpointSlice
-		}
-
-		// It doesn't matter which rest config we specify, as the URL will be overwritten with the
-		// virtual workspace URL anyway.
+		// The syncmanager needs to be able to determine whether an API is already bound and available
+		// before it can start any sync controllers. That discovery logic is encapsulated in the ResourceProber.
+		prober := discovery.NewResourceProber(endpoint.EndpointSlice.Config, endpointSliceCluster.GetClient(), endpoint.EndpointSlice.Name)
 
-		return syncmanager.Add(ctx, mgr, cluster, kcpRestConfig, log, endpoint.APIExport.APIExport, endpointSlice, opts.PublishedResourceSelector, opts.Namespace, opts.AgentName)
+		return syncmanager.Add(ctx, mgr, prober, dmcm, log, opts.PublishedResourceSelector, opts.Namespace, opts.AgentName)
 	}); err != nil {
 		return err
 	}

go.mod

@@ -16,6 +16,7 @@ require (
 	github.com/go-logr/zapr v1.3.0
 	github.com/google/cel-go v0.26.0
 	github.com/google/go-cmp v0.7.0
+	github.com/google/uuid v1.6.0
 	github.com/kcp-dev/api-syncagent/sdk v0.0.0-00010101000000-000000000000
 	github.com/kcp-dev/kcp v0.29.1-0.20251210093424-08fb9eb48494
 	github.com/kcp-dev/logicalcluster/v3 v3.0.5
@@ -79,7 +80,6 @@ require (
 	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/google/btree v1.1.3 // indirect
 	github.com/google/gnostic-models v0.7.0 // indirect
-	github.com/google/uuid v1.6.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.3 // indirect
 	github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect

internal/controller/sync/controller.go

@@ -134,12 +134,12 @@ func Create(
 	// The manager parameter is mostly unused and will be removed in future CR versions.
 	c, err := mccontroller.NewUnmanaged(ControllerName, remoteManager, ctrlOptions)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to instantiate new controller: %w", err)
 	}
 
 	// watch the target resource in the virtual workspace
 	if err := c.MultiClusterWatch(mcsource.TypedKind(remoteDummy, mchandler.TypedEnqueueRequestForObject[*unstructured.Unstructured]())); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to setup remote-side watch: %w", err)
 	}
 
 	// watch the source resource in the local cluster, but enqueue the origin remote object
@@ -158,7 +158,7 @@ func Create(
 	})
 
 	if err := c.Watch(source.TypedKind(localManager.GetCache(), localDummy, enqueueRemoteObjForLocalObj, nameFilter)); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to setup local-side watch: %w", err)
 	}
 
 	log.Info("Done setting up unmanaged controller.")