service: Refactor to the slice structure #603
Conversation
Signed-off-by: Ce Gao <[email protected]>
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Ce Gao <[email protected]>
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Ce Gao <[email protected]>
|
/assign @ScorpioCPH |
|
/retest |
1 similar comment
|
/retest |
|
/assign @jlewi It seems that @ScorpioCPH is busy these days. We need more contributors 😄 |
|
Set it to p1 since it blocks #532 |
|
Can you update the PR description to please describe what this change is doing and why you are doing ? This will make it much easier to review the PR. |
|
Sorry, my mistake. Updated 😄 |
| return err | ||
| } | ||
| replicas := int(*spec.Replicas) | ||
| // Get all pods for the type rt. |
There was a problem hiding this comment.
Comment looks incorrect; I assume you mean service.
There was a problem hiding this comment.
Thanks. Sorry to harp on the PR description but a good PR description makes the PR review much easier my making it apparent what things to focus on.
So in this case the key things I think are worth mentioning in the PR description are
-
To make the controller more robust we don't want to assume that if there are N services and N replicas that everything is ok.
-
Instead what this PR is doing is for each replica index, checking all services corresponding to that replica (based on labels) and deciding whether services need to be created, deleted, or no action is needed.
| } | ||
|
|
||
| for _, service := range activeServices { | ||
| // getServiceSlices returns a slice, which element is the slice of service. |
There was a problem hiding this comment.
Comment isn't all that clear. It looks like the return value is a list of list so it would be good to explain what each dimension corresponds to e.g.
result[i] - Is an array of pointers to services corresponding to Services for replica i
|
OK, I will take more care of the description. |
Signed-off-by: Ce Gao <[email protected]>
|
I fixed the errors in the comments, PTAL |
|
/retest |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jlewi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* service: Refactor to the slice structure Signed-off-by: Ce Gao <[email protected]> * controller.go: Remove useless code Signed-off-by: Ce Gao <[email protected]> * controller: Fix test Signed-off-by: Ce Gao <[email protected]> * service: Fix comments Signed-off-by: Ce Gao <[email protected]>
* service: Refactor to the slice structure Signed-off-by: Ce Gao <[email protected]> * controller.go: Remove useless code Signed-off-by: Ce Gao <[email protected]> * controller: Fix test Signed-off-by: Ce Gao <[email protected]> * service: Fix comments Signed-off-by: Ce Gao <[email protected]>
\assign @ScorpioCPH
This PR is to refactor the code related to service as #548 does.
Signed-off-by: Ce Gao [email protected]
This change is