address comments

Signed-off-by: Ryan Zhang <[email protected]>

Author: Ryan Zhang

hack/Azure/property-based-scheduling.md

@@ -424,6 +424,6 @@ All the AKS clusters in the resource group will be removed.
 Congrats! We hope that property-based scheduling (preview) has improved your overall Fleet experience. If you have any questions, feedback, or concerns, please raise [a GitHub issue](https://github.com/Azure/fleet/issues).
 
 Aside from property-based scheduling, Fleet offers many other scheduling features that are useful in a
-multi-cluster environment; check out the [How-to Guide: Using the Fleet `ClusterResourcePlacement` API](https://github.com/Azure/fleet/tree/main/docs/howtos/crp.md) for more information.
+multi-cluster environment; check out the [How-to Guide: Using the Fleet `ClusterResourcePlacement` API](https://kubefleet.dev/docs/how-tos/crp/) for more information.
 
-You can also review Fleet's [source code](https://github.com/Azure/fleet) or review its [documentation](https://github.com/Azure/fleet/tree/main/docs) on GitHub.
+You can also review Fleet's [source code](https://github.com/kubefleet-dev/kubefleet/) or review its [documentation](https://kubefleet.dev/docs/) on GitHub.

pkg/controllers/clusterresourceplacement/controller_test.go

@@ -749,20 +749,20 @@ func TestGetOrCreateClusterSchedulingPolicySnapshot(t *testing.T) {
 			}
 			got, err := r.getOrCreateSchedulingPolicySnapshot(ctx, crp, int(limit))
 			if err != nil {
-				t.Fatalf("failed to getOrCreateClusterSchedulingPolicySnapshot: %v", err)
+				t.Fatalf("failed to getOrCreateSchedulingPolicySnapshot: %v", err)
 			}
 
 			// Convert interface to concrete type for comparison
 			gotSnapshot, ok := got.(*fleetv1beta1.ClusterSchedulingPolicySnapshot)
 			if !ok {
-				t.Fatalf("expected *ClusterSchedulingPolicySnapshot, got %T", got)
+				t.Fatalf("getOrCreateSchedulingPolicySnapshot() got %T, want *ClusterSchedulingPolicySnapshot", got)
 			}
 
 			options := []cmp.Option{
 				cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ResourceVersion"),
 			}
 			if diff := cmp.Diff(tc.wantPolicySnapshots[tc.wantLatestSnapshotIndex], *gotSnapshot, options...); diff != "" {
-				t.Errorf("getOrCreateClusterSchedulingPolicySnapshot() mismatch (-want, +got):\n%s", diff)
+				t.Errorf("getOrCreateSchedulingPolicySnapshot() mismatch (-want, +got):\n%s", diff)
 			}
 			clusterPolicySnapshotList := &fleetv1beta1.ClusterSchedulingPolicySnapshotList{}
 			if err := fakeClient.List(ctx, clusterPolicySnapshotList); err != nil {
@@ -2712,10 +2712,10 @@ func TestGetOrCreateClusterResourceSnapshot(t *testing.T) {
 			resourceSnapshotResourceSizeLimit = tc.selectedResourcesSizeLimit
 			res, got, err := r.getOrCreateResourceSnapshot(ctx, crp, tc.envelopeObjCount, tc.resourceSnapshotSpec, int(limit))
 			if err != nil {
-				t.Fatalf("failed to handle getOrCreateClusterResourceSnapshot: %v", err)
+				t.Fatalf("failed to handle getOrCreateResourceSnapshot: %v", err)
 			}
 			if res.Requeue != tc.wantRequeue {
-				t.Fatalf("getOrCreateClusterResourceSnapshot() got Requeue %v, want %v", res.Requeue, tc.wantRequeue)
+				t.Fatalf("getOrCreateResourceSnapshot() got Requeue %v, want %v", res.Requeue, tc.wantRequeue)
 			}
 
 			options := []cmp.Option{
@@ -2726,7 +2726,7 @@ func TestGetOrCreateClusterResourceSnapshot(t *testing.T) {
 			}
 			if tc.wantRequeue {
 				if res.RequeueAfter <= 0 {
-					t.Fatalf("getOrCreateClusterResourceSnapshot() got RequeueAfter %v, want greater than zero value", res.RequeueAfter)
+					t.Fatalf("getOrCreateResourceSnapshot() got RequeueAfter %v, want greater than zero value", res.RequeueAfter)
 				}
 			}
 			annotationOption := cmp.Transformer("NormalizeAnnotations", func(m map[string]string) map[string]string {

pkg/controllers/clusterresourceplacement/placement_status.go

@@ -191,7 +191,7 @@ func (r *Reconciler) appendScheduledResourcePlacementStatuses(
 		// The allRPS slice has been pre-allocated, so the append call will never produce a new
 		// slice; here, however, Fleet will still return the old slice just in case.
 		allRPS = append(allRPS, *rps)
-		klog.V(2).InfoS("Populated the resource placement status for the scheduled cluster", "clusterResourcePlacement", klog.KObj(placementObj), "cluster", clusterDecision.ClusterName, "resourcePlacementStatus", rps)
+		klog.V(2).InfoS("Populated the resource placement status for the scheduled cluster", "placement", klog.KObj(placementObj), "cluster", clusterDecision.ClusterName, "resourcePlacementStatus", rps)
 	}
 
 	return allRPS, rpsSetCondTypeCounter, nil
@@ -254,9 +254,10 @@ func setPlacementConditions(
 		}
 	}
 
-	klog.V(2).InfoS("Populated the placement conditions", "clusterResourcePlacement", klog.KObj(placementObj))
+	klog.V(2).InfoS("Populated the placement conditions", "placement", klog.KObj(placementObj))
 }
 
+// TODO: make this work with RP
 func (r *Reconciler) buildClusterResourceBindings(ctx context.Context, placementObj fleetv1beta1.PlacementObj, latestSchedulingPolicySnapshot fleetv1beta1.PolicySnapshotObj) (map[string]fleetv1beta1.BindingObj, error) {
 	// List all bindings derived from the CRP.
 	bindingList := &fleetv1beta1.ClusterResourceBindingList{}
@@ -280,7 +281,7 @@ func (r *Reconciler) buildClusterResourceBindings(ctx context.Context, placement
 
 		if len(bindings[i].Spec.TargetCluster) == 0 {
 			err := fmt.Errorf("targetCluster is empty on clusterResourceBinding %s", bindings[i].Name)
-			klog.ErrorS(controller.NewUnexpectedBehaviorError(err), "Found an invalid clusterResourceBinding and skipping it when building placement status", "clusterResourceBinding", klog.KObj(&bindings[i]), "clusterResourcePlacement", crpKObj)
+			klog.ErrorS(controller.NewUnexpectedBehaviorError(err), "Found an invalid clusterResourceBinding and skipping it when building placement status", "clusterResourceBinding", klog.KObj(&bindings[i]), "placement", crpKObj)
 			continue
 		}
 
@@ -294,6 +295,7 @@ func (r *Reconciler) buildClusterResourceBindings(ctx context.Context, placement
 	return res, nil
 }
 
+// TODO: make this work with RP
 // findClusterResourceSnapshotIndexForBindings finds the resource snapshot index for each binding.
 // It returns a map which maps the target cluster name to the resource snapshot index string.
 func (r *Reconciler) findClusterResourceSnapshotIndexForBindings(
@@ -306,19 +308,19 @@ func (r *Reconciler) findClusterResourceSnapshotIndexForBindings(
 	for targetCluster, binding := range bindingMap {
 		resourceSnapshotName := binding.GetBindingSpec().ResourceSnapshotName
 		if resourceSnapshotName == "" {
-			klog.InfoS("Empty resource snapshot name found in binding, controller might observe in-between state", "binding", klog.KObj(binding), "clusterResourcePlacement", crpKObj)
+			klog.InfoS("Empty resource snapshot name found in binding, controller might observe in-between state", "binding", klog.KObj(binding), "placement", crpKObj)
 			res[targetCluster] = ""
 			continue
 		}
 		resourceSnapshot := &fleetv1beta1.ClusterResourceSnapshot{}
 		if err := r.Client.Get(ctx, types.NamespacedName{Name: resourceSnapshotName, Namespace: ""}, resourceSnapshot); err != nil {
 			if apierrors.IsNotFound(err) {
 				klog.InfoS("The resource snapshot specified in binding is not found, probably deleted due to revision history limit",
-					"resourceSnapshotName", resourceSnapshotName, "binding", klog.KObj(binding), "clusterResourcePlacement", crpKObj)
+					"resourceSnapshotName", resourceSnapshotName, "binding", klog.KObj(binding), "placement", crpKObj)
 				res[targetCluster] = ""
 				continue
 			}
-			klog.ErrorS(err, "Failed to get the cluster resource snapshot specified in binding", "resourceSnapshotName", resourceSnapshotName, "binding", klog.KObj(binding), "clusterResourcePlacement", crpKObj)
+			klog.ErrorS(err, "Failed to get the cluster resource snapshot specified in binding", "resourceSnapshotName", resourceSnapshotName, "binding", klog.KObj(binding), "placement", crpKObj)
 			return res, controller.NewAPIServerError(true, err)
 		}
 		res[targetCluster] = resourceSnapshot.GetLabels()[fleetv1beta1.ResourceIndexLabel]
@@ -374,7 +376,7 @@ func (r *Reconciler) setResourcePlacementStatusPerCluster(
 		// The binding uses an out of date resource snapshot, and the RolloutStarted condition is
 		// set to True, Unknown, or has become stale. Fleet might be observing an in-between state.
 		meta.SetStatusCondition(&status.Conditions, condition.RolloutStartedCondition.UnknownResourceConditionPerCluster(placementObj.GetGeneration()))
-		klog.V(5).InfoS("The cluster resource binding has a stale RolloutStarted condition, or it links to an out of date resource snapshot yet has the RolloutStarted condition set to True or Unknown status", "clusterResourceBinding", klog.KObj(binding), "clusterResourcePlacement", klog.KObj(placementObj))
+		klog.V(5).InfoS("The cluster resource binding has a stale RolloutStarted condition, or it links to an out of date resource snapshot yet has the RolloutStarted condition set to True or Unknown status", "clusterResourceBinding", klog.KObj(binding), "placement", klog.KObj(placementObj))
 		res[condition.RolloutStartedCondition] = metav1.ConditionUnknown
 		return res
 	default:
@@ -384,7 +386,7 @@ func (r *Reconciler) setResourcePlacementStatusPerCluster(
 	}
 }
 
-// setResourcePlacementStatusBasedOnBinding sets the cluster's resource placement status based on its corresponding binding status.
+// setResourcePlacementStatusBasedOnBinding sets the placement status based on its corresponding binding status.
 // It updates the status object in place and tracks the set status for each relevant condition type in setStatusByCondType map provided.
 func setResourcePlacementStatusBasedOnBinding(
 	placementObj fleetv1beta1.PlacementObj,
@@ -398,7 +400,7 @@ func setResourcePlacementStatusBasedOnBinding(
 		if !condition.IsConditionStatusTrue(bindingCond, binding.GetGeneration()) &&
 			!condition.IsConditionStatusFalse(bindingCond, binding.GetGeneration()) {
 			meta.SetStatusCondition(&status.Conditions, i.UnknownResourceConditionPerCluster(placementObj.GetGeneration()))
-			klog.V(5).InfoS("Find an unknown condition", "bindingCond", bindingCond, "clusterResourceBinding", klog.KObj(binding), "clusterResourcePlacement", klog.KObj(placementObj))
+			klog.V(5).InfoS("Find an unknown condition", "bindingCond", bindingCond, "clusterResourceBinding", klog.KObj(binding), "placement", klog.KObj(placementObj))
 			setStatusByCondType[i] = metav1.ConditionUnknown
 			break
 		}