feat: add egctl ratelimit config support #2674
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2674 +/- ##
==========================================
- Coverage 63.38% 62.98% -0.41%
==========================================
Files 119 122 +3
Lines 19098 19756 +658
==========================================
+ Hits 12106 12443 +337
- Misses 6193 6506 +313
- Partials 799 807 +8 ☔ View full report in Codecov by Sentry. |
ShyunnY
force-pushed
the
egctl
branch
2 times, most recently
from
faed6ce to
5da05ef
Compare
Signed-off-by: ShyunnY <[email protected]>
| @@ -35,6 +36,10 @@ func newConfigCommand() *cobra.Command { | |||
| return cfgCommand | |||
| } | |||
|
|
|||
| func ratelimitCommand() *cobra.Command { | |||
There was a problem hiding this comment.
why not use ratelimitConfigCommand directly?
There was a problem hiding this comment.
I have thought about this problem, but I hope to write it like the func proxyCommand() function for backward compatibility.
Signed-off-by: ShyunnY <[email protected]>
|
@shawnh2 Thank you for your suggestions on this PR ❤️ , I have modified it. But I don’t quite understand why CI test will produce errors. Is this my problem? |
| expectErr error | ||
| }{ | ||
| { | ||
| caseName: "normally obtain the rate limit pod of Running phase", |
There was a problem hiding this comment.
the log shows there's something wrong with this test case: https://github.com/envoyproxy/gateway/actions/runs/8047676612/job/21977431309?pr=2674#step:4:144.
seems the conditions in this status also needs to be updated according to checkRateLimitPodStatusReady
| tc := tc | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| got := LabelSelector() | ||
| require.Equal(t, tc.expected, got) |
There was a problem hiding this comment.
better go with require.ElementsMatch, since we only can promise the values of got as expected not the orders.
There was a problem hiding this comment.
Okay, do I only need to change these two things?
There was a problem hiding this comment.
It seems to be okay this time 😃
Signed-off-by: ShyunnY <[email protected]>
|
/retest |
What type of PR is this?
feat(egctl): add egctl ratelimit config support
What this PR does / why we need it:
Add the
egctl ratelimitsubcommand toegctl. This subcommand can help developers or users quickly view the ratelimit configuration or debug the ratelimit.Which issue(s) this PR fixes:
Fixes #2584