api
api copied to clipboard
openshift
Monitoring API: Add AlertmanagerMainConfig
Every component will be in a separated PR in order to improve review process
First PR: https://github.com/openshift/api/pull/1929 Related: Enhancements Proposal https://github.com/openshift/enhancements/pull/1627
Hello @marioferh! Some important instructions when contributing to openshift/api: API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.
![openshift-ci[bot] avatar](https://avatars.githubusercontent.com/in/91280?v=4 "Published by a pub.dev verified publisher"){kind=small}
@marioferh Just to clarify, what actor(s) would be creating this resource? Are we creating it on behalf of users or will users be creating it at some point?
@JoelSpeed I'll leave it up to you, but I noticed that there aren't any tests added for the new fields and validations - is that something we require for v1alpha1 APIs?
Yes, I would generally expect to see some testing of CEL validations to check that they are doing what we want, and prevent us breaking that behaviour in the future
@JoelSpeed I'll leave it up to you, but I noticed that there aren't any tests added for the new fields and validations - is that something we require for v1alpha1 APIs?
Yes, I would generally expect to see some testing of CEL validations to check that they are doing what we want, and prevent us breaking that behaviour in the future
do you have any example¿
Tests are documented in https://github.com/openshift/api/tree/master/tests
An example would be to add a file alongside the other tests with your FG name and then add some additional cases specific to your use case, https://github.com/openshift/api/blob/b76bb2c289448e8e57028b6337c1702a23fd8ff6/config/v1/tests/infrastructures.config.openshift.io/DualReplica.yaml#L18-L79
Tests are documented in https://github.com/openshift/api/tree/master/tests
An example would be to add a file alongside the other tests with your FG name and then add some additional cases specific to your use case,
https://github.com/openshift/api/blob/b76bb2c289448e8e57028b6337c1702a23fd8ff6/config/v1/tests/infrastructures.config.openshift.io/DualReplica.yaml#L18-L79
I mean about CEL validation, which ones should I check? everyone? only containerResources ones?
Isn't there only 1 rule on top of the container resources ones? Generally I'd expect tests for each so that we can see if stuff that previously worked stops working as make updates
/retest-required
Remaining retests: 0 against base HEAD 674ad74beffcbdf6aa7a577bf23a269c24f92fe8 and 2 for PR HEAD 211ce8489133265bd7a8b4b79d8bb7a7a7f06ebd in total
/retest-required
Remaining retests: 0 against base HEAD 674ad74beffcbdf6aa7a577bf23a269c24f92fe8 and 2 for PR HEAD 211ce8489133265bd7a8b4b79d8bb7a7a7f06ebd in total
/override ci/prow/verify-crd-schema
Only incorrectly identified newly required fields (they have optional parents)
@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-crd-schema
In response to this:
/override ci/prow/verify-crd-schema
Only incorrectly identified newly required fields (they have optional parents)
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: JoelSpeed, marioferh
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [JoelSpeed]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
/retest-required
Remaining retests: 0 against base HEAD 674ad74beffcbdf6aa7a577bf23a269c24f92fe8 and 2 for PR HEAD bcc4fc1de12c78c50e574444d2879c56d9a85c89 in total
/retest-required
Remaining retests: 0 against base HEAD 674ad74beffcbdf6aa7a577bf23a269c24f92fe8 and 2 for PR HEAD bcc4fc1de12c78c50e574444d2879c56d9a85c89 in total