Skip to content

Add package resource file sizes to PackageRevision interface#954

Open
JamesMcDermott wants to merge 19 commits into
kptdev:mainfrom
Nordix:feature-fetch-size-limit
Open

Add package resource file sizes to PackageRevision interface#954
JamesMcDermott wants to merge 19 commits into
kptdev:mainfrom
Nordix:feature-fetch-size-limit

Conversation

@JamesMcDermott
Copy link
Copy Markdown
Contributor

@JamesMcDermott JamesMcDermott commented Apr 28, 2026

Description

  • What changed:
    • PackageRevision type - added a prrSizeBytes field to PackageRevisionStatus in both v1alpha1 (existing) and v1alpha2 (in-progress CRD migration) APIs
    • On getting/listing PackageRevisions, the total resources size is included in the status field of the returned API object
  • Why it's needed:
    • Provides visibility into the total file size of a package revision's resources, so users can understand storage usage before actually pulling/downloading the package revision resources.
  • How it works:
    • New resources_size INTEGER column to the package_revisions database table
      • Wiring in SQL queries (read, write, update) to include the new resources_size column
      • SQL upgrade/downgrade scripts to add-and-populate or remove column on version change across boundary of this PR
        • needs to repopulate column on upgrade to cover case where package revisions are already present
          • resource file contents will be cached, but resources_size will be 0
          • sync will not pick up this discrepancy without intervention
    • When a draft is closed, the sizes of all individual resource file strings are summed and stored in resources_size
    • When package revisions are retrieved, the stored size is returned in the prrSizeBytes status field

Related Issue(s)


Type of Change

  • Bug fix
  • New feature
  • Enhancement
  • Refactor
  • Documentation
  • Tests
  • Other: ________

Checklist

  • Code follows project style guidelines
  • Self-reviewed changes
  • Tests added/updated
  • Documentation added/updated
  • All tests and gating checks pass

AI Disclosure

  • I have used AI in the creation of this PR.

Examples:

  • Amazon Q to:
    • provide SQL in upgrade scripts to calculate and populate resources_size column on upgrade
    • identify locations in documentation to update
      • actual updates written by hand
    • generate initial seed of this PR description by filling out PULL_REQUEST_TEMPLATE.md

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 28, 2026

Deploy Preview for porch ready!

Name Link
🔨 Latest commit 2c701d9
🔍 Latest deploy log https://app.netlify.com/projects/porch/deploys/6a035d179c05790008c9b54b
😎 Deploy Preview https://deploy-preview-954--porch.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@JamesMcDermott JamesMcDermott force-pushed the feature-fetch-size-limit branch 2 times, most recently from 0830aa2 to 9a4dacc Compare April 29, 2026 15:42
Comment thread api/sql/porch-db.sql Outdated
@nagygergo
Copy link
Copy Markdown
Contributor

Issue that it fixes is incorrect.

Comment thread api/sql/porch-db.sql
Comment thread api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml Outdated
Comment thread api/porch/v1alpha2/packagerevision_types.go Outdated
Comment thread docs/content/en/docs/2_concepts/package-revisions.md
@JamesMcDermott
Copy link
Copy Markdown
Contributor Author

Issue that it fixes is incorrect.

Corrected - good spot, thanks.

@JamesMcDermott JamesMcDermott force-pushed the feature-fetch-size-limit branch from 29b7a4f to 0be6e34 Compare May 6, 2026 14:50
Copy link
Copy Markdown
Collaborator

@efiacor efiacor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall.
I think we need to add some size checks for clone and edit e2e flows.
Maybe using assert.Greater() to avoid having to calculate the exact sizes.

Comment thread test/e2e/api/rpkg_init_test.go Outdated
Comment thread test/e2e/suiteutils/suite.go
Comment thread pkg/registry/porch/table.go Outdated
Copy link
Copy Markdown
Contributor Author

@JamesMcDermott JamesMcDermott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. I think we need to add some size checks for clone and edit e2e flows. Maybe using assert.Greater() to avoid having to calculate the exact sizes.

