Added utils to update configmap

- Added utils to update configmp - Chores to refine vars or logs
knative · Jun 7, 2020 · b868bf5 · b868bf5
1 parent f207c1f
commit b868bf5
Show file tree

Hide file tree

Showing 15 changed files with 841 additions and 58 deletions.
diff --git a/plugins/admin/README.adoc b/plugins/admin/README.adoc
@@ -120,15 +120,15 @@ Use "kn admin autoscaling [command] --help" for more information about a comman
 ====
 ----
 $ kn admin domain set --custom-domain mydomain.com
-Updated Knative route domain mydomain.com
+Set Knative route domain mydomain.com
 ----
 ====
 
 .Update a custom domain with --selector and Service with a label app=v1 will use test.com
 ====
 ----
 $ kn admin domain set --custom-domain test.com --selector app=v1
-Updated Knative route domain test.com
+Set Knative route domain test.com with selector [app=v1]
 ----
 ====
 

diff --git a/plugins/admin/pkg/command/autoscaling/update.go b/plugins/admin/pkg/command/autoscaling/update.go
@@ -17,11 +17,13 @@ package autoscaling
 import (
 	"errors"
 	"fmt"
+
+	"knative.dev/client-contrib/plugins/admin/pkg/command/utils"
+
 	"knative.dev/client-contrib/plugins/admin/pkg"
 
 	"github.com/spf13/cobra"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/oidc"
 
@@ -38,8 +40,8 @@ var (
 func NewAutoscalingUpdateCommand(p *pkg.AdminParams) *cobra.Command {
 	AutoscalingUpdateCommand := &cobra.Command{
 		Use:   "update",
-		Short: "update autoscaling config",
-		Long: `update autoscaling config provided by Knative Pod Autoscaler (KPA)
+		Short: "Update autoscaling config",
+		Long: `Update autoscaling config provided by Knative Pod Autoscaler (KPA)
 
 For example:
 # To enable scale-to-zero
@@ -60,22 +62,21 @@ kn admin autoscaling update --no-scale-to-zero
 			} else if cmd.Flags().Changed("no-scale-to-zero") {
 				scaleToZero = "false"
 			}
+
 			currentCm := &corev1.ConfigMap{}
 			currentCm, err := p.ClientSet.CoreV1().ConfigMaps(knativeServing).Get(configAutoscaler, metav1.GetOptions{})
 			if err != nil {
-				return fmt.Errorf("Failed to get ConfigMaps: %+v", err)
+				return fmt.Errorf("failed to get ConfigMaps: %+v", err)
 			}
 			desiredCm := currentCm.DeepCopy()
 			desiredCm.Data[enableScaleToZero] = scaleToZero
-			if !equality.Semantic.DeepEqual(desiredCm.Data[enableScaleToZero], currentCm.Data[enableScaleToZero]) {
-				_, err = p.ClientSet.CoreV1().ConfigMaps(knativeServing).Update(desiredCm)
-				if err != nil {
-					return fmt.Errorf("Failed to update ConfigMaps: %+v", err)
-				}
-				cmd.Printf("Updated Knative autoscaling config %s: %s\n", enableScaleToZero, scaleToZero)
-			} else {
-				cmd.Printf("Knative autoscaling config %s: %s not changed\n", enableScaleToZero, currentCm.Data[enableScaleToZero])
+
+			err = utils.UpdateConfigMap(p.ClientSet, desiredCm)
+			if err != nil {
+				return fmt.Errorf("failed to update ConfigMap %s in namespace %s: %+v", configAutoscaler, knativeServing, err)
 			}
+			cmd.Printf("Updated Knative autoscaling config %s: %s\n", enableScaleToZero, scaleToZero)
+
 			return nil
 		},
 	}

diff --git a/plugins/admin/pkg/command/autoscaling/update_test.go b/plugins/admin/pkg/command/autoscaling/update_test.go
@@ -53,7 +53,7 @@ func TestNewAsUpdateSetCommand(t *testing.T) {
 		}
 		cmd := NewAutoscalingUpdateCommand(&p)
 		_, err := testutil.ExecuteCommand(cmd, "--scale-to-zero")
-		assert.ErrorContains(t, err, "Failed to get ConfigMaps", err)
+		assert.ErrorContains(t, err, "failed to get ConfigMaps", err)
 	})
 
 	t.Run("enable scale-to-zero successfully", func(t *testing.T) {
@@ -131,4 +131,3 @@ func TestNewAsUpdateSetCommand(t *testing.T) {
 
 	})
 }
