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/02/24 08:59:26 UTC
[GitHub] [apisix-ingress-controller] tokers opened a new pull request #265: feat: use unified translator to translage ApisixRoute v1
tokers opened a new pull request #265:
URL: https://github.com/apache/apisix-ingress-controller/pull/265
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
___
### 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] gxthrj commented on a change in pull request #265: feat: use unified translator to translate ApisixRoute v1
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #265:
URL: https://github.com/apache/apisix-ingress-controller/pull/265#discussion_r581868685
##########
File path: pkg/ingress/apisix/tls.go
##########
@@ -32,8 +31,6 @@ type ApisixTLSCRD configv1.ApisixTls
func (as *ApisixTLSCRD) Convert(sc Secreter) (*apisix.Ssl, error) {
name := as.Name
namespace := as.Namespace
- _, group := BuildAnnotation(as.Annotations)
- conf.AddGroup(group)
Review comment:
We also need to explain on the document that the use of ingress.class has changed and is consistent with the official k8s
##########
File path: pkg/ingress/apisix/tls_test.go
##########
@@ -69,46 +69,6 @@ spec:
assert.EqualValues(t, sslExpect.Group, ssl.Group, "group convert error")
}
-func TestConvert_group_annotation(t *testing.T) {
Review comment:
Why remove this test case
----------------------------------------------------------------
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 #265: feat: use unified translator to translate ApisixRoute v1
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #265:
URL: https://github.com/apache/apisix-ingress-controller/pull/265#discussion_r582413811
##########
File path: pkg/ingress/apisix/tls.go
##########
@@ -32,8 +31,6 @@ type ApisixTLSCRD configv1.ApisixTls
func (as *ApisixTLSCRD) Convert(sc Secreter) (*apisix.Ssl, error) {
name := as.Name
namespace := as.Namespace
- _, group := BuildAnnotation(as.Annotations)
- conf.AddGroup(group)
Review comment:
Will add to document in the `ApisixRoute v2alpha1 ` 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] tokers commented on a change in pull request #265: feat: use unified translator to translate ApisixRoute v1
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #265:
URL: https://github.com/apache/apisix-ingress-controller/pull/265#discussion_r582413984
##########
File path: pkg/ingress/apisix/tls_test.go
##########
@@ -69,46 +69,6 @@ spec:
assert.EqualValues(t, sslExpect.Group, ssl.Group, "group convert error")
}
-func TestConvert_group_annotation(t *testing.T) {
Review comment:
The `ingress.class` now doesn't have the mean for multiple cluster. Also, the multiple cluster feature doesn't work at all. So I think we can remove 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 merged pull request #265: feat: use unified translator to translate ApisixRoute v1
Posted by GitBox <gi...@apache.org>.
gxthrj merged pull request #265:
URL: https://github.com/apache/apisix-ingress-controller/pull/265
----------------------------------------------------------------
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 #265: feat: use unified translator to translate ApisixRoute v1
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #265:
URL: https://github.com/apache/apisix-ingress-controller/pull/265#discussion_r582413984
##########
File path: pkg/ingress/apisix/tls_test.go
##########
@@ -69,46 +69,6 @@ spec:
assert.EqualValues(t, sslExpect.Group, ssl.Group, "group convert error")
}
-func TestConvert_group_annotation(t *testing.T) {
Review comment:
The `ingress.class` now doesn't have the mean for multiple cluster.
----------------------------------------------------------------
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 pull request #265: feat: use unified translator to translage ApisixRoute v1
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #265:
URL: https://github.com/apache/apisix-ingress-controller/pull/265#issuecomment-784919213
@gxthrj Please take a look when you have time.
----------------------------------------------------------------
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