Thanks - I've added the checks in the clone, edit and upgrade tests - the edit one also has a resources-update flow, so good call - it let me do a before/after comparison.

Not sure assert.Greater() would have any value given the adjustment I put in to avoid having exact sizes hardcoded in the tests...

Comment thread pkg/registry/porch/table.go Outdated
Comment thread test/e2e/suiteutils/suite.go
Comment thread test/e2e/api/rpkg_init_test.go Outdated
@liamfallon
Copy link
Copy Markdown
Collaborator

@JamesMcDermott please mark any closed off comments (especially the CoPilot ones) as resolved if they are resolved.

@JamesMcDermott JamesMcDermott force-pushed the feature-fetch-size-limit branch 3 times, most recently from 40fb113 to fa7e6d4 Compare May 12, 2026 14:55
@sonarqubecloud
Copy link
Copy Markdown

Copilot AI review requested due to automatic review settings May 13, 2026 15:02
@JamesMcDermott JamesMcDermott force-pushed the feature-fetch-size-limit branch from 2c701d9 to eca0bf7 Compare May 13, 2026 15:02
@JamesMcDermott JamesMcDermott force-pushed the feature-fetch-size-limit branch from eca0bf7 to 20e4aa2 Compare May 13, 2026 15:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new prrSizeBytes status field on PackageRevision to expose the total size (bytes) of a package revision’s resource contents, with DB schema + migrations to persist/backfill the value and E2E/docs updates to validate and document the new field.

Changes:

  • Add PrrSizeBytes to PackageRevisionStatus across API versions (v1alpha1 + v1alpha2), including conversion + OpenAPI + CRD schema.
  • Persist and read the value via DB cache (resources_size column) and populate it during sync/close-draft flows.
  • Update E2E tests and documentation to surface and validate the new status field.

Reviewed changes

Copilot reviewed 25 out of 27 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
test/e2e/suiteutils/suite.go Adds DB-cache detection flag and (problematic) import path changes.
test/e2e/api/rpkg_lifecycle_test.go Validates revision and adds package resource size validation.
test/e2e/api/rpkg_init_test.go Adds helper to validate PrrSizeBytes and updates imports.
test/e2e/api/rpkg_edit_test.go Adds assertions around size changes when resources are updated.
test/e2e/api/rpkg_clone_test.go Adds size validation after clone/upgrade flows.
test/e2e/api/repository_test.go Adds size validation during list checks and updates imports.
pkg/cache/dbcache/dbreposync.go Computes and stores resources size during external repo sync caching.
pkg/cache/dbcache/dbrepository.go Computes resources size on draft close before persisting.
pkg/cache/dbcache/dbpackagerevisionsql.go Wires resources_size into SELECT/INSERT/UPDATE SQL and scanning.
pkg/cache/dbcache/dbpackagerevision.go Exposes DB-cached size via PackageRevisionStatus.PrrSizeBytes.
docs/content/en/docs/7_cli_api/api-ref.md Documents the new prrSizeBytes status field.
docs/content/en/docs/5_architecture_and_components/package-cache/db-cache.md Documents DB-cache behavior including resource size summing on close.
docs/content/en/docs/4_tutorials_and_how-tos/working_with_package_revisions/inspecting-packages.md Minor formatting adjustment near output explanation.
docs/content/en/docs/2_concepts/package-revisions.md Fixes YAML indentation and updates repo branch example.
deployments/porch/3-porch-postgres-bundle.yaml Adds resources_size column to packaged Postgres schema.
controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller-with-workspacename_test.go Updates copyright header year.
api/sql/porch-db.sql Adds resources_size column to base schema.
api/sql/porch-db-1.6.0-1.5.9.sql Adds downgrade script to drop resources_size.
api/sql/porch-db-1.5.9-1.6.0.sql Adds upgrade script to add/backfill resources_size.
api/sql/porch-db-1.5.9-1.5.10.sql Adds upgrade script to add/backfill resources_size.
api/sql/porch-db-1.5.10-1.5.9.sql Adds downgrade script to drop resources_size.
api/porch/v1alpha2/porch.kpt.dev_packagerevisions.yaml Updates CRD schema docs and adds prrSizeBytes.
api/porch/v1alpha2/packagerevision_types.go Adds PrrSizeBytes to v1alpha2 status type.
api/porch/v1alpha1/zz_generated.conversion.go Wires PrrSizeBytes through generated conversions.
api/porch/v1alpha1/types.go Adds PrrSizeBytes to v1alpha1 status type and fixes comments.
api/porch/types.go Adds PrrSizeBytes to internal API status type and fixes comments.
api/generated/openapi/zz_generated.openapi.go Updates generated OpenAPI schema to include prrSizeBytes.
Files not reviewed (1)
  • api/porch/v1alpha1/zz_generated.conversion.go: Language not supported

