From e584b2f777a8574adef5029f7a1697bda4a95e24 Mon Sep 17 00:00:00 2001 From: Jian Qiu Date: Tue, 28 Mar 2023 16:35:55 +0800 Subject: [PATCH] Update doc for workload sync remove auto-generated apibinding add a validation on sync command to avoid setting imported-apis in --api-export flag Signed-off-by: Jian Qiu --- ...tering-kubernetes-clusters-using-syncer.md | 16 ++-- pkg/cliplugins/workload/plugin/sync.go | 13 +-- .../defaultlocation_controller.go | 83 ------------------- test/e2e/syncer/syncer_test.go | 11 ++- tmc/pkg/server/controllers.go | 2 - 5 files changed, 24 insertions(+), 101 deletions(-) diff --git a/docs/content/concepts/registering-kubernetes-clusters-using-syncer.md b/docs/content/concepts/registering-kubernetes-clusters-using-syncer.md index 3914fb78fbca..c3f1805fd4fb 100644 --- a/docs/content/concepts/registering-kubernetes-clusters-using-syncer.md +++ b/docs/content/concepts/registering-kubernetes-clusters-using-syncer.md @@ -96,8 +96,8 @@ kubectl kcp workload sync --syncer-image --resources ro ``` And apply the generated manifests to the physical cluster. The syncer will then import the API schema of foo.bar -to the workspace of the synctarget, following up with an auto generated kubernetes APIExport/APIBinding in the same workspace. -You can then create foo.bar in this workspace, or create an APIBinding in another workspace to bind this APIExport. +to the workspace of the synctarget, following up with an auto generated kubernetes APIExport in the same workspace. +The auto generated APIExport name is `imported-apis`, and you can then create an APIBinding to bind this APIExport. To sync resource from another existing APIExport in the KCP server, run @@ -105,7 +105,8 @@ To sync resource from another existing APIExport in the KCP server, run kubectl kcp workload sync --syncer-image --apiexports another-workspace:another-apiexport -o syncer.yaml ``` -Syncer will start syncing the resources in this `APIExport` as long as the `SyncTarget` has compatible API schemas. +Syncer will start syncing the resources in this `APIExport` as long as the `SyncTarget` has compatible API schemas. The auto generated +APIExport `imported-apis` is a reserved APIExport name and should not be set in the `--apiexports` To see if a certain resource is supported to be synced by the syncer, you can check the state of the `syncedResources` in `SyncTarget` status. @@ -118,8 +119,13 @@ After the `SyncTarget` is ready, switch to any workspace containing some workloa kubectl kcp bind compute ``` -This command will create a `Placement` in the workspace. By default, it will also create `APIBinding`s for global kubernetes `APIExport` and -kubernetes `APIExport` in workspace of `SyncTarget`, if any of these `APIExport`s are supported by the `SyncTarget`. +This command will create a `Placement` in the workspace. By default, it will also create `APIBinding`s for global kubernetes `APIExport`. + +You can also bind to the auto generated `imported-apis` APIExport by running + +```sh +kubectl kcp bind compute --apiexports :imported-apis +``` Alternatively, if you would like to bind other `APIExport`s which are supported by the `SyncerTarget`, run: diff --git a/pkg/cliplugins/workload/plugin/sync.go b/pkg/cliplugins/workload/plugin/sync.go index a0d9dc2d5bbd..f9186f8dbfd0 100644 --- a/pkg/cliplugins/workload/plugin/sync.go +++ b/pkg/cliplugins/workload/plugin/sync.go @@ -198,6 +198,13 @@ func (o *SyncOptions) Validate() error { } } + for _, apiExport := range o.APIExports { + _, name := logicalcluster.NewPath(apiExport).Split() + if name == workloadv1alpha1.ImportedAPISExportName { + errs = append(errs, fmt.Errorf("%s is a reversed APIExport name and should not be set", workloadv1alpha1.ImportedAPISExportName)) + } + } + return utilerrors.NewAggregate(errs) } @@ -458,12 +465,8 @@ func (o *SyncOptions) getResourcesForPermission(ctx context.Context, config *res } // skip if there is only the local kubernetes APIExport in the synctarget workspace, since we may not get syncedResources yet. - clusterName := logicalcluster.From(syncTarget) - if len(syncTarget.Spec.SupportedAPIExports) == 1 && - syncTarget.Spec.SupportedAPIExports[0].Export == workloadv1alpha1.ImportedAPISExportName && - (len(syncTarget.Spec.SupportedAPIExports[0].Path) == 0 || - syncTarget.Spec.SupportedAPIExports[0].Path == clusterName.String()) { + syncTarget.Spec.SupportedAPIExports[0].Export == workloadv1alpha1.ImportedAPISExportName { return true, nil } diff --git a/pkg/reconciler/workload/defaultlocation/defaultlocation_controller.go b/pkg/reconciler/workload/defaultlocation/defaultlocation_controller.go index 4128f7278e1d..81cff20b14f4 100644 --- a/pkg/reconciler/workload/defaultlocation/defaultlocation_controller.go +++ b/pkg/reconciler/workload/defaultlocation/defaultlocation_controller.go @@ -35,12 +35,9 @@ import ( apisv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/apis/v1alpha1" schedulingv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/scheduling/v1alpha1" - workloadv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/workload/v1alpha1" kcpclientset "github.com/kcp-dev/kcp/pkg/client/clientset/versioned/cluster" - apisv1alpha1informers "github.com/kcp-dev/kcp/pkg/client/informers/externalversions/apis/v1alpha1" schedulingv1alpha1informers "github.com/kcp-dev/kcp/pkg/client/informers/externalversions/scheduling/v1alpha1" workloadv1alpha1informers "github.com/kcp-dev/kcp/pkg/client/informers/externalversions/workload/v1alpha1" - apisv1alpha1listers "github.com/kcp-dev/kcp/pkg/client/listers/apis/v1alpha1" schedulingv1alpha1listers "github.com/kcp-dev/kcp/pkg/client/listers/scheduling/v1alpha1" workloadv1alpha1listers "github.com/kcp-dev/kcp/pkg/client/listers/workload/v1alpha1" "github.com/kcp-dev/kcp/pkg/logging" @@ -56,8 +53,6 @@ const ( func NewController( kcpClusterClient kcpclientset.ClusterInterface, syncTargetInformer workloadv1alpha1informers.SyncTargetClusterInformer, - apiExportInformer apisv1alpha1informers.APIExportClusterInformer, - apiBindingInformer apisv1alpha1informers.APIBindingClusterInformer, locationInformer schedulingv1alpha1informers.LocationClusterInformer, ) (*controller, error) { queue := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), ControllerName) @@ -74,31 +69,10 @@ func NewController( }, kcpClusterClient: kcpClusterClient, - apiBindingLister: apiBindingInformer.Lister(), - apiExportsLister: apiExportInformer.Lister(), syncTargetLister: syncTargetInformer.Lister(), locationLister: locationInformer.Lister(), } - apiExportInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ - FilterFunc: func(obj interface{}) bool { - switch t := obj.(type) { - case *apisv1alpha1.APIExport: - return t.Name == workloadv1alpha1.ImportedAPISExportName - } - return false - }, - Handler: cache.ResourceEventHandlerFuncs{ - DeleteFunc: func(obj interface{}) { c.enqueue(obj) }, - }, - }) - - apiBindingInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { c.enqueue(obj) }, - UpdateFunc: func(_, obj interface{}) { c.enqueue(obj) }, - DeleteFunc: func(obj interface{}) { c.enqueue(obj) }, - }) - syncTargetInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { c.enqueue(obj) }, DeleteFunc: func(obj interface{}) { c.enqueue(obj) }, @@ -129,8 +103,6 @@ type controller struct { kcpClusterClient kcpclientset.ClusterInterface syncTargetLister workloadv1alpha1listers.SyncTargetClusterLister - apiExportsLister apisv1alpha1listers.APIExportClusterLister - apiBindingLister apisv1alpha1listers.APIBindingClusterLister locationLister schedulingv1alpha1listers.LocationClusterLister } @@ -245,60 +217,5 @@ func (c *controller) process(ctx context.Context, key string) error { } } - // check that export exists, and create APIBinding if it is not. - _, err = c.apiExportsLister.Cluster(clusterName).Get(workloadv1alpha1.ImportedAPISExportName) - if apierrors.IsNotFound(err) { - return nil - } - if err != nil { - return err - } - - // check that binding exists, and create it if not - bindings, err := c.apiBindingLister.Cluster(clusterName).List(labels.Everything()) - if err != nil { - logger.Error(err, "failed to list APIBindings") - return err - } - for _, binding := range bindings { - if binding.Spec.Reference.Export == nil { - continue - } - if binding.Spec.Reference.Export.Path != "" { - continue - } - if binding.Spec.Reference.Export.Name != workloadv1alpha1.ImportedAPISExportName { - continue - } - logging.WithObject(logger, binding).V(3).Info("APIBinding found pointing to APIExport") - return nil // binding found - } - - // bind to local export - binding := &apisv1alpha1.APIBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: workloadv1alpha1.ImportedAPISExportName, - Annotations: map[string]string{ - logicalcluster.AnnotationKey: clusterName.String(), - workloadv1alpha1.ComputeAPIExportAnnotationKey: "true", - }, - }, - Spec: apisv1alpha1.APIBindingSpec{ - Reference: apisv1alpha1.BindingReference{ - Export: &apisv1alpha1.ExportBindingReference{ - Name: workloadv1alpha1.ImportedAPISExportName, - }, - }, - }, - } - logger = logging.WithObject(logger, binding) - logger.V(2).Info("creating APIBinding") - _, err = c.kcpClusterClient.Cluster(clusterName.Path()).ApisV1alpha1().APIBindings().Create(ctx, binding, metav1.CreateOptions{}) - - if err != nil && !apierrors.IsAlreadyExists(err) { - logger.Error(err, "failed to create APIBinding") - return err - } - return nil } diff --git a/test/e2e/syncer/syncer_test.go b/test/e2e/syncer/syncer_test.go index f277a1bb8751..2e776b167d0e 100644 --- a/test/e2e/syncer/syncer_test.go +++ b/test/e2e/syncer/syncer_test.go @@ -44,7 +44,6 @@ import ( "k8s.io/client-go/rest" "k8s.io/utils/pointer" - apisv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/apis/v1alpha1" workloadv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/workload/v1alpha1" kcpclientset "github.com/kcp-dev/kcp/pkg/client/clientset/versioned/cluster" "github.com/kcp-dev/kcp/pkg/syncer/shared" @@ -93,17 +92,17 @@ func TestSyncerLifecycle(t *testing.T) { kcpClient, err := kcpclientset.NewForConfig(upstreamServer.BaseConfig(t)) require.NoError(t, err) - t.Logf("Waiting for imported-apis APIBinding to be ready...") + t.Logf("Waiting for negotiaged api to be generated...") require.Eventually(t, func() bool { - binding, err := kcpClient.Cluster(wsPath).ApisV1alpha1().APIBindings().Get(ctx, workloadv1alpha1.ImportedAPISExportName, metav1.GetOptions{}) + negotiatedAPIs, err := kcpClient.Cluster(wsPath).ApiresourceV1alpha1().NegotiatedAPIResources().List(ctx, metav1.ListOptions{}) if err != nil { return false } - return binding.Status.Phase == apisv1alpha1.APIBindingPhaseBound - }, wait.ForeverTestTimeout, time.Millisecond*100, "imprted-apis APIBinding is not bound") + return len(negotiatedAPIs.Items) > 0 + }, wait.ForeverTestTimeout, time.Millisecond*100, "negotiaged apis are not generated") t.Logf("Bind location workspace") - framework.NewBindCompute(t, wsPath, upstreamServer).Bind(t) + framework.NewBindCompute(t, wsPath, upstreamServer, framework.WithAPIExportsWorkloadBindOption("root:compute:kubernetes", workloadv1alpha1.ImportedAPISExportName)).Bind(t) upstreamConfig := upstreamServer.BaseConfig(t) upstreamKubeClusterClient, err := kcpkubernetesclientset.NewForConfig(upstreamConfig) diff --git a/tmc/pkg/server/controllers.go b/tmc/pkg/server/controllers.go index 25030b71fa9b..86356b0037fb 100644 --- a/tmc/pkg/server/controllers.go +++ b/tmc/pkg/server/controllers.go @@ -319,8 +319,6 @@ func (s *Server) installWorkloadsDefaultLocationController(ctx context.Context, c, err := workloadsdefaultlocation.NewController( kcpClusterClient, s.Core.KcpSharedInformerFactory.Workload().V1alpha1().SyncTargets(), - s.Core.KcpSharedInformerFactory.Apis().V1alpha1().APIExports(), - s.Core.KcpSharedInformerFactory.Apis().V1alpha1().APIBindings(), s.Core.KcpSharedInformerFactory.Scheduling().V1alpha1().Locations(), ) if err != nil {