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/25 07:05:19 UTC
[GitHub] [apisix-ingress-controller] tokers opened a new issue #133: Refactor: Optimize process of resources push to APISIX
tokers opened a new issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133
When `ApisixRoute`, `ApisixUpstream` and other CRDs are watched by ingress controller, it converts these objects to the format of APISIX Resources, like Route, Upstream and etc.
Currently, one rule in `ApisixRoute` object may generate a Route, Service, Upstream object; an `APISIXService` may generate
a Service and Upstream object; an `ApisixUpstream` will generate an Upstream object. What's more, the order to push them (to APISIX) is important, we must push Upstream before Service, push Service before Route.
Some drawbacks in current implementation:
* the Service object generated by `ApisixRoute` is useless since all configurations in it are also in the Route, we can remove it;
* also, logics for Route, Service, Upstream are highly duplicated, so we can abstract them to reduce the code complexity;
* Resource cascading update are implemented in several different places with the channel to communicate and keep the order, which is complex, we can simplify it by writing sequential codes.
Operations for resources like Route, Service, Upstream and others can be abstracted as a `ResourceOperator` interface.
```go
// pseudo code
// ResourceOperator abstracts all necessary operations for operating an APISIX resource like Route, Upstream.
type ResourceOperator interface {
Get() ([]interface{}, error)
Add(obj interface{}) error
Update(obj interface{}) error
Delete(id string) error
DiffFrom(t interface{}) (bool, error)
.......
}
func NewRouteOperator() ResourceOperator {}
func NewServiceOperator() ResourceOperator {}
func NewUpstreamOperator() ResourceOperator {}
```
For each resource change, we have a struct `ResourceChange`
```go
type ResourceChange struct {
operator ResourceOperator
changeReason string // for logging
}
func ApplyResourceChange(rc *ResourceChange) error {}
```
On a higher land, we can orchestrate the resource change to `ResourcesChange`
```go
type ResourcesChange [][]ResourceChange
func (rsc *ResourceChange) Apply() error {}
```
We put Upstream in ResourcesChange[0] which means it has the first priority to apply, ResourcesChange[1] for Service and so on.
Note the whole apply process can not be atomic, we relay on the user to delete the broken CRD object if the apply process is aborted.
----------------------------------------------------------------
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 issue #133: Refactor: Optimize process of resources push to APISIX
Posted by GitBox <gi...@apache.org>.
tokers commented on issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133#issuecomment-756758069
We have firstly optimized the apisix http client by #147.
----------------------------------------------------------------
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 issue #133: Refactor: Optimize process of resources push to APISIX
Posted by GitBox <gi...@apache.org>.
tokers commented on issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133#issuecomment-756758069
We have firstly optimized the apisix http client by #147.
----------------------------------------------------------------
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 issue #133: Refactor: Optimize process of resources push to APISIX
Posted by GitBox <gi...@apache.org>.
gxthrj commented on issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133#issuecomment-751934245
> Nope, a `ResourceChange` object will be created once we need to generate a resource push event.
I means one CRD may related many Routes/services/upstreams. Do you means they are all in one `[][]ResourceChange` ?
----------------------------------------------------------------
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 closed issue #133: Refactor: Optimize process of resources push to APISIX
Posted by GitBox <gi...@apache.org>.
tokers closed issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133
----------------------------------------------------------------
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 issue #133: Refactor: Optimize process of resources push to APISIX
Posted by GitBox <gi...@apache.org>.
tokers commented on issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133#issuecomment-751952375
> > Nope, a `ResourceChange` object will be created once we need to generate a resource push event.
>
> I means one CRD may related many Routes/services/upstreams. Do you means they are all in one `[][]ResourceChange` ?
A `[][]ResourceChange` object is a group of changes that they are related, there will be many `[][]ResourceChange` but each of them are independent and non-interfering.
----------------------------------------------------------------
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 issue #133: Refactor: Optimize process of resources push to APISIX
Posted by GitBox <gi...@apache.org>.
gxthrj commented on issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133#issuecomment-751930259
> the Service object generated by ApisixRoute is useless since all configurations in it are also in the Route, we can remove it;
There is not much frequency of using `Service` Resource, but creating a service can unify the overall structure (route - service - upstream) and make the logic easier to abstract.
> also, logics for Route, Service, Upstream are highly duplicated, so we can abstract them to reduce the code complexity;
Agree with this.
> Resource cascading update are implemented in several different places with the channel to communicate and keep the order, which is complex, we can simplify it by writing sequential codes.
Previously, I used the event-based notification mechanism implemented by channel, which allows events to arrive at the specified Resource in an orderly and precise manner.
* The sequential codes just keep the order `upstream` -> `service` -> `route`, which is not enough. When deleting resources, the order is reversed. The orders is different in different the operation type .
* Another question is , Is `ResourcesChange ` a global Object ? As what you said `the whole apply process can not be atomic`, It will cause race here.
----------------------------------------------------------------
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 issue #133: Refactor: Optimize process of resources push to APISIX
Posted by GitBox <gi...@apache.org>.
tokers commented on issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133#issuecomment-770595608
Closed since #147 merged.
----------------------------------------------------------------
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 edited a comment on issue #133: Refactor: Optimize process of resources push to APISIX
Posted by GitBox <gi...@apache.org>.
gxthrj edited a comment on issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133#issuecomment-751933082
> I believe the consideration about unifying overall structure can be also achieved if the design is good enough.
Need some specific design about how to unifying the relation `route - upstream` or `route - service` or `service - upstream` and so on.
----------------------------------------------------------------
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 issue #133: Refactor: Optimize process of resources push to APISIX
Posted by GitBox <gi...@apache.org>.
tokers commented on issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133#issuecomment-751952463
> > I believe the consideration about unifying overall structure can be also achieved if the design is good enough.
>
> Need some specific design about how to unifying the relation `route - upstream` or `route - service` or `service - upstream` and so on.
This could be a later step.
----------------------------------------------------------------
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 edited a comment on issue #133: Refactor: Optimize process of resources push to APISIX
Posted by GitBox <gi...@apache.org>.
tokers edited a comment on issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133#issuecomment-751931916
> There is not much frequency of using Service Resource, but creating a service can unify the overall structure (route - service - upstream) and make the logic easier to abstract.
I believe the consideration about unifying overall structure can be also achieved if the design is good enough.
> The sequential codes just keep the order upstream -> service -> route, which is not enough. When deleting resources, the order is reversed. The orders is different in different the operation type .
The current event-based notification also is hard coded since the order is fixed just like the sequential way.
Actually i have another idea about to abstract the dependency, but i think it's over design, so what i pasted here is the sequential way, just rewriting the implementation, but keep this fixed order.
> Another question is , Is ResourcesChange a global Object ? As what you said the whole apply process can not be atomic, It will cause race here.
Nope, a `ResourceChange` object will be created once we need to generate a resource push event.
----------------------------------------------------------------
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 issue #133: Refactor: Optimize process of resources push to APISIX
Posted by GitBox <gi...@apache.org>.
tokers commented on issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133#issuecomment-751987199
> > > I believe the consideration about unifying overall structure can be also achieved if the design is good enough.
> >
> >
> > Need some specific design about how to unifying the relation `route - upstream` or `route - service` or `service - upstream` and so on.
>
> This could be a later step.
I think we should first refactor and abstract the APISIX HTTP Client object, and use sequential way to rewrite the dependencies delivery, then considering to strip the service creation and relax the dependency order.
----------------------------------------------------------------
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 issue #133: Refactor: Optimize process of resources push to APISIX
Posted by GitBox <gi...@apache.org>.
gxthrj commented on issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133#issuecomment-751933082
> I believe the consideration about unifying overall structure can be also achieved if the design is good enough.
Need some specific design about how to unifying the relation `route - upstream` or `route - service` or `service - upstream` and so on.
----------------------------------------------------------------
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 edited a comment on issue #133: Refactor: Optimize process of resources push to APISIX
Posted by GitBox <gi...@apache.org>.
gxthrj edited a comment on issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133#issuecomment-751930259
> the Service object generated by ApisixRoute is useless since all configurations in it are also in the Route, we can remove it;
There is not much frequency of using `Service` Resource, but creating a service can unify the overall structure (route - service - upstream) and make the logic easier to abstract.
> also, logics for Route, Service, Upstream are highly duplicated, so we can abstract them to reduce the code complexity;
Agree with this.
> Resource cascading update are implemented in several different places with the channel to communicate and keep the order, which is complex, we can simplify it by writing sequential codes.
Previously, I used the event-based notification mechanism implemented by channel, which allows events to arrive at the specified Resource in an orderly and precise manner.
* The sequential codes just keep the order `upstream` -> `service` -> `route`, which is not enough. When deleting resources, the order is reversed. The orders is different in different the operation type .
* Another question is , Is `ResourcesChange ` a global Object ? As what you said `the whole apply process can not be atomic`, It will cause race here.
----------------------------------------------------------------
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 issue #133: Refactor: Optimize process of resources push to APISIX
Posted by GitBox <gi...@apache.org>.
tokers commented on issue #133:
URL: https://github.com/apache/apisix-ingress-controller/issues/133#issuecomment-751931916
> The sequential codes just keep the order upstream -> service -> route, which is not enough. When deleting resources, the order is reversed. The orders is different in different the operation type .
The current event-based notification also is hard coded since the order is fixed just like the sequential way.
Actually i have another idea about to abstract the dependency, but i think it's over design, so what i pasted here is the sequential way, just rewriting the implementation, but keep this fixed order.
> Another question is , Is ResourcesChange a global Object ? As what you said the whole apply process can not be atomic, It will cause race here.
Nope, a `ResourceChange` object will be created once we need to generate a resource push event.
----------------------------------------------------------------
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