Comment thread test/e2e/suiteutils/suite.go Outdated
Comment thread test/e2e/api/rpkg_init_test.go Outdated
Comment thread test/e2e/api/repository_test.go Outdated
Comment thread api/sql/porch-db.sql Outdated
Comment thread deployments/porch/3-porch-postgres-bundle.yaml Outdated
Comment thread api/sql/porch-db-1.5.9-1.6.0.sql
Comment thread api/sql/porch-db-1.5.9-1.5.10.sql
Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go
Comment thread pkg/cache/dbcache/dbrepository.go
Comment thread test/e2e/api/rpkg_edit_test.go Outdated
Copilot AI review requested due to automatic review settings May 14, 2026 15:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 6 comments.

Files not reviewed (1)
  • api/porch/v1alpha1/zz_generated.conversion.go: Language not supported
Comments suppressed due to low confidence (1)

test/e2e/suiteutils/suite.go:167

  • The body of checkIfUsingDBCache wraps a trivial expression in an immediately-invoked anonymous function. This can be simplified to a direct assignment: t.UsingDBCache = os.Getenv("DB_CACHE") != "". The IIFE adds no value and obscures the logic.
func (t *TestSuite) checkIfUsingDBCache() {
	t.UsingDBCache = func() bool {
		return os.Getenv("DB_CACHE") != ""
	}()
}

Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go Outdated
Comment thread api/sql/porch-db-1.5.9-1.6.0.sql
Comment thread docs/content/en/docs/2_concepts/package-revisions.md
Comment thread test/e2e/suiteutils/suite.go Outdated
Comment thread test/e2e/api/rpkg_init_test.go
Copilot AI review requested due to automatic review settings May 14, 2026 16:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • api/porch/v1alpha1/zz_generated.conversion.go: Language not supported

- on ClosePackageRevisionDraft:
  - add up individual sizes of PackageRevisionResources
  - store total as a separate metadata field in package_revisions
    table
- on get/list PackageRevision, include total resources size in status
  field of returned API object

kptdev#811
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
- document new API field `prrSizeOnDisk`
- include new "SIZE ON DISK" table column in `rpkg get` output examples

Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
- fix E2E tests by excluding package-size testing from E2E tests for CR cache
  - new mechanism in test suite to detect DB cache when in use
    - can be used in test cases to exclude/include portions depending on cache
- remove "Size on Disk" column from `kubectl get packagerevisions` table
  - agreed not essential information, may set precedent to overload table output
  - also removes need for updates to E2E CLI tests

Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
- also remove restriction that meant size didn't get updated
  on PackageRevisionResources update

Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: ezmcdja <james.j.mcdermott@ericsson.com>
Signed-off-by: ezmcdja <james.j.mcdermott@ericsson.com>
@JamesMcDermott JamesMcDermott force-pushed the feature-fetch-size-limit branch from 05a35f5 to e76c342 Compare May 15, 2026 13:19
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
Copilot AI review requested due to automatic review settings May 15, 2026 14:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 28 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • api/porch/v1alpha1/zz_generated.conversion.go: Language not supported

Comment thread api/porch/v1alpha1/types.go Outdated
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose package resource file sizes in PackageRevision interface

6 participants