Skip to content

Commit 69532f7

Browse files
efiacorefiacor
authored
Add PrrSizeBytes to v1alpha2 PackageRevision status (#977)
* Add PrrSizeBytes to v1alpha2 PackageRevision status Both PR controller and repo controller now calculate and set status.prrSizeBytes on v1alpha2 CRDs from the in-memory resource map. Covers init, clone, push/render, and repo discovery paths. E2E tests use relative assertions (> 0, > initial) for resilience. Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech> * Update PrrSizeBytes → ResourcesSizeBytes Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech> * Address copilot and make generate Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech> --------- Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech>
1 parent e82aacf commit 69532f7

13 files changed

Lines changed: 155 additions & 11 deletions

File tree

‎api/generated/openapi/zz_generated.openapi.go‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎controllers/config/crd/bases/config.porch.kpt.dev_packagevariants.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ spec:
444444
properties:
445445
exec:
446446
description: |-
447-
ExecPath is the the absolute os-specific path to the executable file
447+
ExecPath is the absolute OS-specific path to the executable file.
448448
If user provides an executable file with commands, ExecPath should
449449
contain the entire input string.
450450
type: string

‎controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller_test.go‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ import (
66
"testing"
77
"time"
88

9+
kptfilev1 "github.com/kptdev/kpt/pkg/api/kptfile/v1"
910
"github.com/stretchr/testify/assert"
1011
"github.com/stretchr/testify/mock"
1112
"github.com/stretchr/testify/require"
12-
kptfilev1 "github.com/kptdev/kpt/pkg/api/kptfile/v1"
1313
apierrors "k8s.io/apimachinery/pkg/api/errors"
1414
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1515
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -31,6 +31,7 @@ func setupMockContentDefaults(m *mockrepository.MockPackageContent) {
3131
m.EXPECT().GetCommitInfo().Return(time.Time{}, "").Maybe()
3232
m.EXPECT().GetLock(mock.Anything).Return(kptfilev1.Upstream{}, kptfilev1.Locator{}, nil).Maybe()
3333
m.EXPECT().GetUpstreamLock(mock.Anything).Return(kptfilev1.Upstream{}, kptfilev1.Locator{}, nil).Maybe()
34+
m.EXPECT().GetResourceContents(mock.Anything).Return(map[string]string{"Kptfile": "test"}, nil).Maybe()
3435
}
3536

3637
func newTestReconciler(mockClient *mockclient.MockClient, cache *mockrepository.MockContentCache) *PackageRevisionReconciler {
@@ -385,7 +386,7 @@ func TestReconcileDeletionProposedNoFinalizer(t *testing.T) {
385386
*obj.(*porchv1alpha2.PackageRevision) = *pr
386387
}).Return(nil)
387388
// No Patch expected — finalizer already absent.
388-
389+
389390
// updateLatestRevisionLabels is called after finalizer removal
390391
mockClient.EXPECT().List(mock.Anything, mock.AnythingOfType("*v1alpha2.PackageRevisionList"), mock.Anything, mock.Anything).Return(nil)
391392

@@ -715,7 +716,7 @@ func TestReconcileOwnerRefRepoLookupFails(t *testing.T) {
715716

716717
pr := &porchv1alpha2.PackageRevision{
717718
ObjectMeta: metav1.ObjectMeta{Name: "test-pr", Namespace: "default"},
718-
Spec: porchv1alpha2.PackageRevisionSpec{RepositoryName: "missing-repo"},
719+
Spec: porchv1alpha2.PackageRevisionSpec{RepositoryName: "missing-repo"},
719720
}
720721

721722
mockClient := mockclient.NewMockClient(t)
@@ -1206,7 +1207,6 @@ func TestReconcileNoSource(t *testing.T) {
12061207
assert.Equal(t, ctrl.Result{}, result)
12071208
}
12081209

1209-
12101210
// mockRenderer is a test double for the renderer interface.
12111211
type mockRenderer struct {
12121212
resources map[string]string
@@ -1294,7 +1294,6 @@ func TestReconcileRenderAlreadyRendered(t *testing.T) {
12941294
assert.Nil(t, result)
12951295
}
12961296

1297-
12981297
func TestReconcileRenderSourceTrigger(t *testing.T) {
12991298
ctx := t.Context()
13001299
rendered := map[string]string{"Kptfile": "rendered"}
@@ -1553,7 +1552,6 @@ func TestReconcileSourceCloseDraftFails(t *testing.T) {
15531552
assert.Equal(t, ctrl.Result{}, result)
15541553
}
15551554

1556-
15571555
func TestReconcileRenderErrorSetsStatus(t *testing.T) {
15581556
// Reconcile should handle reconcileRender returning an error
15591557
// by logging and returning (no crash, no requeue).
@@ -1665,7 +1663,6 @@ func TestWriteRenderedResourcesCloseDraftFails(t *testing.T) {
16651663
assert.Contains(t, err.Error(), "close draft after render")
16661664
}
16671665

1668-
16691666
func TestReconcileRenderPipelineFailureNoPush(t *testing.T) {
16701667
ctx := t.Context()
16711668

@@ -1760,7 +1757,6 @@ func TestReconcileRenderPipelineFailureWithPushWriteFails(t *testing.T) {
17601757
assert.Nil(t, result)
17611758
}
17621759

1763-
17641760
func TestRenderWithConcurrencyLimitRequeues(t *testing.T) {
17651761
limiter := make(chan struct{}, 1)
17661762
limiter <- struct{}{} // fill the limiter

‎controllers/packagerevisions/pkg/controllers/packagerevision/status.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ func (r *PackageRevisionReconciler) updateStatus(ctx context.Context, pr *porchv
6161
if _, upstreamLock, err := content.GetUpstreamLock(ctx); err == nil {
6262
status.UpstreamLock = porchv1alpha2.KptLocatorToLocator(upstreamLock)
6363
}
64+
if resources, err := content.GetResourceContents(ctx); err == nil {
65+
status.ResourcesSizeBytes = repository.CalculateResourcesSize(resources)
66+
}
6467
}
6568

6669
applyObj := &porchv1alpha2.PackageRevision{

‎controllers/packagerevisions/pkg/controllers/packagerevision/status_test.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ func TestUpdateStatusWithPublishedContent(t *testing.T) {
8888
content.EXPECT().GetCommitInfo().Return(commitTime, "user@example.com")
8989
content.EXPECT().GetLock(mock.Anything).Return(kptfilev1.Upstream{}, kptfilev1.Locator{}, nil)
9090
content.EXPECT().GetUpstreamLock(mock.Anything).Return(kptfilev1.Upstream{}, kptfilev1.Locator{}, nil)
91+
content.EXPECT().GetResourceContents(mock.Anything).Return(map[string]string{"Kptfile": "abc", "cm.yaml": "defgh"}, nil)
9192

9293
r := &PackageRevisionReconciler{Client: mockClient}
9394
pr := basePR()
@@ -99,6 +100,7 @@ func TestUpdateStatusWithPublishedContent(t *testing.T) {
99100
assert.Equal(t, 5, captured.Revision)
100101
assert.Equal(t, "user@example.com", captured.PublishedBy)
101102
assert.NotNil(t, captured.PublishedAt)
103+
assert.Equal(t, int64(8), captured.ResourcesSizeBytes)
102104
}
103105

104106
func TestUpdateStatusWithDraftContent(t *testing.T) {
@@ -109,6 +111,7 @@ func TestUpdateStatusWithDraftContent(t *testing.T) {
109111
content.EXPECT().Lifecycle(mock.Anything).Return("Draft")
110112
content.EXPECT().GetLock(mock.Anything).Return(kptfilev1.Upstream{}, kptfilev1.Locator{}, nil)
111113
content.EXPECT().GetUpstreamLock(mock.Anything).Return(kptfilev1.Upstream{}, kptfilev1.Locator{}, nil)
114+
content.EXPECT().GetResourceContents(mock.Anything).Return(map[string]string{"Kptfile": "draft-content"}, nil)
112115

113116
r := &PackageRevisionReconciler{Client: mockClient}
114117
pr := basePR()
@@ -118,6 +121,7 @@ func TestUpdateStatusWithDraftContent(t *testing.T) {
118121
assert.Equal(t, 0, captured.Revision)
119122
assert.Empty(t, captured.PublishedBy)
120123
assert.Nil(t, captured.PublishedAt)
124+
assert.Equal(t, int64(13), captured.ResourcesSizeBytes)
121125
}
122126

123127
func TestUpdateRenderStatusInProgress(t *testing.T) {
@@ -180,7 +184,6 @@ func TestSetRenderFailed(t *testing.T) {
180184
assert.Equal(t, porchv1alpha2.ReasonRenderFailed, renderPatch.Conditions[0].Reason)
181185
}
182186

183-
184187
func TestUpdateKptfileFields(t *testing.T) {
185188
mockClient := mockclient.NewMockClient(t)
186189

‎controllers/repositories/pkg/controllers/repository/pkgrevsync.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,21 @@ func (r *RepositoryReconciler) applySeedFields(ctx context.Context, repo *config
199199
func packageRevisionUpToDate(existing, desired *porchv1alpha2.PackageRevision) bool {
200200
return equality.Semantic.DeepEqual(existing.Labels, desired.Labels) &&
201201
existing.Status.Deployment == desired.Status.Deployment &&
202+
resourcesSizeBytesUpToDate(existing.Status.ResourcesSizeBytes, desired.Status.ResourcesSizeBytes) &&
202203
equality.Semantic.DeepEqual(existing.Status.UpstreamLock, desired.Status.UpstreamLock) &&
203204
equality.Semantic.DeepEqual(existing.Status.SelfLock, desired.Status.SelfLock)
204205
}
205206

207+
// resourcesSizeBytesUpToDate returns true if the size field doesn't need updating.
208+
// When desired is 0 (size couldn't be computed, e.g. GetResources failed), we
209+
// treat the existing value as up-to-date to avoid patch churn.
210+
func resourcesSizeBytesUpToDate(existing, desired int64) bool {
211+
if desired == 0 {
212+
return true
213+
}
214+
return existing == desired
215+
}
216+
206217
// buildPackageRevision constructs a PackageRevision resource containing only
207218
// repo-controller-owned fields: identity, labels, ownerRef, locks, deployment.
208219
// Seed fields (lifecycle, publish metadata, Kptfile-derived) are applied
@@ -219,6 +230,11 @@ func buildPackageRevision(ctx context.Context, repo *configapi.Repository, pkgRe
219230
// PackageConditions omitted — PR controller owns after first render.
220231
}
221232

233+
// Calculate resource size for status.resourcesSizeBytes.
234+
if prr, err := pkgRev.GetResources(ctx); err == nil && prr != nil && prr.Spec.Resources != nil {
235+
status.ResourcesSizeBytes = repository.CalculateResourcesSize(prr.Spec.Resources)
236+
}
237+
222238
crd := &porchv1alpha2.PackageRevision{
223239
TypeMeta: metav1.TypeMeta{
224240
Kind: "PackageRevision",

‎controllers/repositories/pkg/controllers/repository/pkgrevsync_test.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ type fakePackageRevision struct {
9191
commitTime time.Time
9292
commitAuthor string
9393
isLatest bool
94+
resources map[string]string
9495
}
9596

9697
func (f *fakePackageRevision) KubeObjectNamespace() string { return f.key.RKey().Namespace }
@@ -110,6 +111,13 @@ func (f *fakePackageRevision) GetPackageRevision(_ context.Context) (*porchv1alp
110111
return nil, nil
111112
}
112113
func (f *fakePackageRevision) GetResources(_ context.Context) (*porchv1alpha1.PackageRevisionResources, error) {
114+
if f.resources != nil {
115+
return &porchv1alpha1.PackageRevisionResources{
116+
Spec: porchv1alpha1.PackageRevisionResourcesSpec{
117+
Resources: f.resources,
118+
},
119+
}, nil
120+
}
113121
return nil, nil
114122
}
115123
func (f *fakePackageRevision) GetUpstreamLock(_ context.Context) (kptfilev1.Upstream, kptfilev1.Locator, error) {
@@ -219,6 +227,27 @@ func TestBuildPackageRevision(t *testing.T) {
219227
assert.Nil(t, crd.Status.UpstreamLock)
220228
assert.Nil(t, crd.Status.SelfLock)
221229
})
230+
231+
t.Run("ResourcesSizeBytes calculated from resources", func(t *testing.T) {
232+
pkgRev := newFakePkgRev("sized-pkg", "ws1", porchv1alpha2.PackageRevisionLifecyclePublished)
233+
pkgRev.resources = map[string]string{
234+
"Kptfile": "abc", // 3 bytes
235+
"cm.yaml": "defgh", // 5 bytes
236+
"ns.yaml": "ij", // 2 bytes
237+
}
238+
239+
crd, err := buildPackageRevision(ctx, repo, pkgRev)
240+
assert.NoError(t, err)
241+
assert.Equal(t, int64(10), crd.Status.ResourcesSizeBytes)
242+
})
243+
244+
t.Run("ResourcesSizeBytes zero when no resources", func(t *testing.T) {
245+
pkgRev := newFakePkgRev("empty-pkg", "ws1", porchv1alpha2.PackageRevisionLifecycleDraft)
246+
247+
crd, err := buildPackageRevision(ctx, repo, pkgRev)
248+
assert.NoError(t, err)
249+
assert.Equal(t, int64(0), crd.Status.ResourcesSizeBytes)
250+
})
222251
}
223252

224253
// --- Tests: packageRevisionUpToDate ---
@@ -253,6 +282,9 @@ func TestPackageRevisionUpToDate(t *testing.T) {
253282
{name: "annotations differ - still up to date", modify: func(pr *porchv1alpha2.PackageRevision) {
254283
pr.Annotations = map[string]string{"foo": "bar"}
255284
}, expected: true},
285+
{name: "ResourcesSizeBytes changed", modify: func(pr *porchv1alpha2.PackageRevision) {
286+
pr.Status.ResourcesSizeBytes = 12345
287+
}, expected: false},
256288
}
257289

258290
for _, tt := range tests {
@@ -266,6 +298,26 @@ func TestPackageRevisionUpToDate(t *testing.T) {
266298
}
267299
}
268300

301+
func TestResourcesSizeBytesUpToDate(t *testing.T) {
302+
tests := []struct {
303+
name string
304+
existing int64
305+
desired int64
306+
expected bool
307+
}{
308+
{name: "both zero", existing: 0, desired: 0, expected: true},
309+
{name: "equal non-zero", existing: 100, desired: 100, expected: true},
310+
{name: "desired changed", existing: 100, desired: 200, expected: false},
311+
{name: "desired zero (unknown) - skip comparison", existing: 500, desired: 0, expected: true},
312+
{name: "existing zero, desired non-zero", existing: 0, desired: 100, expected: false},
313+
}
314+
for _, tt := range tests {
315+
t.Run(tt.name, func(t *testing.T) {
316+
assert.Equal(t, tt.expected, resourcesSizeBytesUpToDate(tt.existing, tt.desired))
317+
})
318+
}
319+
}
320+
269321
// --- Tests: packageRevisionLabels ---
270322

271323
func TestPackageRevisionLabels(t *testing.T) {

‎pkg/repository/util.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,16 @@ func PackageRevisionIsPlaceholder(ctx context.Context, namespace string, referen
212212
return false, nil
213213
}
214214

215+
// CalculateResourcesSize returns the total byte size of a package's resource
216+
// file contents. This is used to populate status.resourcesSizeBytes on the CRD.
217+
func CalculateResourcesSize(resources map[string]string) int64 {
218+
var total int64
219+
for _, v := range resources {
220+
total += int64(len(v))
221+
}
222+
return total
223+
}
224+
215225
func WriteResourcesToFS(fs filesys.FileSystem, rootDir string, resources map[string]string) (string, error) {
216226
if rootDir != "" {
217227
if err := fs.MkdirAll(rootDir); err != nil {

‎pkg/repository/util_test.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,51 @@ func TestPathsOverlap(t *testing.T) {
217217
assert.False(t, PathsOverlap("pkg", "pkg-other"))
218218
}
219219

220+
func TestCalculateResourcesSize(t *testing.T) {
221+
tests := []struct {
222+
name string
223+
resources map[string]string
224+
want int64
225+
}{
226+
{
227+
name: "nil map",
228+
resources: nil,
229+
want: 0,
230+
},
231+
{
232+
name: "empty map",
233+
resources: map[string]string{},
234+
want: 0,
235+
},
236+
{
237+
name: "single file",
238+
resources: map[string]string{"Kptfile": "hello"},
239+
want: 5,
240+
},
241+
{
242+
name: "multiple files",
243+
resources: map[string]string{
244+
"Kptfile": "abc",
245+
"cm.yaml": "defgh",
246+
"nested.txt": "ij",
247+
},
248+
want: 10,
249+
},
250+
{
251+
name: "multi-byte UTF-8 characters",
252+
resources: map[string]string{"file.yaml": "héllo"}, // é is 2 bytes in UTF-8
253+
want: 6,
254+
},
255+
}
256+
257+
for _, tt := range tests {
258+
t.Run(tt.name, func(t *testing.T) {
259+
got := CalculateResourcesSize(tt.resources)
260+
assert.Equal(t, tt.want, got)
261+
})
262+
}
263+
}
264+
220265
func TestValidatePackagePathOverlap(t *testing.T) {
221266
newPr := &porchapi.PackageRevision{
222267
Spec: porchapi.PackageRevisionSpec{

‎test/e2e/crd/clone_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ var _ = Describe("Clone", Ordered, Label("lifecycle"), func() {
5858
Expect(pr.Status.SelfLock).NotTo(BeNil())
5959
Expect(pr.Status.SelfLock.Git).NotTo(BeNil())
6060
Expect(pr.Status.SelfLock.Git.Commit).NotTo(BeEmpty())
61+
62+
By("verifying ResourcesSizeBytes is populated")
63+
Expect(pr.Status.ResourcesSizeBytes).To(BeNumerically(">", int64(0)))
6164
})
6265

6366
It("should clone into a deployment repository", func() {

0 commit comments

Comments
 (0)