From d6081f91f381eb602e228daeb50d7cdafe941ed8 Mon Sep 17 00:00:00 2001 From: ivanscai Date: Wed, 7 Sep 2022 16:21:34 +0800 Subject: [PATCH] =?UTF-8?q?update=20serverURL=20and=20CABundle=20if=20Mana?= =?UTF-8?q?gedCluster=20is=20created=20before=20reg=E2=80=A6=20(#270)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * update serverURL and CABundle if ManagedCluster is created before registration-agent running Signed-off-by: ivan-cai * merge spokeExternalServerURLs to ManagedClusterClientConfigs, and ingnore Unauthorized error Signed-off-by: ivan-cai * add comment for skipping unauthorized error, and skip unauthorized error when creating ManagedCluster Signed-off-by: ivan-cai * use skipUnauthorizedError at creating controller Signed-off-by: ivan-cai Signed-off-by: ivan-cai --- .../managedcluster/creating_controller.go | 87 ++++++++++++++----- .../creating_controller_test.go | 2 +- 2 files changed, 68 insertions(+), 21 deletions(-) diff --git a/pkg/spoke/managedcluster/creating_controller.go b/pkg/spoke/managedcluster/creating_controller.go index 7762e8dcb..a6d79661f 100644 --- a/pkg/spoke/managedcluster/creating_controller.go +++ b/pkg/spoke/managedcluster/creating_controller.go @@ -53,40 +53,87 @@ func NewManagedClusterCreatingController( } func (c *managedClusterCreatingController) sync(ctx context.Context, syncCtx factory.SyncContext) error { - _, err := c.hubClusterClient.ClusterV1().ManagedClusters().Get(ctx, c.clusterName, metav1.GetOptions{}) - switch { - case errors.IsUnauthorized(err), - errors.IsForbidden(err) && strings.Contains(err.Error(), anonymous): + existingCluster, err := c.hubClusterClient.ClusterV1().ManagedClusters().Get(ctx, c.clusterName, metav1.GetOptions{}) + if err != nil && skipUnauthorizedError(err) == nil && strings.Contains(err.Error(), anonymous) { klog.V(4).Infof("unable to get the managed cluster %q from hub: %v", c.clusterName, err) return nil - case errors.IsNotFound(err): - case err == nil: - return nil - case err != nil: + } + + if err != nil && !errors.IsNotFound(err) { return err } - managedCluster := &clusterv1.ManagedCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: c.clusterName, - }, + // create ManagedCluster if not found + if errors.IsNotFound(err) { + managedCluster := &clusterv1.ManagedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: c.clusterName, + }, + } + + if len(c.spokeExternalServerURLs) != 0 { + var managedClusterClientConfigs []clusterv1.ClientConfig + for _, serverURL := range c.spokeExternalServerURLs { + managedClusterClientConfigs = append(managedClusterClientConfigs, clusterv1.ClientConfig{ + URL: serverURL, + CABundle: c.spokeCABundle, + }) + } + managedCluster.Spec.ManagedClusterClientConfigs = managedClusterClientConfigs + } + + _, err = c.hubClusterClient.ClusterV1().ManagedClusters().Create(ctx, managedCluster, metav1.CreateOptions{}) + // ManagedCluster is only allowed created during bootstrap. After bootstrap secret expired, an unauthorized error will be got, skip it + if skipUnauthorizedError(err) != nil { + return fmt.Errorf("unable to create managed cluster with name %q on hub: %w", c.clusterName, err) + } + syncCtx.Recorder().Eventf("ManagedClusterCreated", "Managed cluster %q created on hub", c.clusterName) + return nil } - if len(c.spokeExternalServerURLs) != 0 { - managedClusterClientConfigs := []clusterv1.ClientConfig{} - for _, serverURL := range c.spokeExternalServerURLs { + // do not update ManagedClusterClientConfigs in ManagedCluster if spokeExternalServerURLs is empty + if len(c.spokeExternalServerURLs) == 0 { + return nil + } + + // merge ClientConfig + managedClusterClientConfigs := existingCluster.Spec.ManagedClusterClientConfigs + for _, serverURL := range c.spokeExternalServerURLs { + isIncludeByExisting := false + for _, existingClientConfig := range existingCluster.Spec.ManagedClusterClientConfigs { + if serverURL == existingClientConfig.URL { + isIncludeByExisting = true + break + } + } + + if !isIncludeByExisting { managedClusterClientConfigs = append(managedClusterClientConfigs, clusterv1.ClientConfig{ URL: serverURL, CABundle: c.spokeCABundle, }) } - managedCluster.Spec.ManagedClusterClientConfigs = managedClusterClientConfigs + } + if len(existingCluster.Spec.ManagedClusterClientConfigs) == len(managedClusterClientConfigs) { + return nil } - _, err = c.hubClusterClient.ClusterV1().ManagedClusters().Create(ctx, managedCluster, metav1.CreateOptions{}) - if err != nil { - return fmt.Errorf("unable to create managed cluster with name %q on hub: %w", c.clusterName, err) + // update ManagedClusterClientConfigs in ManagedCluster + clusterCopy := existingCluster.DeepCopy() + clusterCopy.Spec.ManagedClusterClientConfigs = managedClusterClientConfigs + _, err = c.hubClusterClient.ClusterV1().ManagedClusters().Update(ctx, clusterCopy, metav1.UpdateOptions{}) + // ManagedClusterClientConfigs in ManagedCluster is only allowed updated during bootstrap. After bootstrap secret expired, an unauthorized error will be got, skip it + if skipUnauthorizedError(err) != nil { + return fmt.Errorf("unable to update ManagedClusterClientConfigs of managed cluster %q in hub: %w", c.clusterName, err) } - syncCtx.Recorder().Eventf("ManagedClusterCreated", "Managed cluster %q created on hub", c.clusterName) + return nil } + +func skipUnauthorizedError(err error) error { + if errors.IsUnauthorized(err) || errors.IsForbidden(err) { + return nil + } + + return err +} diff --git a/pkg/spoke/managedcluster/creating_controller_test.go b/pkg/spoke/managedcluster/creating_controller_test.go index 044b72453..5dcf72dd7 100644 --- a/pkg/spoke/managedcluster/creating_controller_test.go +++ b/pkg/spoke/managedcluster/creating_controller_test.go @@ -40,7 +40,7 @@ func TestCreateSpokeCluster(t *testing.T) { name: "create an existed cluster", startingObjects: []runtime.Object{testinghelpers.NewManagedCluster()}, validateActions: func(t *testing.T, actions []clienttesting.Action) { - testinghelpers.AssertActions(t, actions, "get") + testinghelpers.AssertActions(t, actions, "get", "update") }, }, }