diff --git a/apis/placement/v1beta1/work_types.go b/apis/placement/v1beta1/work_types.go index f4592f3d7..d2b04e6e3 100644 --- a/apis/placement/v1beta1/work_types.go +++ b/apis/placement/v1beta1/work_types.go @@ -54,6 +54,10 @@ const ( // WorkConditionTypeDiffReported reports whether Fleet has successfully reported the // configuration difference between the states in the hub cluster and a member cluster. WorkConditionTypeDiffReported = "DiffReported" + + // WorkConditionTypeStatusTrimmed reports whether the member agent has to trim + // the status data in the Work object due to size constraints. + WorkConditionTypeStatusTrimmed = "StatusTrimmed" ) // This api is copied from https://github.com/kubernetes-sigs/work-api/blob/master/pkg/apis/v1alpha1/work_types.go. diff --git a/pkg/controllers/workapplier/status.go b/pkg/controllers/workapplier/status.go index 1a17ba150..8a9fe2667 100644 --- a/pkg/controllers/workapplier/status.go +++ b/pkg/controllers/workapplier/status.go @@ -28,9 +28,17 @@ import ( fleetv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" "github.com/kubefleet-dev/kubefleet/pkg/utils/condition" "github.com/kubefleet-dev/kubefleet/pkg/utils/controller" + "github.com/kubefleet-dev/kubefleet/pkg/utils/resource" +) + +const ( + WorkStatusTrimmedDueToOversizedStatusReason = "Oversized" + WorkStatusTrimmedDueToOversizedStatusMsgTmpl = "The status data (drift/diff details and back-reported status) has been trimmed due to size constraints (%d bytes over limit %d)" ) // refreshWorkStatus refreshes the status of a Work object based on the processing results of its manifests. +// +// TO-DO (chenyu1): refactor this method a bit to reduce its complexity and enable parallelization. func (r *Reconciler) refreshWorkStatus( ctx context.Context, work *fleetv1beta1.Work, @@ -184,6 +192,47 @@ func (r *Reconciler) refreshWorkStatus( setWorkDiffReportedCondition(work, manifestCount, diffReportedObjectsCount) work.Status.ManifestConditions = rebuiltManifestConds + // Perform a size check before the status update. If the Work object goes over the size limit, trim + // some data from its status to ensure that update ops can go through. + // + // Note (chenyu1): at this moment, for simplicity reasons, the trimming op follows a very simple logic: + // if the size limit is breached, the work applier will summarize all drift/diff details in the status, + // and drop all back-reported status data. More sophisticated trimming logic does obviously exist; here + // the controller prefers the simple version primarily for two reasons: + // + // a) in most of the time, it is rare to reach the size limit: KubeFleet's snapshotting mechanism + // tries to keep the total manifest size in a Work object below 800KB (exceptions do exist), which leaves ~600KB + // space for the status data. The work applier reports for each manifest two conditions at most in the + // status (which are all quite small in size), plus the drift/diff details and the back-reported status + // (if applicable); considering the observation that drifts/diffs are not common and their details are usually small + // (just a JSON path plus the before/after values), and the observation that most Kubernetes objects + // only have a few KBs of status data and not all API types need status back-reporting, most of the time + // the Work object should have enough space for status data without trimming; + // b) performing more fine-grained, selective trimming can be a very CPU and memory intensive (e.g. + // various serialization calls) and complex process, and it is difficult to yield optimal results + // even with best efforts. + // + // TO-DO (chenyu1): re-visit this part of the code and evaluate the need for more fine-grained sharding + // if we have users that do use placements of a large collection of manifests and/or very large objects + // with drift/diff detection and status back-reporting on. + // + // TO-DO (chenyu1): evaluate if we need to impose more strict size limits on the manifests to ensure that + // Work objects (almost) always have enough space for status data. + sizeDeltaBytes, err := resource.CalculateSizeDeltaOverLimitFor(work, resource.DefaultObjSizeLimitWithPaddingBytes) + if err != nil { + // Normally this should never occur. + klog.ErrorS(err, "Failed to check Work object size before status update", "work", klog.KObj(work)) + wrappedErr := fmt.Errorf("failed to check work object size before status update: %w", err) + return controller.NewUnexpectedBehaviorError(wrappedErr) + } + if sizeDeltaBytes > 0 { + klog.V(2).InfoS("Must trim status data as the work object has grown over its size limit", + "work", klog.KObj(work), + "sizeDeltaBytes", sizeDeltaBytes, "sizeLimitBytes", resource.DefaultObjSizeLimitWithPaddingBytes) + trimWorkStatusDataWhenOversized(work) + } + setWorkStatusTrimmedCondition(work, sizeDeltaBytes, resource.DefaultObjSizeLimitWithPaddingBytes) + // Update the Work object status. if err := r.hubClient.Status().Update(ctx, work); err != nil { return controller.NewAPIServerError(false, err) @@ -643,3 +692,78 @@ func prepareRebuiltManifestCondQIdx(bundles []*manifestProcessingBundle) map[str } return rebuiltManifestCondQIdx } + +// trimWorkStatusDataWhenOversized trims some data from the Work object status when the object +// reaches its size limit. +func trimWorkStatusDataWhenOversized(work *fleetv1beta1.Work) { + // Trim drift/diff details + back-reported status from the Work object status. + // Replace detailed reportings with a summary if applicable. + for idx := range work.Status.ManifestConditions { + manifestCond := &work.Status.ManifestConditions[idx] + + // Note (chenyu1): check for the second term will always pass; it is added as a sanity check. + if manifestCond.DriftDetails != nil && len(manifestCond.DriftDetails.ObservedDrifts) > 0 { + driftCount := len(manifestCond.DriftDetails.ObservedDrifts) + firstDriftPath := manifestCond.DriftDetails.ObservedDrifts[0].Path + // If there are multiple drifts, report only the path of the first drift plus the count of + // other paths. Also, leave out the specific value differences. + pathSummary := firstDriftPath + if len(manifestCond.DriftDetails.ObservedDrifts) > 1 { + pathSummary = fmt.Sprintf("%s and %d more path(s)", firstDriftPath, driftCount-1) + } + manifestCond.DriftDetails.ObservedDrifts = []fleetv1beta1.PatchDetail{ + { + Path: pathSummary, + ValueInMember: "(omitted)", + ValueInHub: "(omitted)", + }, + } + } + + // Note (chenyu1): check for the second term will always pass; it is added as a sanity check. + if manifestCond.DiffDetails != nil && len(manifestCond.DiffDetails.ObservedDiffs) > 0 { + diffCount := len(manifestCond.DiffDetails.ObservedDiffs) + firstDiffPath := manifestCond.DiffDetails.ObservedDiffs[0].Path + // If there are multiple drifts, report only the path of the first drift plus the count of + // other paths. Also, leave out the specific value differences. + pathSummary := firstDiffPath + if len(manifestCond.DiffDetails.ObservedDiffs) > 1 { + pathSummary = fmt.Sprintf("%s and %d more path(s)", firstDiffPath, diffCount-1) + } + manifestCond.DiffDetails.ObservedDiffs = []fleetv1beta1.PatchDetail{ + { + Path: pathSummary, + ValueInMember: "(omitted)", + ValueInHub: "(omitted)", + }, + } + } + + manifestCond.BackReportedStatus = nil + } +} + +// setWorkStatusTrimmedCondition sets or removes the StatusTrimmed condition on a Work object +// based on whether the status has been trimmed due to it being oversized. +// +// Note (chenyu1): at this moment, due to limitations on the hub agent controller side (some +// controllers assume that placement related conditions are always set in a specific sequence), +// this StatusTrimmed condition might not be exposed on the placement status properly yet. +func setWorkStatusTrimmedCondition(work *fleetv1beta1.Work, sizeDeltaBytes, sizeLimitBytes int) { + if sizeDeltaBytes <= 0 { + // Drop the StatusTrimmed condition if it exists. + if isCondRemoved := meta.RemoveStatusCondition(&work.Status.Conditions, fleetv1beta1.WorkConditionTypeStatusTrimmed); isCondRemoved { + klog.V(2).InfoS("StatusTrimmed condition removed from Work object status", "work", klog.KObj(work)) + } + return + } + + // Set or update the StatusTrimmed condition. + meta.SetStatusCondition(&work.Status.Conditions, metav1.Condition{ + Type: fleetv1beta1.WorkConditionTypeStatusTrimmed, + Status: metav1.ConditionTrue, + Reason: WorkStatusTrimmedDueToOversizedStatusReason, + Message: fmt.Sprintf(WorkStatusTrimmedDueToOversizedStatusMsgTmpl, sizeDeltaBytes, sizeLimitBytes), + ObservedGeneration: work.Generation, + }) +} diff --git a/pkg/controllers/workapplier/status_test.go b/pkg/controllers/workapplier/status_test.go index a8d9f3a96..6fca9b7fe 100644 --- a/pkg/controllers/workapplier/status_test.go +++ b/pkg/controllers/workapplier/status_test.go @@ -26,6 +26,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" "k8s.io/utils/ptr" @@ -2030,3 +2031,721 @@ func TestSetWorkDiffReportedCondition(t *testing.T) { }) } } + +// TestTrimWorkStatusDataWhenOversized tests the trimWorkStatusDataWhenOversized function. +func TestTrimWorkStatusDataWhenOversized(t *testing.T) { + now := metav1.Now() + + testCases := []struct { + name string + work *fleetv1beta1.Work + wantWorkStatus fleetv1beta1.WorkStatus + }{ + { + name: "no drift/diff data, no back-reported status", + work: &fleetv1beta1.Work{ + ObjectMeta: metav1.ObjectMeta{ + Name: workName, + Namespace: memberReservedNSName1, + }, + Status: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAppliedReason, + }, + { + Type: fleetv1beta1.WorkConditionTypeAvailable, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAvailableReason, + }, + }, + ManifestConditions: []fleetv1beta1.ManifestCondition{ + { + Identifier: fleetv1beta1.WorkResourceIdentifier{ + Ordinal: 0, + Group: "", + Version: "v1", + Kind: "Namespace", + Namespace: nsName, + }, + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: string(ApplyOrReportDiffResTypeApplied), + }, + { + Type: fleetv1beta1.WorkConditionTypeAvailable, + Status: metav1.ConditionTrue, + Reason: string(AvailabilityResultTypeAvailable), + }, + }, + }, + }, + }, + }, + wantWorkStatus: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAppliedReason, + }, + { + Type: fleetv1beta1.WorkConditionTypeAvailable, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAvailableReason, + }, + }, + ManifestConditions: []fleetv1beta1.ManifestCondition{ + { + Identifier: fleetv1beta1.WorkResourceIdentifier{ + Ordinal: 0, + Group: "", + Version: "v1", + Kind: "Namespace", + Namespace: nsName, + }, + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: string(ApplyOrReportDiffResTypeApplied), + }, + { + Type: fleetv1beta1.WorkConditionTypeAvailable, + Status: metav1.ConditionTrue, + Reason: string(AvailabilityResultTypeAvailable), + }, + }, + }, + }, + }, + }, + { + name: "trim drifts (single drift)", + work: &fleetv1beta1.Work{ + ObjectMeta: metav1.ObjectMeta{ + Name: workName, + Namespace: memberReservedNSName1, + }, + Status: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionFalse, + Reason: condition.WorkNotAllManifestsAppliedReason, + }, + }, + ManifestConditions: []fleetv1beta1.ManifestCondition{ + { + Identifier: fleetv1beta1.WorkResourceIdentifier{ + Ordinal: 0, + Group: "apps", + Version: "v1", + Kind: "Deployment", + Namespace: deployName, + }, + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionFalse, + Reason: string(ApplyOrReportDiffResTypeFoundDrifts), + }, + }, + DriftDetails: &fleetv1beta1.DriftDetails{ + ObservationTime: now, + FirstDriftedObservedTime: now, + ObservedInMemberClusterGeneration: 1, + ObservedDrifts: []fleetv1beta1.PatchDetail{ + { + Path: fmt.Sprintf("/metadata/labels/%s", dummyLabelKey), + ValueInMember: dummyLabelValue2, + ValueInHub: dummyLabelValue1, + }, + }, + }, + }, + }, + }, + }, + wantWorkStatus: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionFalse, + Reason: condition.WorkNotAllManifestsAppliedReason, + }, + }, + ManifestConditions: []fleetv1beta1.ManifestCondition{ + { + Identifier: fleetv1beta1.WorkResourceIdentifier{ + Ordinal: 0, + Group: "apps", + Version: "v1", + Kind: "Deployment", + Namespace: deployName, + }, + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionFalse, + Reason: string(ApplyOrReportDiffResTypeFoundDrifts), + }, + }, + DriftDetails: &fleetv1beta1.DriftDetails{ + ObservationTime: now, + FirstDriftedObservedTime: now, + ObservedInMemberClusterGeneration: 1, + ObservedDrifts: []fleetv1beta1.PatchDetail{ + { + Path: fmt.Sprintf("/metadata/labels/%s", dummyLabelKey), + ValueInMember: "(omitted)", + ValueInHub: "(omitted)", + }, + }, + }, + }, + }, + }, + }, + { + name: "trim drifts (multiple drifts)", + work: &fleetv1beta1.Work{ + ObjectMeta: metav1.ObjectMeta{ + Name: workName, + Namespace: memberReservedNSName1, + }, + Status: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionFalse, + Reason: condition.WorkNotAllManifestsAppliedReason, + }, + }, + ManifestConditions: []fleetv1beta1.ManifestCondition{ + { + Identifier: fleetv1beta1.WorkResourceIdentifier{ + Ordinal: 0, + Group: "apps", + Version: "v1", + Kind: "Deployment", + Namespace: deployName, + }, + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionFalse, + Reason: string(ApplyOrReportDiffResTypeFoundDrifts), + }, + }, + DriftDetails: &fleetv1beta1.DriftDetails{ + ObservationTime: now, + FirstDriftedObservedTime: now, + ObservedInMemberClusterGeneration: 1, + ObservedDrifts: []fleetv1beta1.PatchDetail{ + { + Path: fmt.Sprintf("/metadata/labels/%s-1", dummyLabelKey), + ValueInMember: dummyLabelValue2, + ValueInHub: dummyLabelValue1, + }, + { + Path: fmt.Sprintf("/metadata/labels/%s-2", dummyLabelKey), + ValueInMember: dummyLabelValue2, + ValueInHub: dummyLabelValue1, + }, + }, + }, + }, + }, + }, + }, + wantWorkStatus: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionFalse, + Reason: condition.WorkNotAllManifestsAppliedReason, + }, + }, + ManifestConditions: []fleetv1beta1.ManifestCondition{ + { + Identifier: fleetv1beta1.WorkResourceIdentifier{ + Ordinal: 0, + Group: "apps", + Version: "v1", + Kind: "Deployment", + Namespace: deployName, + }, + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionFalse, + Reason: string(ApplyOrReportDiffResTypeFoundDrifts), + }, + }, + DriftDetails: &fleetv1beta1.DriftDetails{ + ObservationTime: now, + FirstDriftedObservedTime: now, + ObservedInMemberClusterGeneration: 1, + ObservedDrifts: []fleetv1beta1.PatchDetail{ + { + Path: fmt.Sprintf("/metadata/labels/%s-1 and %d more path(s)", dummyLabelKey, 1), + ValueInMember: "(omitted)", + ValueInHub: "(omitted)", + }, + }, + }, + }, + }, + }, + }, + { + name: "trim diffs (single diff)", + work: &fleetv1beta1.Work{ + ObjectMeta: metav1.ObjectMeta{ + Name: workName, + Namespace: memberReservedNSName1, + }, + Status: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeDiffReported, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsDiffReportedReason, + }, + }, + ManifestConditions: []fleetv1beta1.ManifestCondition{ + { + Identifier: fleetv1beta1.WorkResourceIdentifier{ + Ordinal: 0, + Group: "apps", + Version: "v1", + Kind: "Deployment", + Namespace: deployName, + }, + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeDiffReported, + Status: metav1.ConditionTrue, + Reason: string(ApplyOrReportDiffResTypeFoundDiff), + }, + }, + DiffDetails: &fleetv1beta1.DiffDetails{ + ObservationTime: now, + FirstDiffedObservedTime: now, + ObservedInMemberClusterGeneration: ptr.To(int64(1)), + ObservedDiffs: []fleetv1beta1.PatchDetail{ + { + Path: fmt.Sprintf("/metadata/labels/%s", dummyLabelKey), + ValueInMember: dummyLabelValue2, + ValueInHub: dummyLabelValue1, + }, + }, + }, + }, + }, + }, + }, + wantWorkStatus: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeDiffReported, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsDiffReportedReason, + }, + }, + ManifestConditions: []fleetv1beta1.ManifestCondition{ + { + Identifier: fleetv1beta1.WorkResourceIdentifier{ + Ordinal: 0, + Group: "apps", + Version: "v1", + Kind: "Deployment", + Namespace: deployName, + }, + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeDiffReported, + Status: metav1.ConditionTrue, + Reason: string(ApplyOrReportDiffResTypeFoundDiff), + }, + }, + DiffDetails: &fleetv1beta1.DiffDetails{ + ObservationTime: now, + FirstDiffedObservedTime: now, + ObservedInMemberClusterGeneration: ptr.To(int64(1)), + ObservedDiffs: []fleetv1beta1.PatchDetail{ + { + Path: fmt.Sprintf("/metadata/labels/%s", dummyLabelKey), + ValueInMember: "(omitted)", + ValueInHub: "(omitted)", + }, + }, + }, + }, + }, + }, + }, + { + name: "trim diffs (multiple diffs)", + work: &fleetv1beta1.Work{ + ObjectMeta: metav1.ObjectMeta{ + Name: workName, + Namespace: memberReservedNSName1, + }, + Status: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeDiffReported, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsDiffReportedReason, + }, + }, + ManifestConditions: []fleetv1beta1.ManifestCondition{ + { + Identifier: fleetv1beta1.WorkResourceIdentifier{ + Ordinal: 0, + Group: "apps", + Version: "v1", + Kind: "Deployment", + Namespace: deployName, + }, + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeDiffReported, + Status: metav1.ConditionTrue, + Reason: string(ApplyOrReportDiffResTypeFoundDiff), + }, + }, + DiffDetails: &fleetv1beta1.DiffDetails{ + ObservationTime: now, + FirstDiffedObservedTime: now, + ObservedInMemberClusterGeneration: ptr.To(int64(1)), + ObservedDiffs: []fleetv1beta1.PatchDetail{ + { + Path: fmt.Sprintf("/metadata/labels/%s-1", dummyLabelKey), + ValueInMember: dummyLabelValue2, + ValueInHub: dummyLabelValue1, + }, + { + Path: fmt.Sprintf("/metadata/labels/%s-2", dummyLabelKey), + ValueInMember: dummyLabelValue2, + ValueInHub: dummyLabelValue1, + }, + }, + }, + }, + }, + }, + }, + wantWorkStatus: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeDiffReported, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsDiffReportedReason, + }, + }, + ManifestConditions: []fleetv1beta1.ManifestCondition{ + { + Identifier: fleetv1beta1.WorkResourceIdentifier{ + Ordinal: 0, + Group: "apps", + Version: "v1", + Kind: "Deployment", + Namespace: deployName, + }, + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeDiffReported, + Status: metav1.ConditionTrue, + Reason: string(ApplyOrReportDiffResTypeFoundDiff), + }, + }, + DiffDetails: &fleetv1beta1.DiffDetails{ + ObservationTime: now, + FirstDiffedObservedTime: now, + ObservedInMemberClusterGeneration: ptr.To(int64(1)), + ObservedDiffs: []fleetv1beta1.PatchDetail{ + { + Path: fmt.Sprintf("/metadata/labels/%s-1 and %d more path(s)", dummyLabelKey, 1), + ValueInMember: "(omitted)", + ValueInHub: "(omitted)", + }, + }, + }, + }, + }, + }, + }, + { + name: "trim back-reported status", + work: &fleetv1beta1.Work{ + ObjectMeta: metav1.ObjectMeta{ + Name: workName, + Namespace: memberReservedNSName1, + }, + Status: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAppliedReason, + }, + { + Type: fleetv1beta1.WorkConditionTypeAvailable, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAvailableReason, + }, + }, + ManifestConditions: []fleetv1beta1.ManifestCondition{ + { + Identifier: fleetv1beta1.WorkResourceIdentifier{ + Ordinal: 0, + Group: "apps", + Version: "v1", + Kind: "Deployment", + Namespace: deployName, + }, + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: string(ApplyOrReportDiffResTypeApplied), + }, + { + Type: fleetv1beta1.WorkConditionTypeAvailable, + Status: metav1.ConditionTrue, + Reason: string(AvailabilityResultTypeAvailable), + }, + }, + BackReportedStatus: &fleetv1beta1.BackReportedStatus{ + ObservationTime: now, + ObservedStatus: runtime.RawExtension{ + Raw: []byte(dummyLabelValue1), + }, + }, + }, + }, + }, + }, + wantWorkStatus: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAppliedReason, + }, + { + Type: fleetv1beta1.WorkConditionTypeAvailable, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAvailableReason, + }, + }, + ManifestConditions: []fleetv1beta1.ManifestCondition{ + { + Identifier: fleetv1beta1.WorkResourceIdentifier{ + Ordinal: 0, + Group: "apps", + Version: "v1", + Kind: "Deployment", + Namespace: deployName, + }, + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: string(ApplyOrReportDiffResTypeApplied), + }, + { + Type: fleetv1beta1.WorkConditionTypeAvailable, + Status: metav1.ConditionTrue, + Reason: string(AvailabilityResultTypeAvailable), + }, + }, + }, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + trimWorkStatusDataWhenOversized(tc.work) + if diff := cmp.Diff(tc.work.Status, tc.wantWorkStatus); diff != "" { + t.Errorf("trimmed work status mismatches (-got, +want):\n%s", diff) + } + }) + } +} + +// TestSetWorkStatusTrimmedCondition tests the setWorkStatusTrimmedCondition function. +func TestSetWorkStatusTrimmedCondition(t *testing.T) { + testCases := []struct { + name string + work *fleetv1beta1.Work + sizeDeltaBytes int + sizeLimitBytes int + wantWorkStatusConditions []metav1.Condition + }{ + { + name: "no trimming needed (size delta <= 0)", + work: &fleetv1beta1.Work{ + ObjectMeta: metav1.ObjectMeta{ + Name: workName, + Generation: 1, + }, + Status: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAppliedReason, + ObservedGeneration: 1, + }, + }, + }, + }, + sizeDeltaBytes: 0, + sizeLimitBytes: 1024, + wantWorkStatusConditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAppliedReason, + ObservedGeneration: 1, + }, + }, + }, + { + name: "remove existing StatusTrimmed condition when size delta <= 0", + work: &fleetv1beta1.Work{ + ObjectMeta: metav1.ObjectMeta{ + Name: workName, + Generation: 2, + }, + Status: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAppliedReason, + ObservedGeneration: 2, + }, + { + Type: fleetv1beta1.WorkConditionTypeStatusTrimmed, + Status: metav1.ConditionTrue, + Reason: WorkStatusTrimmedDueToOversizedStatusReason, + Message: fmt.Sprintf(WorkStatusTrimmedDueToOversizedStatusMsgTmpl, 500, 1024), + ObservedGeneration: 1, + }, + }, + }, + }, + sizeDeltaBytes: 0, + sizeLimitBytes: 1024, + wantWorkStatusConditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAppliedReason, + ObservedGeneration: 2, + }, + }, + }, + { + name: "set StatusTrimmed condition when size delta > 0", + work: &fleetv1beta1.Work{ + ObjectMeta: metav1.ObjectMeta{ + Name: workName, + Generation: 1, + }, + Status: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAppliedReason, + ObservedGeneration: 1, + }, + }, + }, + }, + sizeDeltaBytes: 500, + sizeLimitBytes: 1024, + wantWorkStatusConditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAppliedReason, + ObservedGeneration: 1, + }, + { + Type: fleetv1beta1.WorkConditionTypeStatusTrimmed, + Status: metav1.ConditionTrue, + Reason: WorkStatusTrimmedDueToOversizedStatusReason, + Message: fmt.Sprintf(WorkStatusTrimmedDueToOversizedStatusMsgTmpl, 500, 1024), + ObservedGeneration: 1, + }, + }, + }, + { + name: "update existing StatusTrimmed condition with new values", + work: &fleetv1beta1.Work{ + ObjectMeta: metav1.ObjectMeta{ + Name: workName, + Generation: 2, + }, + Status: fleetv1beta1.WorkStatus{ + Conditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAppliedReason, + ObservedGeneration: 2, + }, + { + Type: fleetv1beta1.WorkConditionTypeStatusTrimmed, + Status: metav1.ConditionTrue, + Reason: WorkStatusTrimmedDueToOversizedStatusReason, + Message: fmt.Sprintf(WorkStatusTrimmedDueToOversizedStatusMsgTmpl, 200, 1024), + ObservedGeneration: 1, + }, + }, + }, + }, + sizeDeltaBytes: 750, + sizeLimitBytes: 2048, + wantWorkStatusConditions: []metav1.Condition{ + { + Type: fleetv1beta1.WorkConditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: condition.WorkAllManifestsAppliedReason, + ObservedGeneration: 2, + }, + { + Type: fleetv1beta1.WorkConditionTypeStatusTrimmed, + Status: metav1.ConditionTrue, + Reason: WorkStatusTrimmedDueToOversizedStatusReason, + Message: fmt.Sprintf(WorkStatusTrimmedDueToOversizedStatusMsgTmpl, 750, 2048), + ObservedGeneration: 2, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + setWorkStatusTrimmedCondition(tc.work, tc.sizeDeltaBytes, tc.sizeLimitBytes) + if diff := cmp.Diff( + tc.work.Status.Conditions, tc.wantWorkStatusConditions, + ignoreFieldConditionLTTMsg, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("work status conditions mismatches (-got, +want):\n%s", diff) + } + }) + } +} diff --git a/pkg/utils/resource/resource.go b/pkg/utils/resource/resource.go index 0f514d8ab..405a08f97 100644 --- a/pkg/utils/resource/resource.go +++ b/pkg/utils/resource/resource.go @@ -21,6 +21,14 @@ import ( "crypto/sha256" "encoding/json" "fmt" + + "k8s.io/apimachinery/pkg/runtime" +) + +const ( + // etcd has a 1.5 MiB limit for objects by default, and Kubernetes clients might + // reject request entities too large (~2/~3 MiB, depending on the protocol in use). + DefaultObjSizeLimitWithPaddingBytes = 1415578 // 1.35 MiB, or ~1.42 MB. ) // HashOf returns the hash of the resource. @@ -31,3 +39,20 @@ func HashOf(resource any) (string, error) { } return fmt.Sprintf("%x", sha256.Sum256(jsonBytes)), nil } + +// CalculateSizeDeltaOverLimitFor calculates the size delta in bytes of a given object +// over a specified size limit. It returns a positive value if the object size exceeds +// the limit or a negative value if the object size is below the limit. +// +// This utility is useful in cases where KubeFleet needs to check if it can create/update +// an object with additional information. +func CalculateSizeDeltaOverLimitFor(obj runtime.Object, sizeLimitBytes int) (int, error) { + jsonBytes, err := json.Marshal(obj) + if err != nil { + return 0, fmt.Errorf("cannot determine object size: %w", err) + } + if sizeLimitBytes < 0 { + return 0, fmt.Errorf("size limit must be non-negative") + } + return len(jsonBytes) - sizeLimitBytes, nil +} diff --git a/pkg/utils/resource/resource_test.go b/pkg/utils/resource/resource_test.go index bfc8418a7..75dc6f854 100644 --- a/pkg/utils/resource/resource_test.go +++ b/pkg/utils/resource/resource_test.go @@ -17,9 +17,13 @@ limitations under the License. package resource import ( + "encoding/json" "testing" + "github.com/google/go-cmp/cmp" placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestHashOf(t *testing.T) { @@ -51,3 +55,65 @@ func TestHashOf(t *testing.T) { }) } } + +// TestCalculateSizeDeltaOverLimitFor tests the CalculateSizeDeltaOverLimitFor function. +func TestCalculateSizeDeltaOverLimitFor(t *testing.T) { + cm := &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "app", + Namespace: "default", + }, + Data: map[string]string{ + "key": "value", + }, + } + cmBytes, err := json.Marshal(cm) + if err != nil { + t.Fatalf("Failed to marshal configMap") + } + cmSizeBytes := len(cmBytes) + + testCases := []struct { + name string + sizeLimitBytes int + wantErred bool + }{ + { + name: "under size limit (negative delta)", + sizeLimitBytes: 10000, + }, + { + name: "over size limit (positive delta)", + sizeLimitBytes: 1, + }, + { + name: "invalid size limit (negative size limit)", + // Invalid size limit. + sizeLimitBytes: -1, + wantErred: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + sizeDeltaBytes, err := CalculateSizeDeltaOverLimitFor(cm, tc.sizeLimitBytes) + + if tc.wantErred { + if err == nil { + t.Fatalf("CalculateSizeDeltaOverLimitFor() error = nil, want erred") + } + return + } + // Note: this test spec uses runtime calculation rather than static values for expected + // size delta comparison as different platforms have slight differences in the serialization process, + // which may produce different sizing results. + if !cmp.Equal(sizeDeltaBytes, cmSizeBytes-tc.sizeLimitBytes) { + t.Errorf("CalculateSizeDeltaOverLimitFor() = %d, want %d", sizeDeltaBytes, cmSizeBytes-tc.sizeLimitBytes) + } + }) + } +}