You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/12/13 09:49:17 UTC
[GitHub] [apisix-ingress-controller] fatpa opened a new pull request, #1519: Add prefer name param into ApisixClusterConfig
fatpa opened a new pull request, #1519:
URL: https://github.com/apache/apisix-ingress-controller/pull/1519
<!-- Please answer these questions before submitting a pull request -->
### Type of change:
<!-- Please delete options that are not relevant. -->
- [ ] Bugfix
- [x] New feature provided
- [ ] Improve performance
- [ ] Backport patches
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
Add `perfer_name` to apisixclusterconfig `prometheus.properties`
### Pre-submission checklist:
<!--
Please follow the requirements:
1. Use Draft if the PR is not ready to be reviewed
2. Test is required for the feat/fix PR, unless you have a good reason
3. Doc is required for the feat PR
4. Use a new commit to resolve review instead of `push -f`
5. Use "request review" to notify the reviewer once you have resolved the review
-->
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix-ingress-controller#community) first**
Refer to #1516
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] fatpa commented on pull request #1519: feat: Add prefer_name into ApisixClusterConfig
Posted by GitBox <gi...@apache.org>.
fatpa commented on PR #1519:
URL: https://github.com/apache/apisix-ingress-controller/pull/1519#issuecomment-1355192282
> we have entered the rv1.6 elease window, I will put this to v1.7, thanks
Ok, I will fix the e2e test bug first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1519: feat: Add prefer_name into ApisixClusterConfig
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1519:
URL: https://github.com/apache/apisix-ingress-controller/pull/1519#issuecomment-1367782781
sorry for delay, re-run all test cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1519: feat: Add prefer_name into ApisixClusterConfig
Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on code in PR #1519:
URL: https://github.com/apache/apisix-ingress-controller/pull/1519#discussion_r1091529318
##########
utils/kind-with-registry.sh:
##########
@@ -67,7 +67,7 @@ nodes:
containerdConfigPatches:
- |-
[plugins."io.containerd.grpc.v1.cri".registry.mirrors."localhost:${reg_port}"]
- endpoint = ["http://${reg_host}:5000"]
+ endpoint = ["http://${reg_host}:${reg_port}"]
Review Comment:
I don't think adjustments are needed here.
Have you tested it? They use addresses inside the container
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] lingsamuel commented on pull request #1519: feat: Add prefer_name into ApisixClusterConfig
Posted by GitBox <gi...@apache.org>.
lingsamuel commented on PR #1519:
URL: https://github.com/apache/apisix-ingress-controller/pull/1519#issuecomment-1354378682
New features must include corresponding e2e tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1519: feat: Add prefer_name into ApisixClusterConfig
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1519:
URL: https://github.com/apache/apisix-ingress-controller/pull/1519#issuecomment-1355190769
we have entered the rv1.6 elease window, I will put this to v1.7, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 merged pull request #1519: feat: Add prefer_name into ApisixClusterConfig
Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 merged PR #1519:
URL: https://github.com/apache/apisix-ingress-controller/pull/1519
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] fatpa commented on pull request #1519: feat: Add prefer_name into ApisixClusterConfig
Posted by GitBox <gi...@apache.org>.
fatpa commented on PR #1519:
URL: https://github.com/apache/apisix-ingress-controller/pull/1519#issuecomment-1377037085
Need to rerun unit-test-ci.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] fatpa commented on pull request #1519: feat: Add prefer_name into ApisixClusterConfig
Posted by GitBox <gi...@apache.org>.
fatpa commented on PR #1519:
URL: https://github.com/apache/apisix-ingress-controller/pull/1519#issuecomment-1348162160
Please take a look. @tao12345666333
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] fatpa commented on a diff in pull request #1519: feat: Add prefer_name into ApisixClusterConfig
Posted by "fatpa (via GitHub)" <gi...@apache.org>.
fatpa commented on code in PR #1519:
URL: https://github.com/apache/apisix-ingress-controller/pull/1519#discussion_r1091544834
##########
utils/kind-with-registry.sh:
##########
@@ -67,7 +67,7 @@ nodes:
containerdConfigPatches:
- |-
[plugins."io.containerd.grpc.v1.cri".registry.mirrors."localhost:${reg_port}"]
- endpoint = ["http://${reg_host}:5000"]
+ endpoint = ["http://${reg_host}:${reg_port}"]
Review Comment:
My misunderstanding. Had reverted the script.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1519: feat: Add prefer_name into ApisixClusterConfig
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on code in PR #1519:
URL: https://github.com/apache/apisix-ingress-controller/pull/1519#discussion_r1063971016
##########
pkg/providers/apisix/translation/apisix_cluster_config.go:
##########
@@ -35,7 +37,9 @@ func (t *translator) TranslateClusterConfigV2beta3(acc *configv2beta3.ApisixClus
if acc.Spec.Monitoring != nil {
if acc.Spec.Monitoring.Prometheus.Enable {
- globalRule.Plugins["prometheus"] = &prometheusPluginConfig{}
+ globalRule.Plugins["prometheus"] = &prometheusPluginConfig{
+ PreferName: acc.Spec.Monitoring.Prometheus.PreferName,
+ }
Review Comment:
ditto
##########
pkg/providers/apisix/translation/apisix_cluster_config_test.go:
##########
@@ -35,7 +36,8 @@ func TestTranslateClusterConfig(t *testing.T) {
Spec: configv2beta3.ApisixClusterConfigSpec{
Monitoring: &configv2beta3.ApisixClusterMonitoringConfig{
Prometheus: configv2beta3.ApisixClusterPrometheusConfig{
- Enable: true,
+ Enable: true,
+ PreferName: true,
Review Comment:
ditto
##########
pkg/kube/apisix/apis/config/v2beta3/types.go:
##########
@@ -288,6 +288,8 @@ type ApisixClusterMonitoringConfig struct {
type ApisixClusterPrometheusConfig struct {
// Enable means whether enable Prometheus or not.
Enable bool `json:"enable" yaml:"enable"`
+ // PreferName means whether prints Route/Service name or ID in Prometheus metric
+ PreferName bool `json:"prefer_name" yaml:"prefer_name"`
Review Comment:
v2beta3 has been deprecated, we don't need change this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] fatpa commented on pull request #1519: feat: Add prefer_name into ApisixClusterConfig
Posted by GitBox <gi...@apache.org>.
fatpa commented on PR #1519:
URL: https://github.com/apache/apisix-ingress-controller/pull/1519#issuecomment-1356269022
New feature's e2e tests have run successfully.
But there are some errors on the external-service e2e tests, it seems not linked to this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] codecov-commenter commented on pull request #1519: feat: Add prefer_name into ApisixClusterConfig
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1519:
URL: https://github.com/apache/apisix-ingress-controller/pull/1519#issuecomment-1354528778
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1519?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1519](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1519?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7e6cf4f) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/c6c2742fe9fe60efd40f2c0ecc5c2fc7f2166a2a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c6c2742) will **increase** coverage by `0.16%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #1519 +/- ##
==========================================
+ Coverage 41.22% 41.38% +0.16%
==========================================
Files 85 85
Lines 7440 7442 +2
==========================================
+ Hits 3067 3080 +13
+ Misses 4020 4009 -11
Partials 353 353
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1519?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...viders/apisix/translation/apisix\_cluster\_config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1519/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3Byb3ZpZGVycy9hcGlzaXgvdHJhbnNsYXRpb24vYXBpc2l4X2NsdXN0ZXJfY29uZmlnLmdv) | `100.00% <100.00%> (+50.00%)` | :arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org