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 2021/11/08 01:27:00 UTC

[GitHub] [apisix-ingress-controller] tao12345666333 opened a new pull request #740: Fix lb ingress status

tao12345666333 opened a new pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740


   Please answer these questions before submitting a pull request
   
   - Why submit this pull request?
   - [x] Bugfix
   - [ ] New feature provided
   - [ ] Improve performance
   - [ ] Backport patches
   
   - Related issues
   #690 
   ___
   ### Bugfix
   - Description
   
   - How to fix?
   
   ___
   ### New feature or improvement
   - Describe the details and related test reports.
   
   ___
   ### Backport patches
   - Why need to backport?
   
   - Source branch
   
   - Related commits and pull requests
   
   - Target branch
   


-- 
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 change in pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#discussion_r749045841



##########
File path: pkg/ingress/status.go
##########
@@ -163,8 +173,142 @@ func (c *Controller) recordStatus(at interface{}, reason string, err error, stat
 				)
 			}
 		}
+	case *networkingv1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+
+	case *networkingv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+	case *extensionsv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.ExtensionsV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
 	default:
 		// This should not be executed
 		log.Errorf("unsupported resource record: %s", v)
 	}
 }
+
+// ingressPublishAddresses get addressed used to expose Ingress
+func (c *Controller) ingressPublishAddresses() ([]string, error) {
+	ingressPublishService := c.cfg.IngressPublishService
+	ingressStatusAddress := c.cfg.IngressStatusAddress
+	addrs := []string{}
+
+	// if ingressStatusAddress is specified, it will be used first
+	if len(ingressStatusAddress) > 0 {
+		addrs = append(addrs, ingressStatusAddress...)
+		return addrs, nil
+	}
+
+	namespace, name, err := cache.SplitMetaNamespaceKey(ingressPublishService)

Review comment:
       This specified service is only needed here, I don’t think it needs to be checked or processed in advance.




-- 
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 edited a comment on pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#issuecomment-962736571


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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 [#740](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (554c677) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/4a862e206602ae9c7ac534fdfd9a557748b9ad26?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4a862e2) will **decrease** coverage by `0.50%`.
   > The diff coverage is `11.26%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #740      +/-   ##
   ==========================================
   - Coverage   31.70%   31.19%   -0.51%     
   ==========================================
     Files          66       65       -1     
     Lines        6640     6774     +134     
   ==========================================
   + Hits         2105     2113       +8     
   - Misses       4280     4406     +126     
     Partials      255      255              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3JvdXRlLmdv) | `0.00% <ø> (ø)` | |
   | [pkg/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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==) | `7.03% <0.00%> (-1.27%)` | :arrow_down: |
   | [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3Mvc3RhdHVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.55% <100.00%> (+1.72%)` | :arrow_up: |
   | [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `63.33% <100.00%> (+0.83%)` | :arrow_up: |
   | [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-dGVzdC9lMmUvZTJlLmdv) | | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/740?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 [774077a...554c677](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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] tao12345666333 commented on a change in pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#discussion_r760118736



##########
File path: pkg/ingress/status.go
##########
@@ -163,8 +173,142 @@ func (c *Controller) recordStatus(at interface{}, reason string, err error, stat
 				)
 			}
 		}
+	case *networkingv1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+
+	case *networkingv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+	case *extensionsv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.ExtensionsV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
 	default:
 		// This should not be executed
 		log.Errorf("unsupported resource record: %s", v)
 	}
 }
