From 6584baa5abd06426d6732ecb56558a36da0efb79 Mon Sep 17 00:00:00 2001 From: liuwei Date: Thu, 30 Apr 2020 14:37:59 +0800 Subject: [PATCH] rewrite spoke cluster creating process --- pkg/spoke/spokeagent.go | 32 ++++++----- pkg/spoke/spokecluster/creating_controller.go | 53 +++++++++---------- .../spokecluster/creating_controller_test.go | 8 +-- 3 files changed, 45 insertions(+), 48 deletions(-) diff --git a/pkg/spoke/spokeagent.go b/pkg/spoke/spokeagent.go index 539031e7e..7786902bb 100644 --- a/pkg/spoke/spokeagent.go +++ b/pkg/spoke/spokeagent.go @@ -101,7 +101,6 @@ func (o *SpokeAgentOptions) RunSpokeAgent(ctx context.Context, controllerContext // exists a valid client config for hub or not, the controller will be started and then stopped immediately // in scenario #2 and #3, which results in an error message in log: 'Observed a panic: timeout waiting for // informer cache' - var stopBootstrap context.CancelFunc if !ok { // create bootstrap client and shared informer factory from bootstrap hub kube config bootstrapClientConfig, err := clientcmd.BuildConfigFromFlags("", o.BootstrapKubeconfig) @@ -133,6 +132,9 @@ func (o *SpokeAgentOptions) RunSpokeAgent(ctx context.Context, controllerContext // create a SpokeClusterCreatingControlle to create a spoke cluster on hub cluster hasSpokeCluster := false + setHasSpokeClusterFn := func(isSpokeClusterExists bool) { + hasSpokeCluster = isSpokeClusterExists + } caBundle := controllerContext.KubeConfig.CAData if caBundle == nil && controllerContext.KubeConfig.CAFile != "" { @@ -150,12 +152,11 @@ func (o *SpokeAgentOptions) RunSpokeAgent(ctx context.Context, controllerContext o.ClusterName, o.SpokeExternalServerUrl, caBundle, bootstrapClusterClient, - func(isCreated bool) { hasSpokeCluster = isCreated }, + setHasSpokeClusterFn, controllerContext.EventRecorder, ) - var bootstrapCtx context.Context - bootstrapCtx, stopBootstrap = context.WithCancel(ctx) + bootstrapCtx, stopBootstrap := context.WithCancel(ctx) go bootstrapInformerFactory.Start(bootstrapCtx.Done()) go spokeKubeInformerFactory.Start(bootstrapCtx.Done()) @@ -163,22 +164,19 @@ func (o *SpokeAgentOptions) RunSpokeAgent(ctx context.Context, controllerContext go clientCertForHubController.Run(bootstrapCtx, 1) go spokeClusterCreatingController.Run(bootstrapCtx, 1) - // wait for the spoke cluster is created - wait.PollImmediateInfinite(1*time.Second, func() (bool, error) { return hasSpokeCluster, nil }) - } - - // wait for the client config for hub is ready - klog.Info("Waiting for client config for hub to be ready") - if err := wait.PollImmediateInfinite(1*time.Second, o.hasValidHubClientConfig); err != nil { - // TODO we need run the bootstrap CSR forever too to re-establish the client-cert if we ever lose it. - if stopBootstrap != nil { + // wait for the hub client config is ready and the spoke cluster is created. + klog.Info("Waiting for hub client config and spoke cluster to be ready") + if err := wait.PollImmediateInfinite(1*time.Second, func() (bool, error) { + hasValidHubClientConfig, err := o.hasValidHubClientConfig() + return hasValidHubClientConfig && hasSpokeCluster, err + }); err != nil { + // TODO need run the bootstrap CSR forever to re-establish the client-cert if it is ever lost. stopBootstrap() + return err } - return err - } - // stop the ClientCertForHubController for bootstrap once the client config for hub is ready - if stopBootstrap != nil { + // stop the clientCertForHubController and spokeClusterCreatingController for bootstrap + // once the hub client config and spoke cluster is ready stopBootstrap() } diff --git a/pkg/spoke/spokecluster/creating_controller.go b/pkg/spoke/spokecluster/creating_controller.go index 112b2ea43..b7a315972 100644 --- a/pkg/spoke/spokecluster/creating_controller.go +++ b/pkg/spoke/spokecluster/creating_controller.go @@ -13,13 +13,14 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +type setHasSpokeClusterFunc func(bool) + // spokeClusterCreatingController creates a spoke cluster on hub cluster during the spoke agent bootstrap phase type spokeClusterCreatingController struct { - isSpokeClusterExists bool clusterName string spokeExternalServerUrl string spokeCABundle []byte - hasSpokeClusterFunc func(bool) + setHasSpokeCluster setHasSpokeClusterFunc hubClusterClient clientset.Interface } @@ -28,13 +29,13 @@ func NewSpokeClusterCreatingController( clusterName, spokeExternalServerUrl string, spokeCABundle []byte, hubClusterClient clientset.Interface, - hasSpokeClusterFunc func(bool), + setHasSpokeClusterFn setHasSpokeClusterFunc, recorder events.Recorder) factory.Controller { c := &spokeClusterCreatingController{ clusterName: clusterName, spokeExternalServerUrl: spokeExternalServerUrl, spokeCABundle: spokeCABundle, - hasSpokeClusterFunc: hasSpokeClusterFunc, + setHasSpokeCluster: setHasSpokeClusterFn, hubClusterClient: hubClusterClient, } return factory.New(). @@ -44,34 +45,32 @@ func NewSpokeClusterCreatingController( } func (c *spokeClusterCreatingController) sync(ctx context.Context, syncCtx factory.SyncContext) error { - if c.isSpokeClusterExists { - return nil - } - - spokeCluster := &clusterv1.SpokeCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: c.clusterName, - }, - Spec: clusterv1.SpokeClusterSpec{ - SpokeClientConfig: clusterv1.ClientConfig{ - URL: c.spokeExternalServerUrl, - CABundle: c.spokeCABundle, + _, err := c.hubClusterClient.ClusterV1().SpokeClusters().Get(ctx, c.clusterName, metav1.GetOptions{}) + if errors.IsNotFound(err) { + spokeCluster := &clusterv1.SpokeCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: c.clusterName, }, - }, - } - - _, err := c.hubClusterClient.ClusterV1().SpokeClusters().Create(ctx, spokeCluster, metav1.CreateOptions{}) - if errors.IsAlreadyExists(err) { - c.isSpokeClusterExists = true - c.hasSpokeClusterFunc(c.isSpokeClusterExists) + Spec: clusterv1.SpokeClusterSpec{ + SpokeClientConfig: clusterv1.ClientConfig{ + URL: c.spokeExternalServerUrl, + CABundle: c.spokeCABundle, + }, + }, + } + _, err := c.hubClusterClient.ClusterV1().SpokeClusters().Create(ctx, spokeCluster, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("unable to create spoke cluster with name %q on hub: %w", c.clusterName, err) + } + c.setHasSpokeCluster(true) + syncCtx.Recorder().Eventf("SpokeClusterCreated", "Spoke cluster %q created on hub", c.clusterName) return nil } + if err != nil { - return fmt.Errorf("unable to create spoke cluster with name %q on hub: %w", c.clusterName, err) + return err } - c.isSpokeClusterExists = true - c.hasSpokeClusterFunc(c.isSpokeClusterExists) - syncCtx.Recorder().Eventf("SpokeClusterCreated", "Spoke cluster %q created on hub", c.clusterName) + c.setHasSpokeCluster(true) return nil } diff --git a/pkg/spoke/spokecluster/creating_controller_test.go b/pkg/spoke/spokecluster/creating_controller_test.go index abfa70dfa..335bf0c91 100644 --- a/pkg/spoke/spokecluster/creating_controller_test.go +++ b/pkg/spoke/spokecluster/creating_controller_test.go @@ -26,8 +26,8 @@ func TestCreateSpokeCluster(t *testing.T) { startingObjects: []runtime.Object{}, hasSpokeCluster: true, validateActions: func(t *testing.T, actions []clienttesting.Action) { - assertActions(t, actions, "create") - actual := actions[0].(clienttesting.CreateActionImpl).Object + assertActions(t, actions, "get", "create") + actual := actions[1].(clienttesting.CreateActionImpl).Object assertSpokeExternalServerUrl(t, actual, testSpokeExternalServerUrl) assertSpokeCABundle(t, actual, []byte("testcabundle")) }, @@ -37,7 +37,7 @@ func TestCreateSpokeCluster(t *testing.T) { startingObjects: []runtime.Object{newSpokeCluster([]clusterv1.StatusCondition{})}, hasSpokeCluster: true, validateActions: func(t *testing.T, actions []clienttesting.Action) { - assertActions(t, actions, "create") + assertActions(t, actions, "get") }, }, } @@ -50,7 +50,7 @@ func TestCreateSpokeCluster(t *testing.T) { clusterName: testSpokeClusterName, spokeExternalServerUrl: testSpokeExternalServerUrl, spokeCABundle: []byte("testcabundle"), - hasSpokeClusterFunc: func(isCreated bool) { hasSpokeCluster = isCreated }, + setHasSpokeCluster: func(isCreated bool) { hasSpokeCluster = isCreated }, hubClusterClient: clusterClient, }