Rework of the optional manifest enhancement proposal: #111
Conversation
enhancements/optional-manifest.md
Outdated
| - 'monitoring.coreos.com/PrometheusRule/mynamespace/myrule' | ||
| - 'authorization.openshift.io/ClusterRoleBinding//mycrbinding' |
There was a problem hiding this comment.
I'd prefer to see explicit group, version, kind, name, and namespace fields for each list item so that:
- authors don't have to remember the correct order to provide them
- there's no ambiguity when dealing with cluster-scoped APIs
There was a problem hiding this comment.
I have updated the PR for the enhancement proposal and also the PR for the implementation in this sense
enhancements/optional-manifest.md
Outdated
| [maturity levels][maturity-levels]. | ||
|
|
||
| ##### Dev Preview -> Tech Preview | ||
| The optional manifest feature is currently scheduled to be available in the next release. It will first be provided as a tech-preview and begins to gather users' feedbacks. |
There was a problem hiding this comment.
Tech preview features must follow a certain process (as defined here). Can you expand on this section a little bit to clarify the mechanisms we'll use for the feature gate?
There was a problem hiding this comment.
I am getting confused as you are pointing to downstream documentation. The main aspect there is the opt-in through a feature gate.
I did a quick search on the enhancement proposals and "generic constraint" is the only one I found for OLM that has this part covered. Looking at the code I see pkg/feature/feature.go but I have not found any use of it.
It is surely something I could look at but I am questioning the incentive for creating a precedent for a feature that does not introduce new CRDs and does not have any effect when not explicitly used. I understand that users can start depend on this feature without being aware of its maturity level and that is precisely the reason why I was proposing first to introduce it as a tech. preview. The API for this feature is "explicit bundle properties", which does not offer a standard way of versioning (following the CRD way I would have put it under "alpha" otherwise).
If that's the way to go I can just remove this part.
There was a problem hiding this comment.
Yep. Sorry disregard downstream references.
Side note: I think we should remove this dev/tech preview nomenclature from our upstream EP template, and make this section geared more toward introducing features behind generic upstream feature gates.
There was a problem hiding this comment.
@joelanford Thanks for the clarification. I have modified the verbiage and removed references to graduation, tech. preview and GA only keeping the fact that it won't be introduced behind a feature gate and that we will gather feedback on it.
Is it good to merge?
There was a problem hiding this comment.
To clarify, I think we should consider adding this feature behind a feature gate. That way we can get some feedback on it before we fully commit to it.
There was a problem hiding this comment.
pushed a new version with the feature gate. I was not expecting that a feature with such a low radius would be subject to it when as far as I have seen feature gates have not been used much with OLM.
There was a problem hiding this comment.
Yeah, IMO OLM's propensity for straight to GA has made things harder to maintain because we've delivered features under pressure in the past and have not had the time to collect feedback, which results in features we sometimes immediately regret. In the future, I think straight to GA should be the exception rather than the rule.
There are definitely some added maintenance burdens with feature gates, so its worth debating more. But my gut is that if we're able to do it:
- Our upstream selves will be able to deliver new features without having to worry too much about their impact downstream.
- Anyone maintaining OLM in downstreams can pick and choose features to enable and be less conservative when reviewing/proposing upstream features, knowing they can disable anything that they're unprepared to support.
- We'll build a better community because downstream vendor-specific RFEs will be implemented (at worst) behind upstream feature flags.
There was a problem hiding this comment.
@joelanford there are things we would need to discuss regarding feature gates, olm and personas but this is not the subject of this PR. As I have added the feature gate verbiage any other things you would like to see or is it now good to get merged from your point of view?
There was a problem hiding this comment.
Yep. Looks good to me. Thanks!
fgiloux
force-pushed
the
master
branch
3 times, most recently
from
adcc813
to
26b11fb
Compare
- to use group, kind, namespace, name for identifying manifests instead of filenames - adding graduation criteria - adding test strategy - clarifying the case of StatusReasonAlreadyExists Signed-off-by: Frederic Giloux <fgiloux@redhat.com>
|
@joelanford @kevinrizza @perdasilva anything preventing this PR of getting merged? |
Signed-off-by: Frederic Giloux fgiloux@redhat.com