+
+// ingressPublishAddresses get addressed used to expose Ingress
+func (c *Controller) ingressPublishAddresses() ([]string, error) {
+	ingressPublishService := c.cfg.IngressPublishService
+	ingressStatusAddress := c.cfg.IngressStatusAddress
+	addrs := []string{}
+
+	// if ingressStatusAddress is specified, it will be used first
+	if len(ingressStatusAddress) > 0 {
+		addrs = append(addrs, ingressStatusAddress...)
+		return addrs, nil
+	}
+
+	namespace, name, err := cache.SplitMetaNamespaceKey(ingressPublishService)

Review comment:
       Because it is not a parameter required by the real runtime, I don’t want to refuse to provide services to users in advance.




-- 
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 edited a comment on pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#issuecomment-962736571


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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 [#740](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d6ae069) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/4a862e206602ae9c7ac534fdfd9a557748b9ad26?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4a862e2) will **decrease** coverage by `0.58%`.
   > The diff coverage is `6.66%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #740      +/-   ##
   ==========================================
   - Coverage   31.70%   31.12%   -0.59%     
   ==========================================
     Files          66       65       -1     
     Lines        6640     6767     +127     
   ==========================================
   + Hits         2105     2106       +1     
   - Misses       4280     4406     +126     
     Partials      255      255              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3JvdXRlLmdv) | `0.00% <ø> (ø)` | |
   | [pkg/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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==) | `7.03% <0.00%> (-1.27%)` | :arrow_down: |
   | [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3Mvc3RhdHVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `63.33% <100.00%> (+0.83%)` | :arrow_up: |
   | [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-dGVzdC9lMmUvZTJlLmdv) | | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/740?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 [774077a...d6ae069](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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] tokers commented on a change in pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#discussion_r760069714



##########
File path: pkg/ingress/status.go
##########
@@ -163,8 +173,142 @@ func (c *Controller) recordStatus(at interface{}, reason string, err error, stat
 				)
 			}
 		}
+	case *networkingv1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+
+	case *networkingv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+	case *extensionsv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.ExtensionsV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
 	default:
 		// This should not be executed
 		log.Errorf("unsupported resource record: %s", v)
 	}
 }
+
+// ingressPublishAddresses get addressed used to expose Ingress
+func (c *Controller) ingressPublishAddresses() ([]string, error) {
+	ingressPublishService := c.cfg.IngressPublishService
+	ingressStatusAddress := c.cfg.IngressStatusAddress
+	addrs := []string{}
+
+	// if ingressStatusAddress is specified, it will be used first
+	if len(ingressStatusAddress) > 0 {
+		addrs = append(addrs, ingressStatusAddress...)
+		return addrs, nil
+	}
+
+	namespace, name, err := cache.SplitMetaNamespaceKey(ingressPublishService)

Review comment:
       Since the namespace is passed from users, we can split it and check the validity when the program is starting, so we can avoid such an error check here and we can abort the program in time (if the format is incorrect).




-- 
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 edited a comment on pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#issuecomment-962736571


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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 [#740](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8c1ef63) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/4a862e206602ae9c7ac534fdfd9a557748b9ad26?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4a862e2) will **decrease** coverage by `0.58%`.
   > The diff coverage is `6.66%`.
   
   > :exclamation: Current head 8c1ef63 differs from pull request most recent head d6ae069. Consider uploading reports for the commit d6ae069 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #740      +/-   ##
   ==========================================
   - Coverage   31.70%   31.12%   -0.59%     
   ==========================================
     Files          66       65       -1     
     Lines        6640     6767     +127     
   ==========================================
   + Hits         2105     2106       +1     
   - Misses       4280     4406     +126     
     Partials      255      255              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3JvdXRlLmdv) | `0.00% <ø> (ø)` | |
   | [pkg/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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==) | `7.03% <0.00%> (-1.27%)` | :arrow_down: |
   | [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3Mvc3RhdHVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `63.33% <100.00%> (+0.83%)` | :arrow_up: |
   | [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-dGVzdC9lMmUvZTJlLmdv) | | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/740?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 [774077a...d6ae069](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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] tao12345666333 commented on a change in pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#discussion_r749039276



##########
File path: pkg/ingress/status.go
##########
@@ -163,8 +173,142 @@ func (c *Controller) recordStatus(at interface{}, reason string, err error, stat
 				)
 			}
 		}
+	case *networkingv1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+
+	case *networkingv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+	case *extensionsv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.ExtensionsV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
 	default:
 		// This should not be executed
 		log.Errorf("unsupported resource record: %s", v)
 	}
 }
+
+// ingressPublishAddresses get addressed used to expose Ingress
+func (c *Controller) ingressPublishAddresses() ([]string, error) {
+	ingressPublishService := c.cfg.IngressPublishService
+	ingressStatusAddress := c.cfg.IngressStatusAddress
+	addrs := []string{}
+
+	// if ingressStatusAddress is specified, it will be used first
+	if len(ingressStatusAddress) > 0 {
+		addrs = append(addrs, ingressStatusAddress...)
+		return addrs, nil

Review comment:
       I think that directly specifying the IP will be more direct and will have a higher priority. I will modify the description in the configuration.




-- 
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 change in pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#discussion_r749045213



##########
File path: pkg/ingress/status.go
##########
@@ -163,8 +173,142 @@ func (c *Controller) recordStatus(at interface{}, reason string, err error, stat
 				)
 			}
 		}
+	case *networkingv1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+
+	case *networkingv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+	case *extensionsv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.ExtensionsV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
 	default:
 		// This should not be executed
 		log.Errorf("unsupported resource record: %s", v)
 	}
 }
+
+// ingressPublishAddresses get addressed used to expose Ingress
+func (c *Controller) ingressPublishAddresses() ([]string, error) {
+	ingressPublishService := c.cfg.IngressPublishService
+	ingressStatusAddress := c.cfg.IngressStatusAddress
+	addrs := []string{}
+
+	// if ingressStatusAddress is specified, it will be used first
+	if len(ingressStatusAddress) > 0 {
+		addrs = append(addrs, ingressStatusAddress...)
+		return addrs, nil
+	}
+
+	namespace, name, err := cache.SplitMetaNamespaceKey(ingressPublishService)

Review comment:
       I didn't understand what you mean.




-- 
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] gxthrj commented on a change in pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#discussion_r748944286



##########
File path: pkg/ingress/ingress.go
##########
@@ -184,14 +185,64 @@ func (c *ingressController) sync(ctx context.Context, ev *types.Event) error {
 }
 
 func (c *ingressController) handleSyncErr(obj interface{}, err error) {
+	ev := obj.(*types.Event)
+	event := ev.Object.(kube.IngressEvent)
+	namespace, name, errLocal := cache.SplitMetaNamespaceKey(event.Key)
+	if errLocal != nil {
+		log.Errorf("invalid resource key: %s", event.Key)
+		return
+	}
+
+	var ing kube.Ingress
+	switch event.GroupVersion {
+	case kube.IngressV1:
+		ing, err = c.controller.ingressLister.V1(namespace, name)
+	case kube.IngressV1beta1:
+		ing, err = c.controller.ingressLister.V1beta1(namespace, name)
+	case kube.IngressExtensionsV1beta1:
+		ing, err = c.controller.ingressLister.ExtensionsV1beta1(namespace, name)
+	}
+
 	if err == nil {
+		// add status
+		if ev.Type != types.EventDelete {
+			if errLocal == nil {
+				switch ing.GroupVersion() {
+				case kube.IngressV1:
+					c.controller.recordStatus(ing.V1(), _resourceSynced, nil, metav1.ConditionTrue, ing.V1().GetGeneration())
+				case kube.IngressV1beta1:
+					c.controller.recordStatus(ing.V1beta1(), _resourceSynced, nil, metav1.ConditionTrue, ing.V1beta1().GetGeneration())
+				case kube.IngressExtensionsV1beta1:
+					c.controller.recordStatus(ing.ExtensionsV1beta1(), _resourceSynced, nil, metav1.ConditionTrue, ing.ExtensionsV1beta1().GetGeneration())
+				}
+			} else {
+				log.Errorw("failed split namespace/name",
+					zap.Error(errLocal),
+				)
+			}
+		}
 		c.workqueue.Forget(obj)
 		return
 	}
 	log.Warnw("sync ingress failed, will retry",
 		zap.Any("object", obj),
 		zap.Error(err),
 	)
+
+	if errLocal == nil {
+		switch ing.GroupVersion() {
+		case kube.IngressV1:
+			c.controller.recordStatus(ing.V1(), _resourceSynced, err, metav1.ConditionTrue, ing.V1().GetGeneration())

Review comment:
       ```suggestion
   			c.controller.recordStatus(ing.V1(), _resourceSyncAborted, err, metav1.ConditionTrue, ing.V1().GetGeneration())
   ```




-- 
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 edited a comment on pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#issuecomment-962736571


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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 [#740](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a46283a) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/f4708675c6304ad019881ad7e0ac7a0affd3e6bd?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f470867) will **decrease** coverage by `0.20%`.
   > The diff coverage is `11.88%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #740      +/-   ##
   ==========================================
   - Coverage   32.28%   32.08%   -0.21%     
   ==========================================
     Files          66       66              
     Lines        6808     6944     +136     
   ==========================================
   + Hits         2198     2228      +30     
   - Misses       4353     4466     +113     
   + Partials      257      250       -7     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3JvdXRlLmdv) | `0.00% <ø> (ø)` | |
   | [pkg/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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.89% <0.00%> (-1.22%)` | :arrow_down: |
   | [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3Mvc3RhdHVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.77% <100.00%> (+1.95%)` | :arrow_up: |
   | [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `65.68% <100.00%> (+0.68%)` | :arrow_up: |
   | [pkg/apisix/route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2FwaXNpeC9yb3V0ZS5nbw==) | `38.92% <0.00%> (+2.01%)` | :arrow_up: |
   | [pkg/apisix/cluster.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2FwaXNpeC9jbHVzdGVyLmdv) | `33.33% <0.00%> (+3.30%)` | :arrow_up: |
   | [pkg/apisix/plugin.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2FwaXNpeC9wbHVnaW4uZ28=) | `100.00% <0.00%> (+20.00%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/740?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 [f470867...a46283a](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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] codecov-commenter commented on pull request #740: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#issuecomment-962736571


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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 [#740](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (021ec93) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/65f7c88193eb6e83b2bb4ca87a981321a99503e5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (65f7c88) will **decrease** coverage by `0.18%`.
   > The diff coverage is `17.64%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #740      +/-   ##
   ==========================================
   - Coverage   31.70%   31.51%   -0.19%     
   ==========================================
     Files          66       65       -1     
     Lines        6640     6683      +43     
   ==========================================
   + Hits         2105     2106       +1     
   - Misses       4280     4322      +42     
     Partials      255      255              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3JvdXRlLmdv) | `0.00% <ø> (ø)` | |
   | [pkg/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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==) | `7.03% <0.00%> (-1.27%)` | :arrow_down: |
   | [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3Mvc3RhdHVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `63.33% <100.00%> (+0.83%)` | :arrow_up: |
   | [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-dGVzdC9lMmUvZTJlLmdv) | | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/740?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 [65f7c88...021ec93](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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] tokers commented on a change in pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#discussion_r748950390



##########
File path: pkg/ingress/ingress.go
##########
@@ -184,14 +185,64 @@ func (c *ingressController) sync(ctx context.Context, ev *types.Event) error {
 }
 
 func (c *ingressController) handleSyncErr(obj interface{}, err error) {
+	ev := obj.(*types.Event)
+	event := ev.Object.(kube.IngressEvent)
+	namespace, name, errLocal := cache.SplitMetaNamespaceKey(event.Key)
+	if errLocal != nil {
+		log.Errorf("invalid resource key: %s", event.Key)
+		return
+	}
+
+	var ing kube.Ingress
+	switch event.GroupVersion {
+	case kube.IngressV1:
+		ing, err = c.controller.ingressLister.V1(namespace, name)
+	case kube.IngressV1beta1:
+		ing, err = c.controller.ingressLister.V1beta1(namespace, name)
+	case kube.IngressExtensionsV1beta1:
+		ing, err = c.controller.ingressLister.ExtensionsV1beta1(namespace, name)
+	}
+
 	if err == nil {
+		// add status
+		if ev.Type != types.EventDelete {
+			if errLocal == nil {
+				switch ing.GroupVersion() {
+				case kube.IngressV1:
+					c.controller.recordStatus(ing.V1(), _resourceSynced, nil, metav1.ConditionTrue, ing.V1().GetGeneration())
+				case kube.IngressV1beta1:
+					c.controller.recordStatus(ing.V1beta1(), _resourceSynced, nil, metav1.ConditionTrue, ing.V1beta1().GetGeneration())
+				case kube.IngressExtensionsV1beta1:
+					c.controller.recordStatus(ing.ExtensionsV1beta1(), _resourceSynced, nil, metav1.ConditionTrue, ing.ExtensionsV1beta1().GetGeneration())
+				}
+			} else {
+				log.Errorw("failed split namespace/name",
+					zap.Error(errLocal),
+				)
+			}
+		}
 		c.workqueue.Forget(obj)
 		return
 	}
 	log.Warnw("sync ingress failed, will retry",
 		zap.Any("object", obj),
 		zap.Error(err),
 	)
+
+	if errLocal == nil {
+		switch ing.GroupVersion() {
+		case kube.IngressV1:
+			c.controller.recordStatus(ing.V1(), _resourceSynced, err, metav1.ConditionTrue, ing.V1().GetGeneration())

Review comment:
       This is the case that the `errLocal` is `nil`.




-- 
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 edited a comment on pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#issuecomment-962736571


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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 [#740](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d6ae069) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/4a862e206602ae9c7ac534fdfd9a557748b9ad26?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4a862e2) will **decrease** coverage by `0.57%`.
   > The diff coverage is `6.66%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #740      +/-   ##
   ==========================================
   - Coverage   31.70%   31.13%   -0.58%     
   ==========================================
     Files          66       66              
     Lines        6640     6768     +128     
   ==========================================
   + Hits         2105     2107       +2     
   - Misses       4280     4406     +126     
     Partials      255      255              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3JvdXRlLmdv) | `0.00% <ø> (ø)` | |
   | [pkg/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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==) | `7.03% <0.00%> (-1.27%)` | :arrow_down: |
   | [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3Mvc3RhdHVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `63.33% <100.00%> (+0.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/740?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 [774077a...d6ae069](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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] codecov-commenter edited a comment on pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#issuecomment-962736571


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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 [#740](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8e29e5f) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/4a862e206602ae9c7ac534fdfd9a557748b9ad26?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4a862e2) will **decrease** coverage by `0.50%`.
   > The diff coverage is `11.26%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #740      +/-   ##
   ==========================================
   - Coverage   31.70%   31.19%   -0.51%     
   ==========================================
     Files          66       65       -1     
     Lines        6640     6774     +134     
   ==========================================
   + Hits         2105     2113       +8     
   - Misses       4280     4406     +126     
     Partials      255      255              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3JvdXRlLmdv) | `0.00% <ø> (ø)` | |
   | [pkg/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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==) | `7.03% <0.00%> (-1.27%)` | :arrow_down: |
   | [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3Mvc3RhdHVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.55% <100.00%> (+1.72%)` | :arrow_up: |
   | [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `63.33% <100.00%> (+0.83%)` | :arrow_up: |
   | [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-dGVzdC9lMmUvZTJlLmdv) | | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/740?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 [774077a...8e29e5f](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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] tao12345666333 commented on a change in pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#discussion_r760117937



##########
File path: pkg/ingress/status.go
##########
@@ -163,8 +173,142 @@ func (c *Controller) recordStatus(at interface{}, reason string, err error, stat
 				)
 			}
 		}
+	case *networkingv1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+
+	case *networkingv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+	case *extensionsv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.ExtensionsV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
 	default:
 		// This should not be executed
 		log.Errorf("unsupported resource record: %s", v)
 	}
 }
+
+// ingressPublishAddresses get addressed used to expose Ingress
+func (c *Controller) ingressPublishAddresses() ([]string, error) {
+	ingressPublishService := c.cfg.IngressPublishService
+	ingressStatusAddress := c.cfg.IngressStatusAddress
+	addrs := []string{}
+
+	// if ingressStatusAddress is specified, it will be used first
+	if len(ingressStatusAddress) > 0 {
+		addrs = append(addrs, ingressStatusAddress...)
+		return addrs, nil
+	}
+
+	namespace, name, err := cache.SplitMetaNamespaceKey(ingressPublishService)

Review comment:
       But users may not configure Ingress resources.
   
   If the user configures this option, but he only uses custom resources, do we also need to report an error? And if there is any error, it will not affect the actual use of the user.




-- 
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 #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#issuecomment-983574238


   I will fix CI ASAP


-- 
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 edited a comment on pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#issuecomment-962736571


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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 [#740](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (554c677) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/4a862e206602ae9c7ac534fdfd9a557748b9ad26?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4a862e2) will **decrease** coverage by `0.49%`.
   > The diff coverage is `11.26%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #740      +/-   ##
   ==========================================
   - Coverage   31.70%   31.20%   -0.50%     
   ==========================================
     Files          66       66              
     Lines        6640     6775     +135     
   ==========================================
   + Hits         2105     2114       +9     
   - Misses       4280     4406     +126     
     Partials      255      255              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3JvdXRlLmdv) | `0.00% <ø> (ø)` | |
   | [pkg/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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==) | `7.03% <0.00%> (-1.27%)` | :arrow_down: |
   | [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3Mvc3RhdHVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.55% <100.00%> (+1.72%)` | :arrow_up: |
   | [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `63.33% <100.00%> (+0.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/740?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 [774077a...554c677](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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] codecov-commenter edited a comment on pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#issuecomment-962736571


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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 [#740](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (01d7079) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/4a862e206602ae9c7ac534fdfd9a557748b9ad26?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4a862e2) will **decrease** coverage by `0.21%`.
   > The diff coverage is `11.26%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #740      +/-   ##
   ==========================================
   - Coverage   31.70%   31.48%   -0.22%     
   ==========================================
     Files          66       65       -1     
     Lines        6640     6774     +134     
   ==========================================
   + Hits         2105     2133      +28     
   - Misses       4280     4393     +113     
   + Partials      255      248       -7     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3JvdXRlLmdv) | `0.00% <ø> (ø)` | |
   | [pkg/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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==) | `7.03% <0.00%> (-1.27%)` | :arrow_down: |
   | [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3Mvc3RhdHVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.55% <100.00%> (+1.72%)` | :arrow_up: |
   | [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `63.33% <100.00%> (+0.83%)` | :arrow_up: |
   | [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-dGVzdC9lMmUvZTJlLmdv) | | |
   | [pkg/apisix/route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2FwaXNpeC9yb3V0ZS5nbw==) | `37.76% <0.00%> (+2.09%)` | :arrow_up: |
   | [pkg/apisix/cluster.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2FwaXNpeC9jbHVzdGVyLmdv) | `30.75% <0.00%> (+3.50%)` | :arrow_up: |
   | [pkg/apisix/plugin.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2FwaXNpeC9wbHVnaW4uZ28=) | `100.00% <0.00%> (+20.00%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/740?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 [774077a...01d7079](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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] tao12345666333 commented on a change in pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on a change in pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#discussion_r760119492



##########
File path: pkg/ingress/status.go
##########
@@ -163,8 +173,142 @@ func (c *Controller) recordStatus(at interface{}, reason string, err error, stat
 				)
 			}
 		}
+	case *networkingv1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+
+	case *networkingv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+	case *extensionsv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.ExtensionsV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
 	default:
 		// This should not be executed
 		log.Errorf("unsupported resource record: %s", v)
 	}
 }
+
+// ingressPublishAddresses get addressed used to expose Ingress
+func (c *Controller) ingressPublishAddresses() ([]string, error) {
+	ingressPublishService := c.cfg.IngressPublishService
+	ingressStatusAddress := c.cfg.IngressStatusAddress
+	addrs := []string{}
+
+	// if ingressStatusAddress is specified, it will be used first
+	if len(ingressStatusAddress) > 0 {
+		addrs = append(addrs, ingressStatusAddress...)
+		return addrs, nil
+	}
+
+	namespace, name, err := cache.SplitMetaNamespaceKey(ingressPublishService)
+	if err != nil {
+		log.Errorf("invalid ingressPublishService %s: %s", ingressPublishService, err)
+		return nil, err
+	}
+
+	kubeClient := c.kubeClient.Client
+	svc, err := kubeClient.CoreV1().Services(namespace).Get(context.TODO(), name, metav1.GetOptions{})
+	if err != nil {
+		return nil, err
+	}
+
+	switch svc.Spec.Type {
+	case apiv1.ServiceTypeLoadBalancer:
+		if len(svc.Status.LoadBalancer.Ingress) < 1 {
+			return addrs, fmt.Errorf(_gatewayLBNotReadyMessage)
+		}
+
+		for _, ip := range svc.Status.LoadBalancer.Ingress {
+			if ip.IP == "" {
+				// typically AWS load-balancers
+				addrs = append(addrs, ip.Hostname)
+			} else {
+				addrs = append(addrs, ip.IP)
+			}
+		}
+
+		addrs = append(addrs, svc.Spec.ExternalIPs...)
+		return addrs, nil
+	default:
+		return addrs, nil
+	}
+
+}
+
+// ingressLBStatusIPs organizes the available addresses
+func (c *Controller) ingressLBStatusIPs() ([]apiv1.LoadBalancerIngress, error) {
+	lbips := []apiv1.LoadBalancerIngress{}
+	var ips []string
+
+	for {

Review comment:
       The implementation of the wait package also uses the for statement, we can optimize it later.




-- 
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 #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
tao12345666333 merged pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740


   


-- 
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 edited a comment on pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#issuecomment-962736571


   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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 [#740](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a46283a) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/f4708675c6304ad019881ad7e0ac7a0affd3e6bd?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f470867) will **decrease** coverage by `0.21%`.
   > The diff coverage is `11.88%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/graphs/tree.svg?width=650&height=150&src=pr&token=WPLQXPY3V0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #740      +/-   ##
   ==========================================
   - Coverage   32.28%   32.07%   -0.22%     
   ==========================================
     Files          66       65       -1     
     Lines        6808     6943     +135     
   ==========================================
   + Hits         2198     2227      +29     
   - Misses       4353     4466     +113     
   + Partials      257      250       -7     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3JvdXRlLmdv) | `0.00% <ø> (ø)` | |
   | [pkg/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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.89% <0.00%> (-1.22%)` | :arrow_down: |
   | [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2luZ3Jlc3Mvc3RhdHVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.77% <100.00%> (+1.95%)` | :arrow_up: |
   | [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `65.68% <100.00%> (+0.68%)` | :arrow_up: |
   | [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-dGVzdC9lMmUvZTJlLmdv) | | |
   | [pkg/apisix/route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2FwaXNpeC9yb3V0ZS5nbw==) | `38.92% <0.00%> (+2.01%)` | :arrow_up: |
   | [pkg/apisix/cluster.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2FwaXNpeC9jbHVzdGVyLmdv) | `33.33% <0.00%> (+3.30%)` | :arrow_up: |
   | [pkg/apisix/plugin.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740/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-cGtnL2FwaXNpeC9wbHVnaW4uZ28=) | `100.00% <0.00%> (+20.00%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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/740?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 [f470867...a46283a](https://codecov.io/gh/apache/apisix-ingress-controller/pull/740?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] tokers commented on a change in pull request #740: feat: Fix lb ingress status

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #740:
URL: https://github.com/apache/apisix-ingress-controller/pull/740#discussion_r748951999



##########
File path: pkg/ingress/status.go
##########
@@ -163,8 +173,142 @@ func (c *Controller) recordStatus(at interface{}, reason string, err error, stat
 				)
 			}
 		}
+	case *networkingv1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+
+	case *networkingv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+	case *extensionsv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.ExtensionsV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
 	default:
 		// This should not be executed
 		log.Errorf("unsupported resource record: %s", v)
 	}
 }
+
+// ingressPublishAddresses get addressed used to expose Ingress
+func (c *Controller) ingressPublishAddresses() ([]string, error) {
+	ingressPublishService := c.cfg.IngressPublishService
+	ingressStatusAddress := c.cfg.IngressStatusAddress
+	addrs := []string{}
+
+	// if ingressStatusAddress is specified, it will be used first
+	if len(ingressStatusAddress) > 0 {
+		addrs = append(addrs, ingressStatusAddress...)
+		return addrs, nil

Review comment:
       As per the description of this option, the static addresses should only be used if the LB is not ready.

##########
File path: pkg/ingress/status.go
##########
@@ -163,8 +173,142 @@ func (c *Controller) recordStatus(at interface{}, reason string, err error, stat
 				)
 			}
 		}
+	case *networkingv1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+
+	case *networkingv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+	case *extensionsv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.ExtensionsV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
 	default:
 		// This should not be executed
 		log.Errorf("unsupported resource record: %s", v)
 	}
 }
+
+// ingressPublishAddresses get addressed used to expose Ingress
+func (c *Controller) ingressPublishAddresses() ([]string, error) {
+	ingressPublishService := c.cfg.IngressPublishService
+	ingressStatusAddress := c.cfg.IngressStatusAddress
+	addrs := []string{}
+
+	// if ingressStatusAddress is specified, it will be used first
+	if len(ingressStatusAddress) > 0 {
+		addrs = append(addrs, ingressStatusAddress...)
+		return addrs, nil
+	}
+
+	namespace, name, err := cache.SplitMetaNamespaceKey(ingressPublishService)

Review comment:
       The separation  can be handled in advance.

##########
File path: pkg/ingress/status.go
##########
@@ -163,8 +173,142 @@ func (c *Controller) recordStatus(at interface{}, reason string, err error, stat
 				)
 			}
 		}
+	case *networkingv1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+
+	case *networkingv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.NetworkingV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
+	case *extensionsv1beta1.Ingress:
+		// set to status
+		lbips, err := c.ingressLBStatusIPs()
+		if err != nil {
+			log.Errorw("failed to get APISIX gateway external IPs",
+				zap.Error(err),
+			)
+
+		}
+
+		v.Status.LoadBalancer.Ingress = lbips
+		if _, errRecord := kubeClient.ExtensionsV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+			log.Errorw("failed to record status change for IngressV1",
+				zap.Error(errRecord),
+				zap.String("name", v.Name),
+				zap.String("namespace", v.Namespace),
+			)
+		}
 	default:
 		// This should not be executed
 		log.Errorf("unsupported resource record: %s", v)
 	}
 }
+
+// ingressPublishAddresses get addressed used to expose Ingress
+func (c *Controller) ingressPublishAddresses() ([]string, error) {
+	ingressPublishService := c.cfg.IngressPublishService
+	ingressStatusAddress := c.cfg.IngressStatusAddress
+	addrs := []string{}
+
+	// if ingressStatusAddress is specified, it will be used first
+	if len(ingressStatusAddress) > 0 {
+		addrs = append(addrs, ingressStatusAddress...)
+		return addrs, nil
+	}
+
+	namespace, name, err := cache.SplitMetaNamespaceKey(ingressPublishService)
+	if err != nil {
+		log.Errorf("invalid ingressPublishService %s: %s", ingressPublishService, err)
+		return nil, err
+	}
+
+	kubeClient := c.kubeClient.Client
+	svc, err := kubeClient.CoreV1().Services(namespace).Get(context.TODO(), name, metav1.GetOptions{})
+	if err != nil {
+		return nil, err
+	}
+
+	switch svc.Spec.Type {
+	case apiv1.ServiceTypeLoadBalancer:
+		if len(svc.Status.LoadBalancer.Ingress) < 1 {
+			return addrs, fmt.Errorf(_gatewayLBNotReadyMessage)
+		}
+
+		for _, ip := range svc.Status.LoadBalancer.Ingress {
+			if ip.IP == "" {
+				// typically AWS load-balancers
+				addrs = append(addrs, ip.Hostname)
+			} else {
+				addrs = append(addrs, ip.IP)
+			}
+		}
+
+		addrs = append(addrs, svc.Spec.ExternalIPs...)
+		return addrs, nil
+	default:
+		return addrs, nil
+	}
+
+}
+
+// ingressLBStatusIPs organizes the available addresses
+func (c *Controller) ingressLBStatusIPs() ([]apiv1.LoadBalancerIngress, error) {
+	lbips := []apiv1.LoadBalancerIngress{}
+	var ips []string
+
+	for {

Review comment:
       Use https://pkg.go.dev/k8s.io/apimachinery/pkg/util/wait




-- 
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