-
Notifications
You must be signed in to change notification settings - Fork 347
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
feat: support mergepatch to envoyproxy/ratelimit deployment #2374
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2374 +/- ##
==========================================
- Coverage 63.38% 63.38% -0.01%
==========================================
Files 119 119
Lines 19098 19485 +387
==========================================
+ Hits 12106 12350 +244
- Misses 6193 6333 +140
- Partials 799 802 +3 ☔ View full report in Codecov by Sentry. |
0c59f94
to
b727ef9
Compare
/retest |
25b4b55
to
b1a9f18
Compare
/retest |
as a follow up, we need to make sure that if a patch fails, we need to surface that to the |
6322484
to
9193e7d
Compare
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
9193e7d
to
0b7fb63
Compare
api/v1alpha1/shared_types.go
Outdated
// | ||
// By default, StrategicMerge is used as the patch type. | ||
// +optional | ||
Type MergeType `json:"type,omitempty"` |
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.
should be a pointer since its optional
small nit, else LGTM ! |
cea05e6
to
b9ab5a5
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.
LGTM thanks !
lets raise some follow up GH issues that can hopefully be tackled pre GA
- to populate patch errors in GatewayClass status
- to support patching for envoy.service as well
6569c41
to
393da43
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.
LGTM, thanks for bringing this feature in
/retest |
What type of PR is this?
feat(infra): support mergepatch to envoyproxy/ratelimit deployment
What this PR does / why we need it:
Add unsupported fields in deployment, for private customizations like:
Which issue(s) this PR fixes:
Fixes #2346