diff --git a/pkg/controllers/infomanifest/controller.go b/pkg/controllers/infomanifest/controller.go index b60a8e9b5..2cd0de06f 100644 --- 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,12 +73,14 @@ 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) @@ -88,10 +88,6 @@ func (c *Controller) UpdateLocalService(manifests []manifestv1alpha1.Manifest) b 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 index d71ee5793..8fe9dd999 100644 --- 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 index 845bf12b6..0e264785d 100644 --- 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 }