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/04/27 16:10:58 UTC
[GitHub] [apisix-ingress-controller] gxthrj opened a new pull request #408: feat: add upstream status & tls status
gxthrj opened a new pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408
Please answer these questions before submitting a pull request
- Why submit this pull request?
- [ ] Bugfix
- [x] New feature provided
- [ ] Improve performance
- [ ] Backport patches
- Related issues
https://github.com/apache/apisix-ingress-controller/issues/395
___
### 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tokers merged pull request #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
tokers merged pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408
--
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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#discussion_r622222303
##########
File path: pkg/ingress/status.go
##########
@@ -23,30 +23,61 @@ import (
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/apache/apisix-ingress-controller/pkg/kube"
+ configv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1"
configv2alpha1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2alpha1"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
)
const (
_conditionType = "ResourcesAvailable"
_commonSuccessMessage = "Sync Successfully"
)
-// recordRouteStatus record ApisixRoute v2alpha1 status
-func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, status v1.ConditionStatus) {
+// recordStatus record resources status
+func recordStatus(at interface{}, reason string, err error, status v1.ConditionStatus) {
// build condition
+ message := _commonSuccessMessage
+ if err != nil {
+ message = err.Error()
+ }
condition := metav1.Condition{
Type: _conditionType,
Reason: reason,
Status: status,
Message: message,
}
- // set to status
- if ar.Status.Conditions == nil {
- conditions := make([]metav1.Condition, 0)
- ar.Status.Conditions = &conditions
+ switch v := at.(type) {
+ case *configv1.ApisixTls:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv1.ApisixUpstream:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv2alpha1.ApisixRoute:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(v.Namespace).
Review comment:
Added the error logs the same as upon.
--
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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#discussion_r621781482
##########
File path: pkg/ingress/status.go
##########
@@ -50,3 +52,49 @@ func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, s
_, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(ar.Namespace).
UpdateStatus(context.TODO(), ar, metav1.UpdateOptions{})
}
+
+// recordStatus record resources status
+func recordStatus(at interface{}, reason string, err error, status v1.ConditionStatus) {
+ // build condition
+ message := fmt.Sprintf(_messageResourceSynced, _component)
Review comment:
done, use `_commonSuccessMessage` instead.
--
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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#issuecomment-827734245
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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 [#408](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47b81ef) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/98555210326c02db4c2c4ca8e2a8e0a27b17c92b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9855521) will **decrease** coverage by `0.33%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head 47b81ef differs from pull request most recent head 5673a7f. Consider uploading reports for the commit 5673a7f to get more accurate results
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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 #408 +/- ##
==========================================
- Coverage 39.20% 38.86% -0.34%
==========================================
Files 41 40 -1
Lines 3301 3327 +26
==========================================
- Hits 1294 1293 -1
- Misses 1846 1873 +27
Partials 161 161
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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/408/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% <0.00%> (ø)` | |
| [pkg/ingress/apisix\_tls.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3Rscy5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/ingress/apisix\_upstream.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3Vwc3RyZWFtLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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%> (ø)` | |
| [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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/408?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/408?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 [9855521...5673a7f](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#discussion_r622117925
##########
File path: pkg/ingress/status.go
##########
@@ -50,3 +52,49 @@ func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, s
_, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(ar.Namespace).
UpdateStatus(context.TODO(), ar, metav1.UpdateOptions{})
}
+
+// recordStatus record resources status
+func recordStatus(at interface{}, reason string, err error, status v1.ConditionStatus) {
+ // build condition
+ message := fmt.Sprintf(_messageResourceSynced, _component)
+ if err != nil {
+ message = err.Error()
+ }
+ condition := metav1.Condition{
+ Type: _conditionType,
+ Reason: reason,
+ Status: status,
+ Message: message,
+ }
+
+ switch v := at.(type) {
+ case *configv1.ApisixTls:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv1.ApisixUpstream:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv2alpha1.ApisixRoute:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
Review comment:
Add `default` maybe increase readability.
But `panic` may cause trouble in prod, which is only a record.
--
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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#discussion_r622025153
##########
File path: pkg/ingress/status.go
##########
@@ -50,3 +52,49 @@ func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, s
_, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(ar.Namespace).
UpdateStatus(context.TODO(), ar, metav1.UpdateOptions{})
}
+
+// recordStatus record resources status
+func recordStatus(at interface{}, reason string, err error, status v1.ConditionStatus) {
+ // build condition
+ message := fmt.Sprintf(_messageResourceSynced, _component)
+ if err != nil {
+ message = err.Error()
+ }
+ condition := metav1.Condition{
+ Type: _conditionType,
+ Reason: reason,
+ Status: status,
+ Message: message,
+ }
+
+ switch v := at.(type) {
+ case *configv1.ApisixTls:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv1.ApisixUpstream:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv2alpha1.ApisixRoute:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
Review comment:
that's for more rigorous processing. You know that's never happening because you coded it. Onc a newbie tries to add some new features and they are not familiar with the code base, they may make themselves in trouble, and the explicit `panic` can help them capture easily in the development phase.
--
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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#discussion_r622125388
##########
File path: pkg/ingress/status.go
##########
@@ -50,3 +52,49 @@ func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, s
_, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(ar.Namespace).
UpdateStatus(context.TODO(), ar, metav1.UpdateOptions{})
}
+
+// recordStatus record resources status
+func recordStatus(at interface{}, reason string, err error, status v1.ConditionStatus) {
+ // build condition
+ message := fmt.Sprintf(_messageResourceSynced, _component)
+ if err != nil {
+ message = err.Error()
+ }
+ condition := metav1.Condition{
+ Type: _conditionType,
+ Reason: reason,
+ Status: status,
+ Message: message,
+ }
+
+ switch v := at.(type) {
+ case *configv1.ApisixTls:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv1.ApisixUpstream:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv2alpha1.ApisixRoute:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
Review comment:
I add the `defaut` case with error log.
--
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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#issuecomment-827734245
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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 [#408](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9ff369b) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/98555210326c02db4c2c4ca8e2a8e0a27b17c92b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9855521) will **decrease** coverage by `0.33%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head 9ff369b differs from pull request most recent head 5673a7f. Consider uploading reports for the commit 5673a7f to get more accurate results
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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 #408 +/- ##
==========================================
- Coverage 39.20% 38.86% -0.34%
==========================================
Files 41 40 -1
Lines 3301 3327 +26
==========================================
- Hits 1294 1293 -1
- Misses 1846 1873 +27
Partials 161 161
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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/408/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% <0.00%> (ø)` | |
| [pkg/ingress/apisix\_tls.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3Rscy5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/ingress/apisix\_upstream.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3Vwc3RyZWFtLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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%> (ø)` | |
| [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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/408?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/408?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 [9855521...5673a7f](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] gxthrj commented on pull request #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
gxthrj commented on pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#issuecomment-833171024
related to : #394
--
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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#discussion_r622676190
##########
File path: pkg/ingress/status.go
##########
@@ -55,26 +55,32 @@ func recordStatus(at interface{}, reason string, err error, status v1.ConditionS
v.Status.Conditions = &conditions
}
meta.SetStatusCondition(v.Status.Conditions, condition)
- _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
- UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ if _, errRecord := kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+ log.Errorf("resource record error: %s/%s, with err %s", v.Namespace, v.Name, errRecord.Error())
Review comment:
Your error log lost some vital contexts like recording which resource? What does the resource look like?
```suggestion
log.Errorw("failed to record status change for ApisixTls",
zap.Error(err),
zap.String("name", v.Name),
zap.String("namespace", v.Namespace),
)
```
##########
File path: pkg/ingress/status.go
##########
@@ -55,26 +55,32 @@ func recordStatus(at interface{}, reason string, err error, status v1.ConditionS
v.Status.Conditions = &conditions
}
meta.SetStatusCondition(v.Status.Conditions, condition)
- _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
- UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ if _, errRecord := kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+ log.Errorf("resource record error: %s/%s, with err %s", v.Namespace, v.Name, errRecord.Error())
+ }
case *configv1.ApisixUpstream:
// set to status
if v.Status.Conditions == nil {
conditions := make([]metav1.Condition, 0)
v.Status.Conditions = &conditions
}
meta.SetStatusCondition(v.Status.Conditions, condition)
- _, _ = kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
- UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ if _, errRecord := kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+ log.Errorf("resource record error: %s/%s, with err %s", v.Namespace, v.Name, errRecord.Error())
+ }
case *configv2alpha1.ApisixRoute:
// set to status
if v.Status.Conditions == nil {
conditions := make([]metav1.Condition, 0)
v.Status.Conditions = &conditions
}
meta.SetStatusCondition(v.Status.Conditions, condition)
- _, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(v.Namespace).
- UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ if _, errRecord := kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+ log.Errorf("resource record error: %s/%s, with err %s", v.Namespace, v.Name, errRecord.Error())
Review comment:
Ditto.
##########
File path: pkg/ingress/status.go
##########
@@ -55,26 +55,32 @@ func recordStatus(at interface{}, reason string, err error, status v1.ConditionS
v.Status.Conditions = &conditions
}
meta.SetStatusCondition(v.Status.Conditions, condition)
- _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
- UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ if _, errRecord := kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+ log.Errorf("resource record error: %s/%s, with err %s", v.Namespace, v.Name, errRecord.Error())
+ }
case *configv1.ApisixUpstream:
// set to status
if v.Status.Conditions == nil {
conditions := make([]metav1.Condition, 0)
v.Status.Conditions = &conditions
}
meta.SetStatusCondition(v.Status.Conditions, condition)
- _, _ = kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
- UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ if _, errRecord := kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+ log.Errorf("resource record error: %s/%s, with err %s", v.Namespace, v.Name, errRecord.Error())
Review comment:
Ditto.
--
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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#discussion_r622221287
##########
File path: pkg/ingress/status.go
##########
@@ -23,30 +23,61 @@ import (
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/apache/apisix-ingress-controller/pkg/kube"
+ configv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1"
configv2alpha1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2alpha1"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
)
const (
_conditionType = "ResourcesAvailable"
_commonSuccessMessage = "Sync Successfully"
)
-// recordRouteStatus record ApisixRoute v2alpha1 status
-func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, status v1.ConditionStatus) {
+// recordStatus record resources status
+func recordStatus(at interface{}, reason string, err error, status v1.ConditionStatus) {
// build condition
+ message := _commonSuccessMessage
+ if err != nil {
+ message = err.Error()
+ }
condition := metav1.Condition{
Type: _conditionType,
Reason: reason,
Status: status,
Message: message,
}
- // set to status
- if ar.Status.Conditions == nil {
- conditions := make([]metav1.Condition, 0)
- ar.Status.Conditions = &conditions
+ switch v := at.(type) {
+ case *configv1.ApisixTls:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
Review comment:
I thought about that error message should be useful.
Added the error logs.
--
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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#issuecomment-827734245
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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 [#408](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (10be335) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/98555210326c02db4c2c4ca8e2a8e0a27b17c92b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9855521) will **decrease** coverage by `0.39%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head 10be335 differs from pull request most recent head 2a05c9e. Consider uploading reports for the commit 2a05c9e to get more accurate results
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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 #408 +/- ##
==========================================
- Coverage 39.20% 38.80% -0.40%
==========================================
Files 41 40 -1
Lines 3301 3332 +31
==========================================
- Hits 1294 1293 -1
- Misses 1846 1878 +32
Partials 161 161
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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/408/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% <0.00%> (ø)` | |
| [pkg/ingress/apisix\_tls.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3Rscy5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/ingress/apisix\_upstream.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3Vwc3RyZWFtLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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%> (ø)` | |
| [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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/408?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/408?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 [9855521...2a05c9e](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#discussion_r621781656
##########
File path: pkg/ingress/status.go
##########
@@ -50,3 +52,49 @@ func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, s
_, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(ar.Namespace).
UpdateStatus(context.TODO(), ar, metav1.UpdateOptions{})
}
+
+// recordStatus record resources status
Review comment:
Done
--
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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#issuecomment-827734245
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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 [#408](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2da5a12) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/98555210326c02db4c2c4ca8e2a8e0a27b17c92b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9855521) will **decrease** coverage by `0.35%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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 #408 +/- ##
==========================================
- Coverage 39.20% 38.84% -0.36%
==========================================
Files 41 40 -1
Lines 3301 3329 +28
==========================================
- Hits 1294 1293 -1
- Misses 1846 1875 +29
Partials 161 161
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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/408/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% <0.00%> (ø)` | |
| [pkg/ingress/apisix\_tls.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3Rscy5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/ingress/apisix\_upstream.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3Vwc3RyZWFtLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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%> (ø)` | |
| [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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/408?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/408?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 [9855521...2da5a12](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#discussion_r621778936
##########
File path: pkg/kube/apisix/apis/config/v1/types.go
##########
@@ -83,7 +84,8 @@ type ApisixUpstream struct {
metav1.TypeMeta `json:",inline" yaml:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty"`
- Spec *ApisixUpstreamSpec `json:"spec,omitempty" yaml:"spec,omitempty"`
+ Spec *ApisixUpstreamSpec `json:"spec,omitempty" yaml:"spec,omitempty"`
Review comment:
This can be modified in another PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] codecov-commenter commented on pull request #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#issuecomment-827734245
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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 [#408](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d45ee6b) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/98555210326c02db4c2c4ca8e2a8e0a27b17c92b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9855521) will **increase** coverage by `0.02%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head d45ee6b differs from pull request most recent head 5830302. Consider uploading reports for the commit 5830302 to get more accurate results
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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 #408 +/- ##
==========================================
+ Coverage 39.20% 39.22% +0.02%
==========================================
Files 41 40 -1
Lines 3301 3337 +36
==========================================
+ Hits 1294 1309 +15
- Misses 1846 1869 +23
+ Partials 161 159 -2
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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/408/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% <0.00%> (ø)` | |
| [pkg/ingress/apisix\_tls.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3Rscy5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/ingress/apisix\_upstream.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3Vwc3RyZWFtLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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%> (ø)` | |
| [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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/408/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==) | `35.55% <0.00%> (+2.22%)` | :arrow_up: |
| [pkg/apisix/cluster.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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) | `36.23% <0.00%> (+6.28%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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/408?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 [9855521...5830302](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#discussion_r622210624
##########
File path: pkg/ingress/status.go
##########
@@ -23,30 +23,61 @@ import (
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/apache/apisix-ingress-controller/pkg/kube"
+ configv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1"
configv2alpha1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2alpha1"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
)
const (
_conditionType = "ResourcesAvailable"
_commonSuccessMessage = "Sync Successfully"
)
-// recordRouteStatus record ApisixRoute v2alpha1 status
-func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, status v1.ConditionStatus) {
+// recordStatus record resources status
+func recordStatus(at interface{}, reason string, err error, status v1.ConditionStatus) {
// build condition
+ message := _commonSuccessMessage
+ if err != nil {
+ message = err.Error()
+ }
condition := metav1.Condition{
Type: _conditionType,
Reason: reason,
Status: status,
Message: message,
}
- // set to status
- if ar.Status.Conditions == nil {
- conditions := make([]metav1.Condition, 0)
- ar.Status.Conditions = &conditions
+ switch v := at.(type) {
+ case *configv1.ApisixTls:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
Review comment:
Why ignore both the return values?
##########
File path: pkg/ingress/status.go
##########
@@ -50,3 +52,49 @@ func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, s
_, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(ar.Namespace).
UpdateStatus(context.TODO(), ar, metav1.UpdateOptions{})
}
+
+// recordStatus record resources status
+func recordStatus(at interface{}, reason string, err error, status v1.ConditionStatus) {
+ // build condition
+ message := fmt.Sprintf(_messageResourceSynced, _component)
+ if err != nil {
+ message = err.Error()
+ }
+ condition := metav1.Condition{
+ Type: _conditionType,
+ Reason: reason,
+ Status: status,
+ Message: message,
+ }
+
+ switch v := at.(type) {
+ case *configv1.ApisixTls:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv1.ApisixUpstream:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv2alpha1.ApisixRoute:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
Review comment:
OK.
##########
File path: pkg/kube/apisix/apis/config/v1/types.go
##########
@@ -83,7 +84,8 @@ type ApisixUpstream struct {
metav1.TypeMeta `json:",inline" yaml:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty"`
- Spec *ApisixUpstreamSpec `json:"spec,omitempty" yaml:"spec,omitempty"`
+ Spec *ApisixUpstreamSpec `json:"spec,omitempty" yaml:"spec,omitempty"`
Review comment:
Got it.
##########
File path: pkg/ingress/status.go
##########
@@ -23,30 +23,61 @@ import (
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/apache/apisix-ingress-controller/pkg/kube"
+ configv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1"
configv2alpha1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2alpha1"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
)
const (
_conditionType = "ResourcesAvailable"
_commonSuccessMessage = "Sync Successfully"
)
-// recordRouteStatus record ApisixRoute v2alpha1 status
-func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, status v1.ConditionStatus) {
+// recordStatus record resources status
+func recordStatus(at interface{}, reason string, err error, status v1.ConditionStatus) {
// build condition
+ message := _commonSuccessMessage
+ if err != nil {
+ message = err.Error()
+ }
condition := metav1.Condition{
Type: _conditionType,
Reason: reason,
Status: status,
Message: message,
}
- // set to status
- if ar.Status.Conditions == nil {
- conditions := make([]metav1.Condition, 0)
- ar.Status.Conditions = &conditions
+ switch v := at.(type) {
+ case *configv1.ApisixTls:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv1.ApisixUpstream:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv2alpha1.ApisixRoute:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(v.Namespace).
Review comment:
Ditto.
--
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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#issuecomment-827734245
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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 [#408](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (86f32d8) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/98555210326c02db4c2c4ca8e2a8e0a27b17c92b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9855521) will **decrease** coverage by `0.51%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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 #408 +/- ##
==========================================
- Coverage 39.20% 38.68% -0.52%
==========================================
Files 41 41
Lines 3301 3345 +44
==========================================
Hits 1294 1294
- Misses 1846 1890 +44
Partials 161 161
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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/408/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% <0.00%> (ø)` | |
| [pkg/ingress/apisix\_tls.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3Rscy5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/ingress/apisix\_upstream.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2luZ3Jlc3MvYXBpc2l4X3Vwc3RyZWFtLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/ingress/status.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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/408?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 [9855521...86f32d8](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#discussion_r621751777
##########
File path: pkg/ingress/status.go
##########
@@ -50,3 +52,49 @@ func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, s
_, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(ar.Namespace).
UpdateStatus(context.TODO(), ar, metav1.UpdateOptions{})
}
+
+// recordStatus record resources status
+func recordStatus(at interface{}, reason string, err error, status v1.ConditionStatus) {
+ // build condition
+ message := fmt.Sprintf(_messageResourceSynced, _component)
+ if err != nil {
+ message = err.Error()
+ }
+ condition := metav1.Condition{
+ Type: _conditionType,
+ Reason: reason,
+ Status: status,
+ Message: message,
+ }
+
+ switch v := at.(type) {
+ case *configv1.ApisixTls:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv1.ApisixUpstream:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv2alpha1.ApisixRoute:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
Review comment:
Add a `default` arm.
##########
File path: pkg/ingress/status.go
##########
@@ -50,3 +52,49 @@ func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, s
_, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(ar.Namespace).
UpdateStatus(context.TODO(), ar, metav1.UpdateOptions{})
}
+
+// recordStatus record resources status
+func recordStatus(at interface{}, reason string, err error, status v1.ConditionStatus) {
+ // build condition
+ message := fmt.Sprintf(_messageResourceSynced, _component)
Review comment:
Both `_messageResourceSynced` and `_component` are known in advance (defined as package level variables), why not just prepare this message in the `var` block so we don't have to compose it each time `recordStatus` is invoked.
##########
File path: pkg/ingress/status.go
##########
@@ -50,3 +52,49 @@ func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, s
_, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(ar.Namespace).
UpdateStatus(context.TODO(), ar, metav1.UpdateOptions{})
}
+
+// recordStatus record resources status
Review comment:
```suggestion
// recordStatus record resources status
```
##########
File path: pkg/kube/apisix/apis/config/v1/types.go
##########
@@ -83,7 +84,8 @@ type ApisixUpstream struct {
metav1.TypeMeta `json:",inline" yaml:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty"`
- Spec *ApisixUpstreamSpec `json:"spec,omitempty" yaml:"spec,omitempty"`
+ Spec *ApisixUpstreamSpec `json:"spec,omitempty" yaml:"spec,omitempty"`
Review comment:
I'm wondering whether we can just use value for `Spec` field, not pointer.
##########
File path: pkg/ingress/status.go
##########
@@ -50,3 +52,49 @@ func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, s
_, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(ar.Namespace).
UpdateStatus(context.TODO(), ar, metav1.UpdateOptions{})
}
+
+// recordStatus record resources status
+func recordStatus(at interface{}, reason string, err error, status v1.ConditionStatus) {
+ // build condition
+ message := fmt.Sprintf(_messageResourceSynced, _component)
+ if err != nil {
+ message = err.Error()
+ }
+ condition := metav1.Condition{
+ Type: _conditionType,
+ Reason: reason,
+ Status: status,
+ Message: message,
+ }
+
+ switch v := at.(type) {
+ case *configv1.ApisixTls:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv1.ApisixUpstream:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv2alpha1.ApisixRoute:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
Review comment:
As the `default` arm never should be executed, we can just panic there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#discussion_r623719738
##########
File path: pkg/ingress/status.go
##########
@@ -23,30 +23,61 @@ import (
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/apache/apisix-ingress-controller/pkg/kube"
+ configv1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1"
configv2alpha1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2alpha1"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
)
const (
_conditionType = "ResourcesAvailable"
_commonSuccessMessage = "Sync Successfully"
)
-// recordRouteStatus record ApisixRoute v2alpha1 status
-func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, status v1.ConditionStatus) {
+// recordStatus record resources status
+func recordStatus(at interface{}, reason string, err error, status v1.ConditionStatus) {
// build condition
+ message := _commonSuccessMessage
+ if err != nil {
+ message = err.Error()
+ }
condition := metav1.Condition{
Type: _conditionType,
Reason: reason,
Status: status,
Message: message,
}
- // set to status
- if ar.Status.Conditions == nil {
- conditions := make([]metav1.Condition, 0)
- ar.Status.Conditions = &conditions
+ switch v := at.(type) {
+ case *configv1.ApisixTls:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
Review comment:
Got it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#discussion_r621780238
##########
File path: pkg/ingress/status.go
##########
@@ -50,3 +52,49 @@ func recordRouteStatus(ar *configv2alpha1.ApisixRoute, reason, message string, s
_, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(ar.Namespace).
UpdateStatus(context.TODO(), ar, metav1.UpdateOptions{})
}
+
+// recordStatus record resources status
+func recordStatus(at interface{}, reason string, err error, status v1.ConditionStatus) {
+ // build condition
+ message := fmt.Sprintf(_messageResourceSynced, _component)
+ if err != nil {
+ message = err.Error()
+ }
+ condition := metav1.Condition{
+ Type: _conditionType,
+ Reason: reason,
+ Status: status,
+ Message: message,
+ }
+
+ switch v := at.(type) {
+ case *configv1.ApisixTls:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv1.ApisixUpstream:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ case *configv2alpha1.ApisixRoute:
+ // set to status
+ if v.Status.Conditions == nil {
+ conditions := make([]metav1.Condition, 0)
+ v.Status.Conditions = &conditions
+ }
+ meta.SetStatusCondition(v.Status.Conditions, condition)
+ _, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
Review comment:
I think it is not necessary.
Adding `default` will never be executed.
The status is worked as logging, `panic` is Inappropriate.
--
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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#issuecomment-827734245
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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 [#408](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3118da3) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/98555210326c02db4c2c4ca8e2a8e0a27b17c92b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9855521) will **increase** coverage by `60.79%`.
> The diff coverage is `n/a`.
> :exclamation: Current head 3118da3 differs from pull request most recent head 2da5a12. Consider uploading reports for the commit 2da5a12 to get more accurate results
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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 #408 +/- ##
============================================
+ Coverage 39.20% 100.00% +60.79%
============================================
Files 41 1 -40
Lines 3301 1 -3300
============================================
- Hits 1294 1 -1293
+ Misses 1846 0 -1846
+ Partials 161 0 -161
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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/apisix/stream\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2FwaXNpeC9zdHJlYW1fcm91dGUuZ28=) | | |
| [pkg/log/default\_logger.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2xvZy9kZWZhdWx0X2xvZ2dlci5nbw==) | | |
| [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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=) | | |
| [pkg/apisix/nonexistentclient.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2FwaXNpeC9ub25leGlzdGVudGNsaWVudC5nbw==) | | |
| [pkg/kube/translation/apisix\_ssl.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2t1YmUvdHJhbnNsYXRpb24vYXBpc2l4X3NzbC5nbw==) | | |
| [pkg/kube/translation/context.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2t1YmUvdHJhbnNsYXRpb24vY29udGV4dC5nbw==) | | |
| [pkg/apisix/cluster.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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) | | |
| [pkg/apisix/apisix.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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-cGtnL2FwaXNpeC9hcGlzaXguZ28=) | | |
| [pkg/kube/translation/translator.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2t1YmUvdHJhbnNsYXRpb24vdHJhbnNsYXRvci5nbw==) | | |
| [pkg/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/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==) | | |
| ... and [26 more](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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/408?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 [9855521...2da5a12](https://codecov.io/gh/apache/apisix-ingress-controller/pull/408?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.
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 #408: feat: add upstream status & tls status
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #408:
URL: https://github.com/apache/apisix-ingress-controller/pull/408#discussion_r623655275
##########
File path: pkg/ingress/status.go
##########
@@ -55,26 +55,32 @@ func recordStatus(at interface{}, reason string, err error, status v1.ConditionS
v.Status.Conditions = &conditions
}
meta.SetStatusCondition(v.Status.Conditions, condition)
- _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
- UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ if _, errRecord := kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+ log.Errorf("resource record error: %s/%s, with err %s", v.Namespace, v.Name, errRecord.Error())
Review comment:
Done
##########
File path: pkg/ingress/status.go
##########
@@ -55,26 +55,32 @@ func recordStatus(at interface{}, reason string, err error, status v1.ConditionS
v.Status.Conditions = &conditions
}
meta.SetStatusCondition(v.Status.Conditions, condition)
- _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
- UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ if _, errRecord := kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+ log.Errorf("resource record error: %s/%s, with err %s", v.Namespace, v.Name, errRecord.Error())
+ }
case *configv1.ApisixUpstream:
// set to status
if v.Status.Conditions == nil {
conditions := make([]metav1.Condition, 0)
v.Status.Conditions = &conditions
}
meta.SetStatusCondition(v.Status.Conditions, condition)
- _, _ = kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
- UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ if _, errRecord := kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+ log.Errorf("resource record error: %s/%s, with err %s", v.Namespace, v.Name, errRecord.Error())
Review comment:
Done
##########
File path: pkg/ingress/status.go
##########
@@ -55,26 +55,32 @@ func recordStatus(at interface{}, reason string, err error, status v1.ConditionS
v.Status.Conditions = &conditions
}
meta.SetStatusCondition(v.Status.Conditions, condition)
- _, _ = kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
- UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ if _, errRecord := kube.GetApisixClient().ApisixV1().ApisixTlses(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+ log.Errorf("resource record error: %s/%s, with err %s", v.Namespace, v.Name, errRecord.Error())
+ }
case *configv1.ApisixUpstream:
// set to status
if v.Status.Conditions == nil {
conditions := make([]metav1.Condition, 0)
v.Status.Conditions = &conditions
}
meta.SetStatusCondition(v.Status.Conditions, condition)
- _, _ = kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
- UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ if _, errRecord := kube.GetApisixClient().ApisixV1().ApisixUpstreams(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+ log.Errorf("resource record error: %s/%s, with err %s", v.Namespace, v.Name, errRecord.Error())
+ }
case *configv2alpha1.ApisixRoute:
// set to status
if v.Status.Conditions == nil {
conditions := make([]metav1.Condition, 0)
v.Status.Conditions = &conditions
}
meta.SetStatusCondition(v.Status.Conditions, condition)
- _, _ = kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(v.Namespace).
- UpdateStatus(context.TODO(), v, metav1.UpdateOptions{})
+ if _, errRecord := kube.GetApisixClient().ApisixV2alpha1().ApisixRoutes(v.Namespace).
+ UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil {
+ log.Errorf("resource record error: %s/%s, with err %s", v.Namespace, v.Name, errRecord.Error())
Review comment:
Done
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org