-
diff --git a/plugins/admin/pkg/command/domain/set.go b/plugins/admin/pkg/command/domain/set.go
@@ -19,22 +19,28 @@ import (
 	"fmt"
 	"strings"
 
+	"knative.dev/client-contrib/plugins/admin/pkg/command/utils"
+
 	"knative.dev/client-contrib/plugins/admin/pkg"
 
 	"github.com/spf13/cobra"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-var selector []string
-var domain string
+var (
+	ScaleToZero    bool
+	selector       []string
+	domain         string
+	knativeServing = "knative-serving"
+	configDomain   = "config-domain"
+)
 
 // NewDomainSetCommand return the command to set knative custom domain
 func NewDomainSetCommand(p *pkg.AdminParams) *cobra.Command {
 	domainSetCommand := &cobra.Command{
 		Use:   "set",
-		Short: "set route domain",
+		Short: "Set route domain",
 		Long: `Set Knative route domain for service
 
 For example:
@@ -52,9 +58,9 @@ kn admin domain set --custom-domain mydomain.com --selector app=v1
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			currentCm := &corev1.ConfigMap{}
-			currentCm, err := p.ClientSet.CoreV1().ConfigMaps("knative-serving").Get("config-domain", metav1.GetOptions{})
+			currentCm, err := p.ClientSet.CoreV1().ConfigMaps(knativeServing).Get(configDomain, metav1.GetOptions{})
 			if err != nil {
-				return fmt.Errorf("failed to get configmaps: %+v", err)
+				return fmt.Errorf("failed to get ConfigMap %s in namespace %s: %+v", configDomain, knativeServing, err)
 			}
 			desiredCm := currentCm.DeepCopy()
 			labels := "selector:\n"
@@ -79,14 +85,16 @@ kn admin domain set --custom-domain mydomain.com --selector app=v1
 			}
 
 			desiredCm.Data[domain] = value
-			if !equality.Semantic.DeepEqual(desiredCm.Data, currentCm.Data) {
-				_, err = p.ClientSet.CoreV1().ConfigMaps("knative-serving").Update(desiredCm)
-				if err != nil {
-					return fmt.Errorf("Failed to update ConfigMaps: %+v", err)
-				}
-				cmd.Printf("Updated knative route domain to %q\n", domain)
+
+			err = utils.UpdateConfigMap(p.ClientSet, desiredCm)
+			if err != nil {
+				return fmt.Errorf("failed to update ConfigMap %s in namespace %s: %+v", configDomain, knativeServing, err)
+			}
+
+			if value == "" {
+				cmd.Printf("Set knative route domain %q\n", domain)
 			} else {
-				cmd.Printf("Knative route domain %q not changed\n", domain)
+				cmd.Printf("Set knative route domain %q with selector %+v\n", domain, selector)
 			}
 			return nil
 		},

diff --git a/plugins/admin/pkg/command/domain/set_test.go b/plugins/admin/pkg/command/domain/set_test.go
@@ -49,8 +49,8 @@ func TestNewDomainSetCommand(t *testing.T) {
 	t.Run("incompleted args", func(t *testing.T) {
 		cm := &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "config-domain",
-				Namespace: "knative-serving",
+				Name:      configDomain,
+				Namespace: knativeServing,
 			},
 			Data: make(map[string]string),
 		}
@@ -71,14 +71,14 @@ func TestNewDomainSetCommand(t *testing.T) {
 		}
 		cmd := NewDomainSetCommand(&p)
 		_, err := testutil.ExecuteCommand(cmd, "--custom-domain", "dummy.domain")
-		assert.ErrorContains(t, err, "failed to get configmaps", err)
+		assert.ErrorContains(t, err, "failed to get ConfigMap", err)
 	})
 
 	t.Run("setting domain config without selector", func(t *testing.T) {
 		cm := &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "config-domain",
-				Namespace: "knative-serving",
+				Name:      configDomain,
+				Namespace: knativeServing,
 			},
 			Data: make(map[string]string),
 		}
@@ -90,7 +90,7 @@ func TestNewDomainSetCommand(t *testing.T) {
 		_, err := testutil.ExecuteCommand(cmd, "--custom-domain", "dummy.domain")
 		assert.NilError(t, err)
 
-		cm, err = client.CoreV1().ConfigMaps("knative-serving").Get("config-domain", metav1.GetOptions{})
+		cm, err = client.CoreV1().ConfigMaps(knativeServing).Get(configDomain, metav1.GetOptions{})
 		assert.NilError(t, err)
 		assert.Check(t, len(cm.Data) == 1, "expected configmap lengh to be 1")
 
@@ -102,8 +102,8 @@ func TestNewDomainSetCommand(t *testing.T) {
 	t.Run("setting domain config with unchanged value", func(t *testing.T) {
 		cm := &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "config-domain",
-				Namespace: "knative-serving",
+				Name:      configDomain,
+				Namespace: knativeServing,
 			},
 			Data: map[string]string{
 				"dummy.domain": "",
@@ -118,7 +118,7 @@ func TestNewDomainSetCommand(t *testing.T) {
 		_, err := testutil.ExecuteCommand(cmd, "--custom-domain", "dummy.domain")
 		assert.NilError(t, err)
 
-		updated, err := client.CoreV1().ConfigMaps("knative-serving").Get("config-domain", metav1.GetOptions{})
+		updated, err := client.CoreV1().ConfigMaps(knativeServing).Get(configDomain, metav1.GetOptions{})
 		assert.NilError(t, err)
 		assert.Check(t, equality.Semantic.DeepEqual(updated, cm), "configmap should not changed")
 
@@ -127,8 +127,8 @@ func TestNewDomainSetCommand(t *testing.T) {
 	t.Run("adding domain config without selector with existing domain configuration", func(t *testing.T) {
 		cm := &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "config-domain",
-				Namespace: "knative-serving",
+				Name:      configDomain,
+				Namespace: knativeServing,
 			},
 			Data: map[string]string{
 				"foo.bar": "",
@@ -141,9 +141,9 @@ func TestNewDomainSetCommand(t *testing.T) {
 		cmd := NewDomainSetCommand(&p)
 		o, err := testutil.ExecuteCommand(cmd, "--custom-domain", "dummy.domain")
 		assert.NilError(t, err)
-		assert.Check(t, strings.Contains(o, "Updated knative route domain to"), "expected update information in standard output")
+		assert.Check(t, strings.Contains(o, "Set knative route domain \"dummy.domain\""), "expected update information in standard output")
 
-		cm, err = client.CoreV1().ConfigMaps("knative-serving").Get("config-domain", metav1.GetOptions{})
+		cm, err = client.CoreV1().ConfigMaps(knativeServing).Get(configDomain, metav1.GetOptions{})
 		assert.NilError(t, err)
 		assert.Check(t, len(cm.Data) == 1, "expected configmap lengh to be 1, actual %d", len(cm.Data))
 
@@ -155,8 +155,8 @@ func TestNewDomainSetCommand(t *testing.T) {
 	t.Run("adding domain config with selector", func(t *testing.T) {
 		cm := &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "config-domain",
-				Namespace: "knative-serving",
+				Name:      configDomain,
+				Namespace: knativeServing,
 			},
 			Data: map[string]string{
 				"foo.bar": "",
@@ -170,9 +170,9 @@ func TestNewDomainSetCommand(t *testing.T) {
 
 		o, err := testutil.ExecuteCommand(cmd, "--custom-domain", "dummy.domain", "--selector", "app=dummy")
 		assert.NilError(t, err)
-		assert.Check(t, strings.Contains(o, "Updated knative route domain to"), "invalid output %q", o)
+		assert.Check(t, strings.Contains(o, "Set knative route domain \"dummy.domain\" with selector [app=dummy]"), "invalid output %q", o)
 
-		cm, err = client.CoreV1().ConfigMaps("knative-serving").Get("config-domain", metav1.GetOptions{})
+		cm, err = client.CoreV1().ConfigMaps(knativeServing).Get(configDomain, metav1.GetOptions{})
 		assert.NilError(t, err)
 		assert.Check(t, len(cm.Data) == 2, "expected configmap lengh to be 2, actual %d", len(cm.Data))
 

diff --git a/plugins/admin/pkg/command/domain/unset.go b/plugins/admin/pkg/command/domain/unset.go
@@ -18,6 +18,8 @@ import (
 	"errors"
 	"fmt"
 
+	"knative.dev/client-contrib/plugins/admin/pkg/command/utils"
+
 	"knative.dev/client-contrib/plugins/admin/pkg"
 
 	"github.com/spf13/cobra"
@@ -44,7 +46,7 @@ kn admin domain unset --custom-domain mydomain.com
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			currentCm := &corev1.ConfigMap{}
-			currentCm, err := p.ClientSet.CoreV1().ConfigMaps("knative-serving").Get("config-domain", metav1.GetOptions{})
+			currentCm, err := p.ClientSet.CoreV1().ConfigMaps(knativeServing).Get(configDomain, metav1.GetOptions{})
 			if err != nil {
 				return fmt.Errorf("failed to get configmaps: %+v", err)
 			}
@@ -58,10 +60,11 @@ kn admin domain unset --custom-domain mydomain.com
 				return fmt.Errorf("Knative route domain %s not found\n", domain)
 			}
 
-			_, err = p.ClientSet.CoreV1().ConfigMaps("knative-serving").Update(desiredCm)
+			err = utils.UpdateConfigMap(p.ClientSet, desiredCm)
 			if err != nil {
-				return fmt.Errorf("Failed to update ConfigMaps: %+v", err)
+				return fmt.Errorf("failed to update ConfigMap %s in namespace %s: %+v", configDomain, knativeServing, err)
 			}
+
 			cmd.Printf("Unset Knative route domain %s\n", domain)
 			return nil
 		},

diff --git a/plugins/admin/pkg/command/domain/unset_test.go b/plugins/admin/pkg/command/domain/unset_test.go
@@ -31,8 +31,8 @@ func TestNewDomainUnSetCommand(t *testing.T) {
 	t.Run("incompleted args", func(t *testing.T) {
 		cm := &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "config-domain",
-				Namespace: "knative-serving",
+				Name:      configDomain,
+				Namespace: knativeServing,
 			},
 			Data: make(map[string]string),
 		}
@@ -59,8 +59,8 @@ func TestNewDomainUnSetCommand(t *testing.T) {
 	t.Run("route domain not found", func(t *testing.T) {
 		cm := &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "config-domain",
-				Namespace: "knative-serving",
+				Name:      configDomain,
+				Namespace: knativeServing,
 			},
 			Data: map[string]string{
 				"dummy.domain": "",
@@ -79,8 +79,8 @@ func TestNewDomainUnSetCommand(t *testing.T) {
 	t.Run("unset domain", func(t *testing.T) {
 		cm := &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "config-domain",
-				Namespace: "knative-serving",
+				Name:      configDomain,
+				Namespace: knativeServing,
 			},
 			Data: map[string]string{
 				"dummy1.domain": "",
@@ -95,7 +95,7 @@ func TestNewDomainUnSetCommand(t *testing.T) {
 		_, err := testutil.ExecuteCommand(cmd, "--custom-domain", "dummy1.domain")
 		assert.NilError(t, err)
 
-		cm, err = client.CoreV1().ConfigMaps("knative-serving").Get("config-domain", metav1.GetOptions{})
+		cm, err = client.CoreV1().ConfigMaps(knativeServing).Get(configDomain, metav1.GetOptions{})
 		assert.NilError(t, err)
 		assert.Check(t, len(cm.Data) == 1, "expected configmap lengh to be 1")
 
@@ -108,7 +108,7 @@ func TestNewDomainUnSetCommand(t *testing.T) {
 		_, err = testutil.ExecuteCommand(cmd, "--custom-domain", "dummy2.domain")
 		assert.NilError(t, err)
 
-		cm, err = client.CoreV1().ConfigMaps("knative-serving").Get("config-domain", metav1.GetOptions{})
+		cm, err = client.CoreV1().ConfigMaps(knativeServing).Get(configDomain, metav1.GetOptions{})
 		assert.NilError(t, err)
 		assert.Check(t, len(cm.Data) == 0, "expected configmap lengh to be 0")
 	})

diff --git a/plugins/admin/pkg/command/registry/add.go b/plugins/admin/pkg/command/registry/add.go
@@ -126,7 +126,7 @@ kn admin registry add \
 			if err != nil {
 				return fmt.Errorf("failed to add registry secret in default serviceaccount: %v", err)
 			}
-			cmd.Printf("Private registry %s added for default serviceaccount\n", prflags.Server)
+			cmd.Printf("Private registry %s is added for default serviceaccount\n", prflags.Server)
 			return nil
 		},
 	}

diff --git a/plugins/admin/pkg/command/utils/utils.go b/plugins/admin/pkg/command/utils/utils.go
@@ -0,0 +1,37 @@
+// Copyright © 2020 The Knative Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/util/retry"
+)
+
+func UpdateConfigMap(client kubernetes.Interface, desiredCm *corev1.ConfigMap) error {
+	return retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		currentCm, err := client.CoreV1().ConfigMaps(desiredCm.Namespace).Get(desiredCm.Name, metav1.GetOptions{})
+		if err != nil {
+			return err
+		}
+		if equality.Semantic.DeepEqual(desiredCm, currentCm) {
+			return nil
+		}
+		_, err = client.CoreV1().ConfigMaps(desiredCm.Namespace).Update(desiredCm)
+		return err
+	})
+}