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 2020/12/17 14:16:54 UTC
[GitHub] [apisix-ingress-controller] gxthrj opened a new pull request #106: fix: tls remove failed & Add a node placeholder when the num of nodes…
gxthrj opened a new pull request #106:
URL: https://github.com/apache/apisix-ingress-controller/pull/106
… in upstream is 0
----------------------------------------------------------------
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 merged pull request #106: fix: tls remove failed
Posted by GitBox <gi...@apache.org>.
gxthrj merged pull request #106:
URL: https://github.com/apache/apisix-ingress-controller/pull/106
----------------------------------------------------------------
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 #106: fix: tls remove failed
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #106:
URL: https://github.com/apache/apisix-ingress-controller/pull/106#discussion_r545544964
##########
File path: docs/FAQ.md
##########
@@ -38,3 +38,13 @@ This is because CRDs are generally declared in the file system, and Apply to ent
So far apisix-ingress-controller doesn't support set admin_key for Apache APISIX, so when you deploy your APISIX cluster, admin_key should be removed from config.
Note since APISIX have two configuration files, the first is config.yaml, which contains the user specified configs, the other is config-default.yaml, which has all default items, config items in these two files will be merged. So admin_key in both files should be removed. You can customize these two configuration files and mount them to APISIX deloyment.
+
+5. Failed to create route with `ApisixRoute`?
+
+When `apisix-ingress-controller` creates a route with CRD, it checks the `Endpoint` resources in Kubernetes (matched by namespace_name_port). If the corresponding endpoint information is not found, the route will not be created and wait for the next retry.
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-io edited a comment on pull request #106: fix: tls remove failed
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #106:
URL: https://github.com/apache/apisix-ingress-controller/pull/106#issuecomment-747466476
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=h1) Report
> Merging [#106](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=desc) (a5db7af) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/983dbb02105d450ac74cf8da48f7c457f2c78d26?el=desc) (983dbb0) will **increase** coverage by `0.79%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #106 +/- ##
==========================================
+ Coverage 59.73% 60.53% +0.79%
==========================================
Files 16 16
Lines 524 527 +3
==========================================
+ Hits 313 319 +6
+ Misses 187 184 -3
Partials 24 24
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/ingress/apisix/tls.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106/diff?src=pr&el=tree#diff-cGtnL2luZ3Jlc3MvYXBpc2l4L3Rscy5nbw==) | `92.00% <100.00%> (+1.09%)` | :arrow_up: |
| [pkg/ingress/apisix/annotation.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106/diff?src=pr&el=tree#diff-cGtnL2luZ3Jlc3MvYXBpc2l4L2Fubm90YXRpb24uZ28=) | `30.43% <0.00%> (+13.04%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=footer). Last update [983dbb0...a5db7af](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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 #106: fix: tls remove failed & Add a node placeholder when the num of nodes…
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #106:
URL: https://github.com/apache/apisix-ingress-controller/pull/106#discussion_r545526590
##########
File path: pkg/ingress/endpoint/ep.go
##########
@@ -60,5 +60,12 @@ func BuildEps(ns, name string, port int) []*v1.Node {
}
}
}
+ if len(nodes) < 1 {
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] tokers commented on a change in pull request #106: fix: tls remove failed
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #106:
URL: https://github.com/apache/apisix-ingress-controller/pull/106#discussion_r545543007
##########
File path: docs/FAQ.md
##########
@@ -38,3 +38,13 @@ This is because CRDs are generally declared in the file system, and Apply to ent
So far apisix-ingress-controller doesn't support set admin_key for Apache APISIX, so when you deploy your APISIX cluster, admin_key should be removed from config.
Note since APISIX have two configuration files, the first is config.yaml, which contains the user specified configs, the other is config-default.yaml, which has all default items, config items in these two files will be merged. So admin_key in both files should be removed. You can customize these two configuration files and mount them to APISIX deloyment.
+
+5. Failed to create route with `ApisixRoute`?
+
+When `apisix-ingress-controller` creates a route with CRD, it checks the `Endpoint` resources in Kubernetes (matched by namespace_name_port). If the corresponding endpoint information is not found, the route will not be created and wait for the next retry.
Review comment:
We should note that this is the limitation of APISIX.
----------------------------------------------------------------
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 #106: fix: tls remove failed & Add a node placeholder when the num of nodes…
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #106:
URL: https://github.com/apache/apisix-ingress-controller/pull/106#discussion_r545504041
##########
File path: pkg/ingress/endpoint/ep.go
##########
@@ -60,5 +60,12 @@ func BuildEps(ns, name string, port int) []*v1.Node {
}
}
}
+ if len(nodes) < 1 {
Review comment:
That's not a right behavior, if we really cannot collect the endpoint information, we shouldn't push this upstream to APISIX, instead, logging it and just throw it away.
----------------------------------------------------------------
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 #106: fix: tls remove failed & Add a node placeholder when the num of nodes…
Posted by GitBox <gi...@apache.org>.
gxthrj commented on pull request #106:
URL: https://github.com/apache/apisix-ingress-controller/pull/106#issuecomment-747821743
> no test case?
Added some test case for group field.
----------------------------------------------------------------
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-io commented on pull request #106: fix: tls remove failed & Add a node placeholder when the num of nodes…
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #106:
URL: https://github.com/apache/apisix-ingress-controller/pull/106#issuecomment-747466476
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=h1) Report
> Merging [#106](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=desc) (6b1058b) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/983dbb02105d450ac74cf8da48f7c457f2c78d26?el=desc) (983dbb0) will **increase** coverage by `0.22%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #106 +/- ##
==========================================
+ Coverage 59.73% 59.96% +0.22%
==========================================
Files 16 16
Lines 524 527 +3
==========================================
+ Hits 313 316 +3
Misses 187 187
Partials 24 24
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/ingress/apisix/tls.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106/diff?src=pr&el=tree#diff-cGtnL2luZ3Jlc3MvYXBpc2l4L3Rscy5nbw==) | `92.00% <100.00%> (+1.09%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=footer). Last update [983dbb0...6b1058b](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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 #106: fix: tls remove failed & Add a node placeholder when the num of nodes…
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #106:
URL: https://github.com/apache/apisix-ingress-controller/pull/106#discussion_r545515687
##########
File path: pkg/ingress/endpoint/ep.go
##########
@@ -60,5 +60,12 @@ func BuildEps(ns, name string, port int) []*v1.Node {
}
}
}
+ if len(nodes) < 1 {
Review comment:
ok, we do synchronization before sync, if there is no node, we will try again.
----------------------------------------------------------------
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-io edited a comment on pull request #106: fix: tls remove failed & Add a node placeholder when the num of nodes…
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #106:
URL: https://github.com/apache/apisix-ingress-controller/pull/106#issuecomment-747466476
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=h1) Report
> Merging [#106](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=desc) (9ceb452) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/983dbb02105d450ac74cf8da48f7c457f2c78d26?el=desc) (983dbb0) will **increase** coverage by `0.79%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #106 +/- ##
==========================================
+ Coverage 59.73% 60.53% +0.79%
==========================================
Files 16 16
Lines 524 527 +3
==========================================
+ Hits 313 319 +6
+ Misses 187 184 -3
Partials 24 24
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/ingress/apisix/tls.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106/diff?src=pr&el=tree#diff-cGtnL2luZ3Jlc3MvYXBpc2l4L3Rscy5nbw==) | `92.00% <100.00%> (+1.09%)` | :arrow_up: |
| [pkg/ingress/apisix/annotation.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106/diff?src=pr&el=tree#diff-cGtnL2luZ3Jlc3MvYXBpc2l4L2Fubm90YXRpb24uZ28=) | `30.43% <0.00%> (+13.04%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=footer). Last update [983dbb0...9ceb452](https://codecov.io/gh/apache/apisix-ingress-controller/pull/106?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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