Merge pull request #1032 from tu1h/optimize_log

Enhancement log print
kubean-io · Nov 22, 2023 · 485020c · 485020c
2 parents 79e42d7 + 33292d0
commit 485020c
Show file tree

Hide file tree

Showing 3 changed files with 133 additions and 17 deletions.
diff --git a/pkg/controllers/infomanifest/controller.go b/pkg/controllers/infomanifest/controller.go
@@ -45,16 +45,14 @@ func (c *Controller) Start(ctx context.Context) error {
 }
 
 func (c *Controller) FetchLocalServiceCM(namespace string) (*corev1.ConfigMap, error) {
-	if localServiceCM, _ := c.ClientSet.CoreV1().ConfigMaps(namespace).Get(context.Background(), LocalServiceConfigMap, metav1.GetOptions{}); localServiceCM != nil {
-		return localServiceCM, nil
+	localServiceCM, err := c.ClientSet.CoreV1().ConfigMaps(namespace).Get(context.Background(), LocalServiceConfigMap, metav1.GetOptions{})
+	if err != nil && apierrors.IsNotFound(err) && namespace != "default" {
+		localServiceCM, err = c.ClientSet.CoreV1().ConfigMaps("default").Get(context.Background(), LocalServiceConfigMap, metav1.GetOptions{})
 	}
-	if namespace != "default" {
-		namespace = "default"
-		if localServiceCM, _ := c.ClientSet.CoreV1().ConfigMaps(namespace).Get(context.Background(), LocalServiceConfigMap, metav1.GetOptions{}); localServiceCM != nil {
-			return localServiceCM, nil
-		}
+	if err != nil {
+		return nil, err
 	}
-	return nil, fmt.Errorf("not found kubean localService ConfigMap")
+	return localServiceCM, nil
 }
 
 func (c *Controller) ParseConfigMapToLocalService(localServiceConfigMap *corev1.ConfigMap) (*manifestv1alpha1.LocalService, error) {
@@ -75,23 +73,21 @@ func (c *Controller) ParseConfigMapToLocalService(localServiceConfigMap *corev1.
 // UpdateLocalService sync the content of local-service configmap into spec.
 func (c *Controller) UpdateLocalService(manifests []manifestv1alpha1.Manifest) bool {
 	if c.IsOnlineENV() {
-		// if not airgap env , then do nothing and return
+		// if not airgap environment, do nothing and return
 		return false
 	}
 	localServiceConfigMap, err := c.FetchLocalServiceCM(util.GetCurrentNSOrDefault())
 	if err != nil {
-		klog.Warningf("ignoring error: %s", err.Error())
+		if !apierrors.IsNotFound(err) {
+			klog.Warningf("ignoring error: %s", err.Error())
+		}
 		return false
 	}
 	localService, err := c.ParseConfigMapToLocalService(localServiceConfigMap)
 	if err != nil {
 		klog.Warningf("ignoring error: %s", err.Error())
 		return false
 	}
-	if err != nil {
-		klog.Warningf("ignoring error: %s", err.Error())
-		return false
-	}
 	for _, manifest := range manifests {
 		if !reflect.DeepEqual(&manifest.Spec.LocalService, localService) {
 			manifest.Spec.LocalService = *localService

diff --git a/pkg/controllers/infomanifest/controller_test.go b/pkg/controllers/infomanifest/controller_test.go
@@ -426,6 +426,127 @@ func TestFetchLocalServiceCM(t *testing.T) {
 	}
 }
 
+func TestUpdateLocalService(t *testing.T) {
+	controller := &Controller{
+		Client:                    newFakeClient(),
+		ClientSet:                 clientsetfake.NewSimpleClientset(),
+		InfoManifestClientSet:     manifestv1alpha1fake.NewSimpleClientset(),
+		LocalArtifactSetClientSet: localartifactsetv1alpha1fake.NewSimpleClientset(),
+	}
+
+	tests := []struct {
+		name        string
+		prequisites func() error
+		cleaner     func()
+		args        []manifestv1alpha1.Manifest
+		want        bool
+	}{
+		{
+			name: "air-gap, but no localservice configmap",
+			prequisites: func() error {
+				_, err := controller.LocalArtifactSetClientSet.KubeanV1alpha1().LocalArtifactSets().Create(context.Background(), &localartifactsetv1alpha1.LocalArtifactSet{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "localartifactset-1",
+					},
+				}, metav1.CreateOptions{})
+				if err != nil {
+					return err
+				}
+				return nil
+			},
+			cleaner: func() {
+				controller.LocalArtifactSetClientSet.KubeanV1alpha1().LocalArtifactSets().Delete(context.Background(), "localartifactset-1", metav1.DeleteOptions{})
+			},
+			args: []manifestv1alpha1.Manifest{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "manifest-1",
+					},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "air-gap, localservice configmap exists and correct data structure",
+			prequisites: func() error {
+				if _, err := controller.LocalArtifactSetClientSet.KubeanV1alpha1().LocalArtifactSets().Create(context.Background(), &localartifactsetv1alpha1.LocalArtifactSet{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "localartifactset-2",
+					},
+				}, metav1.CreateOptions{}); err != nil {
+					return err
+				}
+				if _, err := controller.ClientSet.CoreV1().ConfigMaps("default").Create(context.Background(), &corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: LocalServiceConfigMap,
+					},
+					Data: map[string]string{"localService": "imageRepoScheme: 'https'"},
+				}, metav1.CreateOptions{}); err != nil {
+					return err
+				}
+				return nil
+			},
+			cleaner: func() {
+				controller.LocalArtifactSetClientSet.KubeanV1alpha1().LocalArtifactSets().Delete(context.Background(), "localartifactset-2", metav1.DeleteOptions{})
+				controller.ClientSet.CoreV1().ConfigMaps("default").Delete(context.Background(), LocalServiceConfigMap, metav1.DeleteOptions{})
+			},
+			args: []manifestv1alpha1.Manifest{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "manifest-2",
+					},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "air-gap, localservice configmap exists and but incorrect data structure",
+			prequisites: func() error {
+				if _, err := controller.LocalArtifactSetClientSet.KubeanV1alpha1().LocalArtifactSets().Create(context.Background(), &localartifactsetv1alpha1.LocalArtifactSet{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "localartifactset-3",
+					},
+				}, metav1.CreateOptions{}); err != nil {
+					return err
+				}
+				if _, err := controller.ClientSet.CoreV1().ConfigMaps("default").Create(context.Background(), &corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: LocalServiceConfigMap,
+					},
+					Data: map[string]string{"localService": "test"},
+				}, metav1.CreateOptions{}); err != nil {
+					return err
+				}
+				return nil
+			},
+			cleaner: func() {
+				controller.LocalArtifactSetClientSet.KubeanV1alpha1().LocalArtifactSets().Delete(context.Background(), "localartifactset-3", metav1.DeleteOptions{})
+				controller.ClientSet.CoreV1().ConfigMaps("default").Delete(context.Background(), LocalServiceConfigMap, metav1.DeleteOptions{})
+			},
+			args: []manifestv1alpha1.Manifest{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "manifest-3",
+					},
+				},
+			},
+			want: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if err := test.prequisites(); err != nil {
+				t.Fatal(err)
+			}
+			defer test.cleaner()
+			if controller.UpdateLocalService(test.args) != test.want {
+				t.Fatal()
+			}
+		})
+	}
+}
+
 func TestReconcile(t *testing.T) {
 	controller := &Controller{
 		Client:                    newFakeClient(),

diff --git a/pkg/controllers/offlineversion/controller.go b/pkg/controllers/offlineversion/controller.go
@@ -74,8 +74,7 @@ func (c *Controller) MergeManifestsStatus(localartifactset *localartifactsetv1al
 		}
 		klog.Infof("Update manifest status for %s since %s", manifest.Name, localartifactset.Name)
 		if _, err := c.InfoManifestClientSet.KubeanV1alpha1().Manifests().UpdateStatus(context.Background(), manifest, metav1.UpdateOptions{}); err != nil {
-			klog.Error(err)
-			return nil, err
+			return nil, fmt.Errorf("failed to merge status for manifest %s, %v", manifest.Name, err)
 		}
 	}
 	return manifests, nil
@@ -93,7 +92,7 @@ func (c *Controller) Reconcile(ctx context.Context, req controllerruntime.Reques
 
 	sprayRelease, ok := localartifactset.ObjectMeta.Labels[constants.KeySprayRelease]
 	if !ok {
-		klog.Infof("No label %s found in %s", constants.KeySprayRelease, localartifactset.Name)
+		klog.Warningf("No label %s found in %s", constants.KeySprayRelease, localartifactset.Name)
 		return controllerruntime.Result{}, nil
 	}