Comments (3)
I would vote for starting with warning on missing resource requests. Independent on all the good points made for the need to have request resources set it is not a good user experience when you have something that used to work and suddenly the validation starts to fail when actually you have not changed anything that justifies it. At least if we decide to fail the test later on they will have been warned beforehand and it would not come as a surprise.
from api.
So, concerns were raised against adding the validator to enforce/fail when the requested resource is not set. In this way, I'd like to add here and share the mainly strongest argumentations for both cases:
The motivation to mandatory ask for authors to set at least the resource request is:
- If a LimitRange is activated in a namespace for computing resources like CPU and memory, users must specify requests or limits for those values. Otherwise, the system may reject Pod creation.
- If a resourQuota be configured and the pods do not have at least requests or limits for those values the system may reject Pod creation.
And then, the above configuration is commonly adopted and recommended in multi-tenancy and or production clusters where OLM is used usually. More info: https://github.com/operator-framework/operator-sdk/pull/5353/files
The argumentation against making the request resource mandatory is:
We want these operator authors to set GOOD resource requests. To create good resource requests, operator authors will need time and customer usage data
.
NOTE: An against argumentation here is that: the Operator Authors can do tests and benchmarks before publishing their first solution or indeed adopting reasonable init values for the requests such as:
resources:
requests:
cpu: 10m
memory: 64Mi
- Usually, we only make mandatory what will explicitly not work well with OLM and that shows to be more an indirectly need/requirement.
Additionally, another idea was raised as well: giving operators a "badge" in the UI when the operator meets these criteria.
Anyway, everybody seems to go along and agrees that the Operator Author should be hammered with warnings.
Also, it is important to highlight that:
- The
kube-rbac-proxy
container scaffold by default via SDK/KB has not these values set - The container to install the Ansible Operator/manager has not these values set in the default scaffold as well.
So, if we make it fail then, we will start to fail any distribution that is scaffolded by SDK.
IHMO: The argumentation to add the requested resource as mandatory shows strong. But, we can also get started by warning and then, we can revisit this one in the future as well.
c/c @kevinrizza @awgreene @fgiloux
from api.
I thought we had pluggable test suites these days. If so, then making the test fail should not be an issue as long as the user can decide whether or not they want to execute these tests. I agree, not all environments are sensitive to this. LimitRange
on the other hand are less ideal, because it requires the user to know what are acceptable values for requests
for a Java-based controller vs. a golang-based controller.
from api.
Related Issues (20)
- Improve API documentation for CSV and PackageManifest HOT 2
- Add package manifest apis HOT 3
- Remove go-get usage from Makefile in preparation of Go 1.18 HOT 2
- CSV Validation should check `spec.version` and `metadata.annotations.skipRange` HOT 2
- Begin to check the size of each bundle file into bundle size checker to ensure that all are not bigger than ~1MB HOT 1
- Version in CSV name should match the version field HOT 3
- ClusterServiceVersionValidator is not raising errors when the annotations are duplicated HOT 3
- [Validators] - Review and improve warn/error message in order to provide how to sort them out HOT 1
- Is there a reason that we're not using the regex from [semver](https://semver.org/)?
- The multiarch validator should not warning when only linux.amd64 platform is found HOT 1
- CSVValidator does not fail on invalid env in deployment spec
- Bump Go version to 1.18 HOT 1
- Catalog source poll is not required if the image refers to a digest
- Bump the yq tool dependency from v3 to v4 HOT 1
- Converting an operator group object to an unstructured object causes a panic HOT 1
- Serializing Subscription resources without including "status" field?
- Godoc missing from Subscription API
- RFE: good practices validator only warn for empty CRD desc if spec.customresourcedefinitions in csv is not empty
- Enhance partial explanations on `catalogsource.status`
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from api.