add bundle size validator check #210
Conversation
pkg/validation/internal/bundle.go
Outdated
| @@ -15,6 +17,9 @@ import ( | |||
|
|
|||
| var BundleValidator interfaces.Validator = interfaces.ValidatorFunc(validateBundles) | |||
|
|
|||
| // max_bundle_sized_allowed defines the max size of the bundle | |||
| const max_bundle_sized_allowed = 1048576 | |||
There was a problem hiding this comment.
@kevinrizza could you please help out just by double-checking that is the correct amount?
There was a problem hiding this comment.
c/c @perdasilva @njhale @anik120 could you please help us check if that amount is the right one and etc?
pkg/validation/internal/bundle.go
Outdated
| @@ -15,6 +17,9 @@ import ( | |||
|
|
|||
| var BundleValidator interfaces.Validator = interfaces.ValidatorFunc(validateBundles) | |||
|
|
|||
| // max_bundle_sized_allowed defines the max size of the bundle | |||
| const max_bundle_sized_allowed = 1048576 | |||
There was a problem hiding this comment.
| const max_bundle_sized_allowed = 1048576 | |
| // max_bundle_size is the maximum size of a bundle in bytes. | |
| // This ensures the bundle can be staged in a single ConfigMap by OLM during installation. The value is derived from the standard upper bound for k8s resources (~1MB). | |
| const max_bundle_size = 1048576 |
There was a problem hiding this comment.
This looks like the number of bits, not bytes.
There was a problem hiding this comment.
There are two variables that change this:
- decrease: overhead of storing in a ConfigMap; i.e. it should really be 1MB - |ConfigMap metadata|
- increase: newer versions of OLM use gzip to compress bundle content on cluster
Doing some napkin math based on the results cited in original gzip PR yields a compression ratio of ~5.6:1; i.e. a space saving of ~82%. If we assume that bundle is representative of the entire population, then a ~4MB cap seems reasonable (5.61MB$SAFETY_BUFFER, where SAFETY_BUFFER=.75).
There was a problem hiding this comment.
So, do you think that we can safely increase this amount to ~4MB?
I know that currently if the bundle is too big that fails in the deploy right?
The validator here would give the opportunity we alert and give a proper message before that happens.
|
Is it also worth checking each file to ensure that they can be applied by etcd? Even if OLM can unpack it, if a single crd hits the default etcd file limit on clusters (1MB), then the bundle won't be applied just a little further into install |
go.mod
Outdated
| @@ -9,6 +9,7 @@ require ( | |||
| github.com/go-bindata/go-bindata/v3 v3.1.3 | |||
| github.com/google/cel-go v0.9.0 | |||
| github.com/mikefarah/yq/v3 v3.0.0-20201202084205-8846255d1c37 | |||
| github.com/operator-framework/operator-registry v1.19.5 | |||
There was a problem hiding this comment.
this is a circular dependency. the api project shouldn't depend on the operator-registry
There was a problem hiding this comment.
So, it shows for me that the code to do the zip ought to be shipped here and we do a follow up for opm get it from here. Tks
/hold
There was a problem hiding this comment.
openshift-ci
bot
added
do-not-merge/hold
|
@camilamacedo86 rename this file: |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, njhale The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
Description
Co-authored-by: Brett Tofel [email protected]
Closes: #163
Related to: operator-framework/operator-registry#685