Skip to content

Commit d06d219

Browse files
author
Ryan Zhang
committed
test
1 parent 9a376db commit d06d219

File tree

5 files changed

+150
-14
lines changed

5 files changed

+150
-14
lines changed

pkg/controllers/workgenerator/controller.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req controllerruntime.Reques
188188
Status: metav1.ConditionFalse,
189189
Type: string(fleetv1beta1.ResourceBindingOverridden),
190190
Reason: condition.OverriddenFailedReason,
191-
Message: fmt.Sprintf("Failed to apply the override rules on the resources: %s", errorMessage),
191+
Message: errorMessage,
192192
ObservedGeneration: resourceBinding.Generation,
193193
})
194194
} else {
@@ -624,13 +624,13 @@ func (r *Reconciler) fetchAllResourceSnapshots(ctx context.Context, resourceBind
624624
func (r *Reconciler) getConfigMapEnvelopWorkObj(ctx context.Context, workNamePrefix string, resourceBinding *fleetv1beta1.ClusterResourceBinding,
625625
resourceSnapshot *fleetv1beta1.ClusterResourceSnapshot, envelopeObj *unstructured.Unstructured, resourceOverrideSnapshotHash, clusterResourceOverrideSnapshotHash string) (*fleetv1beta1.Work, error) {
626626
// we group all the resources in one configMap to one work
627-
manifest, err := extractResFromConfigMap(envelopeObj)
627+
manifests, err := extractResFromConfigMap(envelopeObj)
628628
if err != nil {
629629
klog.ErrorS(err, "configMap has invalid content", "snapshot", klog.KObj(resourceSnapshot),
630630
"resourceBinding", klog.KObj(resourceBinding), "configMapWrapper", klog.KObj(envelopeObj))
631631
return nil, controller.NewUserError(err)
632632
}
633-
klog.V(2).InfoS("Successfully extract the enveloped resources from the configMap", "numOfResources", len(manifest),
633+
klog.V(2).InfoS("Successfully extract the enveloped resources from the configMap", "numOfResources", len(manifests),
634634
"snapshot", klog.KObj(resourceSnapshot), "resourceBinding", klog.KObj(resourceBinding), "configMapWrapper", klog.KObj(envelopeObj))
635635

636636
// Try to see if we already have a work represent the same enveloped object for this CRP in the same cluster
@@ -643,7 +643,7 @@ func (r *Reconciler) getConfigMapEnvelopWorkObj(ctx context.Context, workNamePre
643643
fleetv1beta1.EnvelopeNamespaceLabel: envelopeObj.GetNamespace(),
644644
}
645645
workList := &fleetv1beta1.WorkList{}
646-
if err := r.Client.List(ctx, workList, envelopWorkLabelMatcher); err != nil {
646+
if err = r.Client.List(ctx, workList, envelopWorkLabelMatcher); err != nil {
647647
return nil, controller.NewAPIServerError(true, err)
648648
}
649649
// we need to create a new work object
@@ -680,7 +680,7 @@ func (r *Reconciler) getConfigMapEnvelopWorkObj(ctx context.Context, workNamePre
680680
},
681681
Spec: fleetv1beta1.WorkSpec{
682682
Workload: fleetv1beta1.WorkloadTemplate{
683-
Manifests: manifest,
683+
Manifests: manifests,
684684
},
685685
ApplyStrategy: resourceBinding.Spec.ApplyStrategy,
686686
},
@@ -699,7 +699,7 @@ func (r *Reconciler) getConfigMapEnvelopWorkObj(ctx context.Context, workNamePre
699699
work.Annotations[fleetv1beta1.ParentResourceSnapshotNameAnnotation] = resourceBinding.Spec.ResourceSnapshotName
700700
work.Annotations[fleetv1beta1.ParentResourceOverrideSnapshotHashAnnotation] = resourceOverrideSnapshotHash
701701
work.Annotations[fleetv1beta1.ParentClusterResourceOverrideSnapshotHashAnnotation] = clusterResourceOverrideSnapshotHash
702-
work.Spec.Workload.Manifests = manifest
702+
work.Spec.Workload.Manifests = manifests
703703
work.Spec.ApplyStrategy = resourceBinding.Spec.ApplyStrategy
704704
return &work, nil
705705
}

pkg/controllers/workgenerator/override.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,9 @@ func (r *Reconciler) applyOverrides(resource *placementv1beta1.ResourceContent,
137137
continue // should not happen
138138
}
139139
if err := applyOverrideRules(resource, cluster, snapshot.Spec.OverrideSpec.Policy.OverrideRules); err != nil {
140-
klog.ErrorS(err, "Failed to apply the override rules", "clusterResourceOverrideSnapshot", klog.KObj(snapshot))
141-
return false, err
140+
overrideErr := fmt.Errorf("failed to apply the cluster override rule `%s` on resource `%s/%s/%s`: err = %s", snapshot.Name, uResource.GroupVersionKind(), uResource.GetNamespace(), uResource.GetName(), err.Error())
141+
klog.ErrorS(err, "Failed to apply the override rules", "resource", klog.KObj(&uResource), "clusterResourceOverrideSnapshot", klog.KObj(snapshot))
142+
return false, controller.NewUserError(overrideErr)
142143
}
143144
}
144145
klog.V(2).InfoS("Applied clusterResourceOverrideSnapshots", "resource", klog.KObj(&uResource), "numberOfOverrides", len(croMap[key]))
@@ -160,8 +161,9 @@ func (r *Reconciler) applyOverrides(resource *placementv1beta1.ResourceContent,
160161
continue // should not happen
161162
}
162163
if err := applyOverrideRules(resource, cluster, snapshot.Spec.OverrideSpec.Policy.OverrideRules); err != nil {
163-
klog.ErrorS(err, "Failed to apply the override rules", "resourceOverrideSnapshot", klog.KObj(snapshot))
164-
return false, err
164+
overrideErr := fmt.Errorf("failed to apply the resource override rule %s/%s on resource `%s/%s/%s`: err = %s", snapshot.Namespace, snapshot.Name, uResource.GroupVersionKind(), uResource.GetNamespace(), uResource.GetName(), err.Error())
165+
klog.ErrorS(err, "Failed to apply the override rules", "resource", klog.KObj(&uResource), "resourceOverrideSnapshot", klog.KObj(snapshot))
166+
return false, controller.NewUserError(overrideErr)
165167
}
166168
}
167169
klog.V(2).InfoS("Applied resourceOverrideSnapshots", "resource", klog.KObj(&uResource), "numberOfOverrides", len(roMap[key]))
@@ -174,7 +176,7 @@ func applyOverrideRules(resource *placementv1beta1.ResourceContent, cluster *clu
174176
matched, err := overrider.IsClusterMatched(cluster, rule)
175177
if err != nil {
176178
klog.ErrorS(controller.NewUnexpectedBehaviorError(err), "Found an invalid override rule")
177-
return controller.NewUserError(err) // should not happen though and should be rejected by the webhook
179+
return err // should not happen though and should be rejected by the webhook
178180
}
179181
if !matched {
180182
continue
@@ -265,7 +267,7 @@ func replaceClusterLabelKeyVariables(input string, cluster *clusterv1beta1.Membe
265267
return "", fmt.Errorf("label key %s not found on cluster %s", keyName, cluster.Name)
266268
}
267269
// replace this instance of the variable with the actual label value
268-
fullVariable := result[startIdx : endIdx+1]
270+
fullVariable := fmt.Sprintf("%s%s}", placementv1alpha1.OverrideClusterLabelKeyVariablePrefix, keyName)
269271
result = strings.Replace(result, fullVariable, labelValue, 1)
270272
}
271273
return result, nil

pkg/controllers/workgenerator/override_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1123,7 +1123,7 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) {
11231123
}
11241124

11251125
var clusterRole rbacv1.ClusterRole
1126-
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, &clusterRole); err != nil {
1126+
if err = runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, &clusterRole); err != nil {
11271127
t.Fatalf("Failed to convert the result to clusterole: %v, want nil", err)
11281128
}
11291129

test/e2e/actuals_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -976,7 +976,7 @@ func workNamespaceRemovedFromClusterActual(cluster *framework.Cluster) func() er
976976
ns := appNamespace()
977977
return func() error {
978978
if err := client.Get(ctx, types.NamespacedName{Name: ns.Name}, &corev1.Namespace{}); !errors.IsNotFound(err) {
979-
return fmt.Errorf("work namespace %s still exists or an unexpected error occurred: %w", ns.Name, err)
979+
return fmt.Errorf("work namespace %s still exists on cluster %s or an unexpected error occurred: %w", ns.Name, cluster.ClusterName, err)
980980
}
981981
return nil
982982
}

test/e2e/placement_ro_test.go

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ var _ = Context("creating resourceOverride with multiple jsonPatchOverrides to o
161161
Namespace: roNamespace,
162162
},
163163
Spec: placementv1alpha1.ResourceOverrideSpec{
164+
Placement: &placementv1alpha1.PlacementRef{
165+
Name: crpName, // assigned CRP name
166+
},
164167
ResourceSelectors: configMapSelector(),
165168
Policy: &placementv1alpha1.OverridePolicy{
166169
OverrideRules: []placementv1alpha1.OverrideRule{
@@ -599,6 +602,9 @@ var _ = Context("creating resourceOverride with a templated rules with cluster n
599602
Namespace: roNamespace,
600603
},
601604
Spec: placementv1alpha1.ResourceOverrideSpec{
605+
Placement: &placementv1alpha1.PlacementRef{
606+
Name: crpName, // assigned CRP name
607+
},
602608
ResourceSelectors: configMapSelector(),
603609
Policy: &placementv1alpha1.OverridePolicy{
604610
OverrideRules: []placementv1alpha1.OverrideRule{
@@ -693,6 +699,9 @@ var _ = Context("creating resourceOverride with delete configMap", Ordered, func
693699
Namespace: roNamespace,
694700
},
695701
Spec: placementv1alpha1.ResourceOverrideSpec{
702+
Placement: &placementv1alpha1.PlacementRef{
703+
Name: crpName, // assigned CRP name
704+
},
696705
ResourceSelectors: configMapSelector(),
697706
Policy: &placementv1alpha1.OverridePolicy{
698707
OverrideRules: []placementv1alpha1.OverrideRule{
@@ -789,3 +798,128 @@ var _ = Context("creating resourceOverride with delete configMap", Ordered, func
789798
}, consistentlyDuration, consistentlyInterval).Should(BeTrue(), "Failed to delete work resources on member cluster %s", memberCluster.ClusterName)
790799
})
791800
})
801+
802+
var _ = FContext("creating resourceOverride with a templated rules with cluster label key replacement", Ordered, func() {
803+
crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess())
804+
roName := fmt.Sprintf(roNameTemplate, GinkgoParallelProcess())
805+
roNamespace := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess())
806+
807+
BeforeAll(func() {
808+
By("creating work resources")
809+
createWorkResources()
810+
811+
// Create the ro before crp so that the observed resource index is predictable.
812+
ro := &placementv1alpha1.ResourceOverride{
813+
ObjectMeta: metav1.ObjectMeta{
814+
Name: roName,
815+
Namespace: roNamespace,
816+
},
817+
Spec: placementv1alpha1.ResourceOverrideSpec{
818+
Placement: &placementv1alpha1.PlacementRef{
819+
Name: crpName, // assigned CRP name
820+
},
821+
ResourceSelectors: configMapSelector(),
822+
Policy: &placementv1alpha1.OverridePolicy{
823+
OverrideRules: []placementv1alpha1.OverrideRule{
824+
{
825+
ClusterSelector: &placementv1beta1.ClusterSelector{
826+
ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{
827+
{
828+
LabelSelector: &metav1.LabelSelector{
829+
MatchExpressions: []metav1.LabelSelectorRequirement{
830+
{
831+
Key: regionLabelName,
832+
Operator: metav1.LabelSelectorOpExists,
833+
},
834+
},
835+
},
836+
},
837+
},
838+
},
839+
JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{
840+
{
841+
Operator: placementv1alpha1.JSONPatchOverrideOpReplace,
842+
Path: "/data/region",
843+
Value: apiextensionsv1.JSON{Raw: []byte(fmt.Sprintf(`"%s%s}"`, placementv1alpha1.OverrideClusterLabelKeyVariablePrefix, regionLabelName))},
844+
},
845+
},
846+
},
847+
},
848+
},
849+
},
850+
}
851+
By(fmt.Sprintf("creating resourceOverride %s", roName))
852+
Expect(hubClient.Create(ctx, ro)).To(Succeed(), "Failed to create resourceOverride %s", roName)
853+
854+
// Create the CRP.
855+
createCRP(crpName)
856+
})
857+
858+
AfterAll(func() {
859+
By(fmt.Sprintf("deleting placement %s and related resources", crpName))
860+
ensureCRPAndRelatedResourcesDeleted(crpName, allMemberClusters)
861+
862+
By(fmt.Sprintf("deleting resourceOverride %s", roName))
863+
cleanupResourceOverride(roName, roNamespace)
864+
})
865+
866+
It("should update CRP status as expected", func() {
867+
wantRONames := []placementv1beta1.NamespacedName{
868+
{Namespace: roNamespace, Name: fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0)},
869+
}
870+
crpStatusUpdatedActual := crpStatusWithOverrideUpdatedActual(workResourceIdentifiers(), allMemberClusterNames, "0", nil, wantRONames)
871+
Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName)
872+
})
873+
874+
It("should replace the cluster label key in the configMap", func() {
875+
cmName := fmt.Sprintf(appConfigMapNameTemplate, GinkgoParallelProcess())
876+
cmNamespace := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess())
877+
for _, cluster := range allMemberClusters {
878+
879+
wantConfigMap := &corev1.ConfigMap{
880+
ObjectMeta: metav1.ObjectMeta{
881+
Name: cmName,
882+
Namespace: cmNamespace,
883+
},
884+
Data: map[string]string{
885+
"region": labelsByClusterName[cluster.ClusterName][regionLabelName],
886+
},
887+
}
888+
configMapActual := configMapPlacedOnClusterActual(cluster, wantConfigMap)
889+
Eventually(configMapActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update configmap %s data as expected", cmName)
890+
}
891+
})
892+
893+
It("should handle non-existent cluster label key gracefully", func() {
894+
// Update the ResourceOverride to use a non-existent label key
895+
Eventually(func() error {
896+
ro := &placementv1alpha1.ResourceOverride{}
897+
if err := hubClient.Get(ctx, types.NamespacedName{Name: roName, Namespace: roNamespace}, ro); err != nil {
898+
return err
899+
}
900+
ro.Spec.Policy.OverrideRules[0].JSONPatchOverrides[0].Value = apiextensionsv1.JSON{Raw: []byte(fmt.Sprintf(`"%snon-existent-label}"`, placementv1alpha1.OverrideClusterLabelKeyVariablePrefix))}
901+
return hubClient.Update(ctx, ro)
902+
}, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update resourceOverride %s with non-existent label key", roName)
903+
904+
// Verify CRP status reflects the error
905+
crpStatusUpdatedActual := crpStatusWithOverrideUpdatedFailedActual(workResourceIdentifiers(), allMemberClusterNames, "0", nil, nil)
906+
Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "CRP %s status should reflect the error due to non-existent label key", crpName)
907+
908+
// Verify the configMap remains unchanged
909+
cmName := fmt.Sprintf(appConfigMapNameTemplate, GinkgoParallelProcess())
910+
cmNamespace := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess())
911+
for _, cluster := range allMemberClusters {
912+
wantConfigMap := &corev1.ConfigMap{
913+
ObjectMeta: metav1.ObjectMeta{
914+
Name: cmName,
915+
Namespace: cmNamespace,
916+
},
917+
Data: map[string]string{
918+
"region": labelsByClusterName[cluster.ClusterName][regionLabelName],
919+
},
920+
}
921+
configMapActual := configMapPlacedOnClusterActual(cluster, wantConfigMap)
922+
Consistently(configMapActual, consistentlyDuration, consistentlyInterval).Should(Succeed(), "ConfigMap %s should remain unchanged", cmName)
923+
}
924+
})
925+
})

0 commit comments

Comments
 (0)