Minor fixes

pkg/controllers/workapplier/controller.go

@@ -192,6 +192,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		klog.ErrorS(err, "Failed to retrieve the work", "work", req.NamespacedName)
 		return ctrl.Result{}, controller.NewAPIServerError(true, err)
 	}
+
 	workRef := klog.KObj(work)
 
 	// Garbage collect the AppliedWork object if the Work object has been deleted.
@@ -261,7 +262,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	}
 
 	// If the Work object is not yet available, reconcile again in 5 seconds.
-	if isWorkObjectAvailable(work) {
+	if !isWorkObjectAvailable(work) {
 		return ctrl.Result{RequeueAfter: availabilityCheckRequeueAfter}, nil
 	}
 	// Otherwise, reconcile again in 3 minutes for drift detection purposes.
@@ -352,7 +353,7 @@ func (r *Reconciler) writeAheadManifestProcessingAttempts(
 	// a previous apply attempt has been recorded (**successful or not**), Fleet will skip the write-ahead
 	// op.
 	workAppliedCond := meta.FindStatusCondition(work.Status.Conditions, fleetv1beta1.WorkConditionTypeApplied)
-	if workAppliedCond.ObservedGeneration == work.Generation {
+	if workAppliedCond != nil && workAppliedCond.ObservedGeneration == work.Generation {
 		klog.V(2).InfoS("Fleet has attempted to apply the current set of manifests before and recorded the results; will skip the write-ahead process", "work", klog.KObj(work))
 		return nil
 	}
@@ -432,6 +433,11 @@ func (r *Reconciler) writeAheadManifestProcessingAttempts(
 	//
 	// Note that the Work object might have been refreshed by controllers on the hub cluster
 	// before this step runs; in this case the current reconciliation loop must be abandoned.
+	if work.Status.Conditions == nil {
+		// As a sanity check, set an empty set of conditions. Currently the API definition does
+		// not allow nil conditions.
+		work.Status.Conditions = []metav1.Condition{}
+	}
 	work.Status.ManifestConditions = manifestCondsForWA
 	if err := r.hubClient.Status().Update(ctx, work); err != nil {
 		return controller.NewAPIServerError(false, fmt.Errorf("failed to write ahead manifest processing attempts: %w", err))
@@ -624,6 +630,7 @@ func (r *Reconciler) processOneManifest(
 	}
 
 	// All done.
+	bundle.applyResTyp = ManifestProcessingApplyResultTypeApplied
 }
 
 // findInMemberClusterObjectFor attempts to find the corresponding object in the member cluster

pkg/controllers/workapplier/status.go

@@ -46,33 +46,41 @@ func (r *Reconciler) refreshWorkStatus(
 		setManifestAppliedCondition(manifestCond, bundle.applyResTyp, bundle.applyErr, work.Generation)
 		setManifestAvailableCondition(manifestCond, bundle.availabilityResTyp, bundle.availabilityErr, work.Generation)
 
+		// Check if a first drifted timestamp has been set; if not, set it to the current time.
 		firstDriftedTimestamp := bundle.firstDriftedTimestamp
 		if firstDriftedTimestamp == nil {
 			firstDriftedTimestamp = &now
 		}
-		manifestCond.DriftDetails = &fleetv1beta1.DriftDetails{
-			ObservationTime:                   now,
-			ObservedInMemberClusterGeneration: bundle.inMemberClusterObj.GetGeneration(),
-			FirstDriftedObservedTime:          *firstDriftedTimestamp,
-			ObservedDrifts:                    bundle.drifts,
+		if bundle.drifts != nil {
+			// Populate drift details if there are drifts found.
+			manifestCond.DriftDetails = &fleetv1beta1.DriftDetails{
+				ObservationTime:                   now,
+				ObservedInMemberClusterGeneration: bundle.inMemberClusterObj.GetGeneration(),
+				FirstDriftedObservedTime:          *firstDriftedTimestamp,
+				ObservedDrifts:                    bundle.drifts,
+			}
 		}
 
+		// Check if a first diffed timestamp has been set; if not, set it to the current time.
 		firstDiffedTimestamp := bundle.firstDiffedTimestamp
 		if firstDiffedTimestamp == nil {
 			firstDiffedTimestamp = &now
 		}
-		manifestCond.DiffDetails = &fleetv1beta1.DiffDetails{
-			ObservationTime:                   now,
-			ObservedInMemberClusterGeneration: bundle.inMemberClusterObj.GetGeneration(),
-			FirstDiffedObservedTime:           *firstDiffedTimestamp,
-			ObservedDiffs:                     bundle.diffs,
+		// Populate diff details if there are diffs found.
+		if bundle.diffs != nil {
+			manifestCond.DiffDetails = &fleetv1beta1.DiffDetails{
+				ObservationTime:                   now,
+				ObservedInMemberClusterGeneration: bundle.inMemberClusterObj.GetGeneration(),
+				FirstDiffedObservedTime:           *firstDiffedTimestamp,
+				ObservedDiffs:                     bundle.diffs,
+			}
 		}
 
 		// Tally the stats.
 		if isManifestObjectApplied(bundle.applyResTyp) {
 			appliedManifestsCount++
 		}
-		if !isAppliedObjectAvailable(bundle.availabilityResTyp) {
+		if isAppliedObjectAvailable(bundle.availabilityResTyp) {
 			availableAppliedObjectsCount++
 		}
 	}
@@ -127,7 +135,7 @@ func (r *Reconciler) refreshAppliedWorkStatus(
 
 	// Update the AppliedWork object status.
 	appliedWork.Status.AppliedResources = appliedResources
-	if err := r.hubClient.Status().Update(ctx, appliedWork); err != nil {
+	if err := r.spokeClient.Status().Update(ctx, appliedWork); err != nil {
 		return controller.NewAPIServerError(false, err)
 	}
 	return nil

pkg/controllers/workapplier/suite_test.go

@@ -29,6 +29,8 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
@@ -38,6 +40,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/metrics/server"
 
@@ -52,7 +55,7 @@ var (
 	memberCfg           *rest.Config
 	hubEnv              *envtest.Environment
 	memberEnv           *envtest.Environment
-	memberMgr           manager.Manager
+	hubMgr              manager.Manager
 	hubClient           client.Client
 	memberClient        client.Client
 	memberDynamicClient dynamic.Interface
@@ -77,6 +80,15 @@ func TestAPIs(t *testing.T) {
 	RunSpecs(t, "Work Applier Integration Test Suite")
 }
 
+func setupResources() {
+	ns := &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: memberReservedNSName,
+		},
+	}
+	Expect(hubClient.Create(ctx, ns)).To(Succeed())
+}
+
 var _ = BeforeSuite(func() {
 	ctx, cancel = context.WithCancel(context.TODO())
 
@@ -85,6 +97,8 @@ var _ = BeforeSuite(func() {
 	klog.InitFlags(fs)
 	Expect(fs.Parse([]string{"--v", "5", "-add_dir_header", "true"})).Should(Succeed())
 
+	klog.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
+
 	By("Bootstrapping test environments")
 	hubEnv = &envtest.Environment{
 		CRDDirectoryPaths: []string{
@@ -126,8 +140,11 @@ var _ = BeforeSuite(func() {
 	memberDynamicClient, err = dynamic.NewForConfig(memberCfg)
 	Expect(err).ToNot(HaveOccurred())
 
+	By("Setting up the resources")
+	setupResources()
+
 	By("Setting up the controller and the controller manager")
-	memberMgr, err = ctrl.NewManager(memberCfg, ctrl.Options{
+	hubMgr, err = ctrl.NewManager(hubCfg, ctrl.Options{
 		Scheme: scheme.Scheme,
 		Metrics: server.Options{
 			BindAddress: "0",
@@ -146,16 +163,17 @@ var _ = BeforeSuite(func() {
 		memberReservedNSName,
 		memberDynamicClient,
 		memberClient,
-		memberMgr.GetRESTMapper(),
-		memberMgr.GetEventRecorderFor("work-applier"),
+		memberClient.RESTMapper(),
+		hubMgr.GetEventRecorderFor("work-applier"),
 		maxConcurrentReconciles,
 		workerCount,
 	)
-	Expect(workApplier.SetupWithManager(memberMgr)).To(Succeed())
+	Expect(workApplier.SetupWithManager(hubMgr)).To(Succeed())
 
 	go func() {
 		defer GinkgoRecover()
-		Expect(memberMgr.Start(ctx)).To(Succeed())
+		Expect(workApplier.Join(ctx)).To(Succeed())
+		Expect(hubMgr.Start(ctx)).To(Succeed())
 	}()
 })
 

pkg/controllers/workapplier/utils.go

@@ -397,22 +397,33 @@ func validateOwnerReferences(
 	expectedAppliedWorkOwnerRef *metav1.OwnerReference,
 ) error {
 	manifestObjOwnerRefs := manifestObj.GetOwnerReferences()
-	inMemberClusterObjOwnerRefs := inMemberClusterObj.GetOwnerReferences()
 
 	// If the manifest object already features some owner reference(s), but co-ownership is
 	// disallowed, the validation fails.
+	//
+	// This is just a sanity check; normally the branch will never get triggered as Fleet would
+	// perform sanitization on the manifest object before applying it, which removes all owner
+	// references.
 	if len(manifestObjOwnerRefs) > 0 && !applyStrategy.AllowCoOwnership {
 		return fmt.Errorf("manifest is set to have multiple owner references but co-ownership is disallowed")
 	}
 
 	// Do a sanity check to verify that no AppliedWork object is directly added as an owner
-	// in the manifest object.
+	// in the manifest object. Normally the branch will never get triggered as Fleet would
+	// perform sanitization on the manifest object before applying it, which removes all owner
+	// references.
 	for _, ownerRef := range manifestObjOwnerRefs {
 		if ownerRef.APIVersion == fleetv1beta1.GroupVersion.String() && ownerRef.Kind == fleetv1beta1.AppliedWorkKind {
 			return fmt.Errorf("an AppliedWork object is unexpectedly added as an owner in the manifest object")
 		}
 	}
 
+	if inMemberClusterObj == nil {
+		// The manifest object has never been applied yet; no need to do further validation.
+		return nil
+	}
+	inMemberClusterObjOwnerRefs := inMemberClusterObj.GetOwnerReferences()
+
 	// If the live object is co-owned but co-ownership is no longer allowed, the validation fails.
 	if len(inMemberClusterObjOwnerRefs) > 1 && !applyStrategy.AllowCoOwnership {
 		return fmt.Errorf("object is co-owned by multiple objects but co-ownership has been disallowed")
@@ -769,6 +780,7 @@ func prepareManifestCondForWA(
 				ObservedGeneration: workGeneration,
 				Reason:             ManifestAppliedCondPreparingToProcessReason,
 				Message:            ManifestAppliedCondPreparingToProcessMessage,
+				LastTransitionTime: metav1.Now(),
 			},
 		},
 	}