-
Notifications
You must be signed in to change notification settings - Fork 56
CRD Validation for Etcd resource using CEL expressions #995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@Shreyas-s14 Thank you for your contribution. |
Thank you @Shreyas-s14 for your contribution. Before I can start building your PR, a member of the organization must set the required label(s) {'reviewed/ok-to-test'}. Once started, you can check the build status in the PR checks section below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @Shreyas-s14, for adding the validations for the etcd
CR. I've provided a few initial review comments based on my first glance. I will share more detailed feedback after a thorough review.
/test pull-etcd-druid-e2e-kind |
* Added `itTestEnv.StartManager()` to start the manager in `setupTestEnvironment()`. * Upgraded teh env-test k8s version to `1.30`. * Changed the timeout to `20s` in `assertReconcileSuspensionEventRecorded` to prevent the test timing out. * Removed an unnecessary `time.Sleep` in `TestValidateSpecReplicas`. * Uncommented previously commented options which caused regressions.
2) added version compatibility docs 3) reduced code reuse in specUpdate_test.go
1) increased value of timeout passed in test/it/helper.go createAndAssertEtcdAndAllManagedResources 2) increased the timeout in test/e2e/suite_test.go to 20 minutes each
Reverted the timeout value in test/it/helper.go which was increased to handle failure of prow jobs
…sion-compatibility-matrix.md
c6a0f93
to
a0c45a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @Shreyas-s14, for addressing my review comments.
/lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@unmarshall thanks for the fix.
/lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
How to categorize this PR?
/area quality
/area control-plane
/area open-source
/kind enhancement
What this PR does / why we need it:
kubebuilder markers
, using a combination of CEL Expressions via thex-validation
and thevalidation
markers.api/core/crds
is changed to now have API methods which takes in a k8s version to provide the appropriate CRD.make deploy-*
andmake prepare-helm-chart
targets to work with CRDs with and without CEL expressions.Which issue(s) this PR fixes:
Fixes #989
Special notes for your reviewer:
Release note: