Restructure NEG controller #489
Conversation
k8s-ci-robot
added
size/XL
freehan
force-pushed
the
unblocking-syncer
branch
2 times, most recently
from
f3381c0
to
bd8019c
Compare
pkg/neg/interfaces.go
Outdated
| @@ -17,36 +17,15 @@ limitations under the License. | |||
| package neg | |||
There was a problem hiding this comment.
Does there need to be an interfaces file at all? Could the below types just live in neg/types and neg/syncer?
There was a problem hiding this comment.
All the interfaces here are internal interfaces for controller and manager.
The rest are shared or imported in some other packages.
| type syncer struct { | ||
| servicePort | ||
| // batchSyncer handles synchorizing NEGs for one service port. It handles sync, resync and retry on error. | ||
| // It syncs NEG in batch and waits for all operation to complete before continue to the next batch. |
There was a problem hiding this comment.
This is called batch because we're batch-processing multiple NetworkEndpoints right? Can we make a note of that?
There was a problem hiding this comment.
I could not think of a better name. Calling it batch just because it process the NEG changes in batch and wait for all operation to finish before continue.
| @@ -46,25 +48,26 @@ const ( | |||
| maxRetryDelay = 600 * time.Second | |||
There was a problem hiding this comment.
this should probably be a flag
There was a problem hiding this comment.
Many controllers hard code parameters like this:
https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/service/service_controller.go#L59
pkg/neg/syncer/batch.go
Outdated
| @@ -25,14 +25,16 @@ import ( | |||
| "time" | |||
|
|
|||
| "github.com/golang/glog" | |||
| compute "google.golang.org/api/compute/v0.beta" | |||
There was a problem hiding this comment.
if we're removing the compute prefix here, we should do it for all files touched
There was a problem hiding this comment.
hmmm. Not sure why this is removed accidentally. Will add it back.
freehan
force-pushed
the
unblocking-syncer
branch
from
bd8019c
to
45c8074
Compare
freehan
force-pushed
the
unblocking-syncer
branch
from
45c8074
to
6255616
Compare
| @@ -438,3 +438,7 @@ func TraverseIngressBackends(ing *extensions.Ingress, process func(id ServicePor | |||
| } | |||
| return | |||
| } | |||
|
|
|||
| func ServiceKeyFunc(namespace, name string) string { | |||
|
/approve |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bowei, freehan 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 |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bowei, freehan 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 |
No functional changes. Only refactoring:
/metricsand/syncer/typessyncertobatchSyncercc: @agau4779