Correct label selector for addon to get works (open-cluster-managemen…

…t-io#497) (#77)

Signed-off-by: Jian Qiu <jqiu@redhat.com>

.github/workflows/e2e.yml

@@ -32,7 +32,7 @@ jobs:
       - name: Setup kind
         uses: engineerd/setup-kind@v0.5.0
         with:
-          version: v0.17.0
+          version: v0.22.0
       - name: install imagebuilder
         run: go install github.com/openshift/imagebuilder/cmd/imagebuilder@v1.2.3
       - name: Build images
@@ -61,7 +61,7 @@ jobs:
       - name: Setup kind
         uses: engineerd/setup-kind@v0.5.0
         with:
-          version: v0.17.0
+          version: v0.22.0
       - name: install imagebuilder
         run: go install github.com/openshift/imagebuilder/cmd/imagebuilder@v1.2.3
       - name: Build images
@@ -90,7 +90,7 @@ jobs:
       - name: Setup kind
         uses: engineerd/setup-kind@v0.5.0
         with:
-          version: v0.17.0
+          version: v0.22.0
       - name: install imagebuilder
         run: go install github.com/openshift/imagebuilder/cmd/imagebuilder@v1.2.3
       - name: Build images

pkg/addon/controllers/addonprogressing/controller.go

@@ -153,7 +153,7 @@ func (c *addonProgressingController) updateAddonProgressingAndLastApplied(
 		return patcher.PatchStatus(ctx, newaddon, newaddon.Status, oldaddon.Status)
 	}
 
-	var hostingClusterName string = ""
+	var hostingClusterName string
 	if newaddon.Annotations != nil && len(newaddon.Annotations[addonapiv1alpha1.HostingClusterNameAnnotationKey]) > 0 {
 		hostingClusterName = newaddon.Annotations[addonapiv1alpha1.HostingClusterNameAnnotationKey]
 	}
@@ -182,16 +182,20 @@ func (c *addonProgressingController) updateAddonProgressingAndLastApplied(
 	}
 
 	// get addon works
+	// first get all works for addon in default mode.
 	requirement, _ := labels.NewRequirement(addonapiv1alpha1.AddonLabelKey, selection.Equals, []string{newaddon.Name})
-	selector := labels.NewSelector().Add(*requirement)
+	namespaceNotExistRequirement, _ := labels.NewRequirement(addonapiv1alpha1.AddonNamespaceLabelKey, selection.DoesNotExist, []string{})
+	selector := labels.NewSelector().Add(*requirement).Add(*namespaceNotExistRequirement)
 	addonWorks, err := c.workLister.ManifestWorks(newaddon.Namespace).List(selector)
 	if err != nil {
 		setAddOnProgressingAndLastApplied(isUpgrade, ProgressingFailed, err.Error(), newaddon)
 		return patcher.PatchStatus(ctx, newaddon, newaddon.Status, oldaddon.Status)
 	}
 
+	// next get all works for addon in hosted mode
 	if len(hostingClusterName) > 0 {
-		// get hosted addon works
+		namespaceRequirement, _ := labels.NewRequirement(addonapiv1alpha1.AddonNamespaceLabelKey, selection.Equals, []string{newaddon.Namespace})
+		selector = labels.NewSelector().Add(*requirement).Add(*namespaceRequirement)
 		hostedAddonWorks, err := c.workLister.ManifestWorks(hostingClusterName).List(selector)
 		if err != nil {
 			setAddOnProgressingAndLastApplied(isUpgrade, ProgressingFailed, err.Error(), newaddon)

pkg/addon/controllers/addonprogressing/controller_test.go

@@ -532,6 +532,88 @@ func TestReconcile(t *testing.T) {
 				}
 			},
 		},
+		{
+			name:    "works for hosted and default addon in the same namespace",
+			syncKey: "cluster1/test",
+			managedClusteraddon: []runtime.Object{func() *addonapiv1alpha1.ManagedClusterAddOn {
+				addon := addontesting.NewAddon("test", "cluster1")
+				addon.Status.ConfigReferences = []addonapiv1alpha1.ConfigReference{
+					{
+						ConfigGroupResource: v1alpha1.ConfigGroupResource{Group: "core", Resource: "foo"},
+						DesiredConfig: &v1alpha1.ConfigSpecHash{
+							ConfigReferent: v1alpha1.ConfigReferent{Name: "test", Namespace: "open-cluster-management"},
+							SpecHash:       "hashnew",
+						},
+						LastAppliedConfig: &v1alpha1.ConfigSpecHash{
+							ConfigReferent: v1alpha1.ConfigReferent{Name: "test", Namespace: "open-cluster-management"},
+							SpecHash:       "hash",
+						},
+					},
+				}
+				meta.SetStatusCondition(&addon.Status.Conditions, metav1.Condition{
+					Type:    addonapiv1alpha1.ManagedClusterAddOnManifestApplied,
+					Status:  metav1.ConditionTrue,
+					Reason:  addonapiv1alpha1.AddonManifestAppliedReasonManifestsApplied,
+					Message: "manifests of addon are applied successfully",
+				})
+				return addon
+			}()},
+			clusterManagementAddon: []runtime.Object{addontesting.NewClusterManagementAddon("test", "testcrd", "testcr").Build()},
+			work: func() []runtime.Object {
+				work := addontesting.NewManifestWork(
+					"addon-test-deploy",
+					"cluster1",
+					testingcommon.NewUnstructured("v1", "ConfigMap", "default", "test1"),
+					testingcommon.NewUnstructured("v1", "Deployment", "default", "test1"),
+				)
+				work.SetLabels(map[string]string{
+					addonapiv1alpha1.AddonLabelKey: "test",
+				})
+				work.SetAnnotations(map[string]string{
+					workapiv1.ManifestConfigSpecHashAnnotationKey: "{\"foo.core/open-cluster-management/test\":\"hashnew\"}",
+				})
+				work.Status.Conditions = []metav1.Condition{
+					{
+						Type:   workapiv1.WorkApplied,
+						Status: metav1.ConditionTrue,
+					},
+					{
+						Type:   workapiv1.WorkAvailable,
+						Status: metav1.ConditionTrue,
+					},
+				}
+				hostedWork := addontesting.NewManifestWork(
+					"addon-test-deploy-hosting-another-cluster",
+					"cluster1",
+					testingcommon.NewUnstructured("v1", "ConfigMap", "default", "test1"),
+				)
+				hostedWork.SetLabels(map[string]string{
+					addonapiv1alpha1.AddonLabelKey:          "test",
+					addonapiv1alpha1.AddonNamespaceLabelKey: "another-cluster",
+				})
+				return []runtime.Object{work, hostedWork}
+			}(),
+			validateAddonActions: func(t *testing.T, actions []clienttesting.Action) {
+				testingcommon.AssertActions(t, actions, "patch")
+				actual := actions[0].(clienttesting.PatchActionImpl).Patch
+
+				addOn := &addonapiv1alpha1.ManagedClusterAddOn{}
+				err := json.Unmarshal(actual, addOn)
+				if err != nil {
+					t.Fatal(err)
+				}
+				configCond := meta.FindStatusCondition(addOn.Status.Conditions, addonapiv1alpha1.ManagedClusterAddOnConditionProgressing)
+				if !(configCond != nil && configCond.Reason == addonapiv1alpha1.ProgressingReasonUpgradeSucceed && configCond.Status == metav1.ConditionFalse) {
+					t.Errorf("Condition Progressing is incorrect")
+				}
+				if len(addOn.Status.ConfigReferences) != 1 {
+					t.Errorf("ConfigReferences object is not correct: %v", addOn.Status.ConfigReferences)
+				}
+				if addOn.Status.ConfigReferences[0].LastAppliedConfig.SpecHash != addOn.Status.ConfigReferences[0].DesiredConfig.SpecHash {
+					t.Errorf("LastAppliedConfig object is not correct: %v", addOn.Status.ConfigReferences[0].LastAppliedConfig.SpecHash)
+				}
+			},
+		},
 		{
 			name:    "update managedclusteraddon to configuration unsupported...",
 			syncKey: "cluster1/test",

test/integration-test.mk

@@ -2,7 +2,7 @@ TEST_TMP :=/tmp
 
 export KUBEBUILDER_ASSETS ?=$(TEST_TMP)/kubebuilder/bin
 
-K8S_VERSION ?=1.23.1
+K8S_VERSION ?=1.29.3
 KB_TOOLS_ARCHIVE_NAME :=kubebuilder-tools-$(K8S_VERSION)-$(GOHOSTOS)-$(GOHOSTARCH).tar.gz
 KB_TOOLS_ARCHIVE_PATH := $(TEST_TMP)/$(KB_TOOLS_ARCHIVE_NAME)