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/05/23 14:12:22 UTC
[GitHub] [apisix-ingress-controller] hahayyum opened a new pull request, #1040: fix : The ingress backend is modified several times, resulting in residual update events
hahayyum opened a new pull request, #1040:
URL: https://github.com/apache/apisix-ingress-controller/pull/1040
After updating k8s ingress(backend) multiple times, the EventUpdate event always exists and cannot end #1027
--
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] hahayyum commented on pull request #1040: fix : The ingress backend is modified several times, resulting in residual update events
Posted by GitBox <gi...@apache.org>.
hahayyum commented on PR #1040:
URL: https://github.com/apache/apisix-ingress-controller/pull/1040#issuecomment-1163909260
@tao12345666333 The conflict has been resolved.
--
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] hahayyum commented on pull request #1040: fix : The ingress backend is modified several times, resulting in residual update events
Posted by GitBox <gi...@apache.org>.
hahayyum commented on PR #1040:
URL: https://github.com/apache/apisix-ingress-controller/pull/1040#issuecomment-1135309249
@tokers
Yes, don't check the old ingress, just keep the latest one
--
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] hahayyum commented on pull request #1040: fix : The ingress backend is modified several times, resulting in residual update events
Posted by GitBox <gi...@apache.org>.
hahayyum commented on PR #1040:
URL: https://github.com/apache/apisix-ingress-controller/pull/1040#issuecomment-1136703389
@tokers
E2E test (suite features), causing PR operation failure
error info : localhost: port connection failed, resulting in the failure of normal list / watch
So, what do I need to change？
--
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] hahayyum commented on a diff in pull request #1040: fix : The ingress backend is modified several times, resulting in residual update events
Posted by GitBox <gi...@apache.org>.
hahayyum commented on code in PR #1040:
URL: https://github.com/apache/apisix-ingress-controller/pull/1040#discussion_r879981530
##########
pkg/kube/translation/ingress.go:
##########
@@ -273,6 +281,29 @@ func (t *translator) translateIngressV1beta1(ing *networkingv1beta1.Ingress) (*T
return ctx, nil
}
+func (t *translator) translateDefaultUpstreamFromIngressV1(namespace string, backend *networkingv1.IngressServiceBackend) *apisixv1.Upstream {
+ var portNumber int32
+ if backend.Port.Name != "" {
+ svc, err := t.ServiceLister.Services(namespace).Get(backend.Name)
+ if err != nil {
+ for _, port := range svc.Spec.Ports {
Review Comment:
You're right. I missed this part of the test
--
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 #1040: fix : The ingress backend is modified several times, resulting in residual update events
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1040:
URL: https://github.com/apache/apisix-ingress-controller/pull/1040#issuecomment-1163238028
@hahayyum sorry for delay, could you please resovle the conflicts? so that we can prepare to merge it
--
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] tokers commented on a diff in pull request #1040: fix : The ingress backend is modified several times, resulting in residual update events
Posted by GitBox <gi...@apache.org>.
tokers commented on code in PR #1040:
URL: https://github.com/apache/apisix-ingress-controller/pull/1040#discussion_r879971391
##########
pkg/kube/translation/ingress.go:
##########
@@ -393,6 +428,29 @@ func (t *translator) translateIngressExtensionsV1beta1(ing *extensionsv1beta1.In
return ctx, nil
}
+func (t *translator) translateDefaultUpstreamFromIngressV1beta1(namespace string, svcName string, svcPort intstr.IntOrString) *apisixv1.Upstream {
+ var portNumber int32
+ if svcPort.Type == intstr.String {
+ svc, err := t.ServiceLister.Services(namespace).Get(svcName)
+ if err != nil {
Review Comment:
Ditto.
##########
pkg/kube/translation/ingress.go:
##########
@@ -273,6 +281,29 @@ func (t *translator) translateIngressV1beta1(ing *networkingv1beta1.Ingress) (*T
return ctx, nil
}
+func (t *translator) translateDefaultUpstreamFromIngressV1(namespace string, backend *networkingv1.IngressServiceBackend) *apisixv1.Upstream {
+ var portNumber int32
+ if backend.Port.Name != "" {
+ svc, err := t.ServiceLister.Services(namespace).Get(backend.Name)
+ if err != nil {
+ for _, port := range svc.Spec.Ports {
Review Comment:
Should not use the `svc` variable when `err` is not `nil`, this might cause the program panic.
##########
pkg/kube/translation/ingress.go:
##########
@@ -273,6 +281,29 @@ func (t *translator) translateIngressV1beta1(ing *networkingv1beta1.Ingress) (*T
return ctx, nil
}
+func (t *translator) translateDefaultUpstreamFromIngressV1(namespace string, backend *networkingv1.IngressServiceBackend) *apisixv1.Upstream {
+ var portNumber int32
+ if backend.Port.Name != "" {
+ svc, err := t.ServiceLister.Services(namespace).Get(backend.Name)
+ if err != nil {
+ for _, port := range svc.Spec.Ports {
+ if port.Name == backend.Port.Name {
+ portNumber = port.Port
+ break
+ }
+ }
+ } else {
+ portNumber = 0
+ }
+
+ } else {
+ portNumber = backend.Port.Number
+ }
+ ups := apisixv1.NewDefaultUpstream()
+ ups.Name = apisixv1.ComposeUpstreamName(namespace, backend.Name, "", portNumber)
+ ups.ID = id.GenID(ups.Name)
+ return ups
+}
Review Comment:
Leave an empty there.
--
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 #1040: fix : The ingress backend is modified several times, resulting in residual update events
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1040:
URL: https://github.com/apache/apisix-ingress-controller/pull/1040#issuecomment-1136904680
re-run all jobs.
--
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 #1040: fix : The ingress backend is modified several times, resulting in residual update events
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1040:
URL: https://github.com/apache/apisix-ingress-controller/pull/1040#issuecomment-1136771042
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1040?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 [#1040](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1040?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a667e7e) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/f6f0a3b5552ba8fda556e0edb9296c1c0a4c3e31?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f6f0a3b) will **decrease** coverage by `0.16%`.
> The diff coverage is `32.00%`.
```diff
@@ Coverage Diff @@
## master #1040 +/- ##
==========================================
- Coverage 30.82% 30.66% -0.17%
==========================================
Files 74 74
Lines 8606 8652 +46
==========================================
Hits 2653 2653
- Misses 5670 5710 +40
- Partials 283 289 +6
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1040?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1040/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-cGtnL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `6.54% <0.00%> (ø)` | |
| [pkg/kube/translation/translator.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1040/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-cGtnL2t1YmUvdHJhbnNsYXRpb24vdHJhbnNsYXRvci5nbw==) | `44.76% <0.00%> (-1.32%)` | :arrow_down: |
| [pkg/kube/translation/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1040/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-cGtnL2t1YmUvdHJhbnNsYXRpb24vaW5ncmVzcy5nbw==) | `68.11% <35.82%> (-9.70%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1040?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1040?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [f6f0a3b...a667e7e](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1040?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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
[GitHub] [apisix-ingress-controller] hahayyum commented on a diff in pull request #1040: fix : The ingress backend is modified several times, resulting in residual update events
Posted by GitBox <gi...@apache.org>.
hahayyum commented on code in PR #1040:
URL: https://github.com/apache/apisix-ingress-controller/pull/1040#discussion_r879983617
##########
pkg/kube/translation/ingress.go:
##########
@@ -273,6 +281,29 @@ func (t *translator) translateIngressV1beta1(ing *networkingv1beta1.Ingress) (*T
return ctx, nil
}
+func (t *translator) translateDefaultUpstreamFromIngressV1(namespace string, backend *networkingv1.IngressServiceBackend) *apisixv1.Upstream {
+ var portNumber int32
+ if backend.Port.Name != "" {
+ svc, err := t.ServiceLister.Services(namespace).Get(backend.Name)
+ if err != nil {
+ for _, port := range svc.Spec.Ports {
+ if port.Name == backend.Port.Name {
+ portNumber = port.Port
+ break
+ }
+ }
+ } else {
+ portNumber = 0
+ }
+
+ } else {
+ portNumber = backend.Port.Number
+ }
+ ups := apisixv1.NewDefaultUpstream()
+ ups.Name = apisixv1.ComposeUpstreamName(namespace, backend.Name, "", portNumber)
+ ups.ID = id.GenID(ups.Name)
+ return ups
+}
Review Comment:
In fact, only one ID needs to be returned here to ensure that the original data can be updated or deleted
--
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 #1040: fix : The ingress backend is modified several times, resulting in residual update events
Posted by GitBox <gi...@apache.org>.
tao12345666333 merged PR #1040:
URL: https://github.com/apache/apisix-ingress-controller/pull/1040
--
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 #1040: fix : The ingress backend is modified several times, resulting in residual update events
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1040:
URL: https://github.com/apache/apisix-ingress-controller/pull/1040#issuecomment-1163910541
thanks! Let me take a look
--
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