Remove pkg/context dependency from NEG controller #1331
Conversation
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
the
approved
pkg/neg/controller_test.go
Outdated
| dynamicClient := dynamicfake.NewSimpleDynamicClient(dynamicSchema) | ||
|
|
||
| destrinationGVR := schema.GroupVersionResource{Group: "networking.istio.io", Version: "v1alpha3", Resource: "destinationrules"} |
pkg/neg/manager_test.go
Outdated
| negtypes.MockNetworkEndpointAPIs(fakeGCE) | ||
| fakeCloud := negtypes.NewAdapter(fakeGCE) | ||
|
|
||
| //backendConfigClient := backendconfigclient.NewSimpleClientset() |
There was a problem hiding this comment.
commented lines can be deleted.
pkg/neg/manager_test.go
Outdated
| @@ -723,9 +745,10 @@ func TestFilterCommonPorts(t *testing.T) { | |||
| func TestNegCRCreations(t *testing.T) { | |||
| t.Parallel() | |||
|
|
|||
| svcNegClient := negfake.NewSimpleClientset() | |||
| //svcNegClient := negfake.NewSimpleClientset() | |||
pkg/neg/manager_test.go
Outdated
| @@ -919,9 +942,12 @@ func TestNegCRDuplicateCreations(t *testing.T) { | |||
| for _, tc := range testCases { | |||
| t.Run(tc.desc, func(t *testing.T) { | |||
|
|
|||
| svcNegClient := negfake.NewSimpleClientset() | |||
| //svcNegClient := negfake.NewSimpleClientset() | |||
pkg/neg/manager_test.go
Outdated
| @@ -83,39 +76,56 @@ func NewTestSyncerManager(kubeClient kubernetes.Interface) *syncerManager { | |||
| } | |||
|
|
|||
| func NewTestSyncerManagerWithNegClient(kubeClient kubernetes.Interface, svcNegClient svcnegclient.Interface) *syncerManager { | |||
There was a problem hiding this comment.
this function can be merged with the above since we always create the svcNegClient.
pkg/neg/controller_test.go
Outdated
| namer := namer_util.NewNamer(ClusterID, "") | ||
| dynamicSchema := runtime.NewScheme() | ||
| //dynamicSchema.AddKnownTypeWithName(schema.GroupVersionKind{Group: "networking.istio.io", Version: "v1alpha3", Kind: "List"}, &unstructured.UnstructuredList{}) | ||
| //backendConfigClient := backendconfigclient.NewSimpleClientset() |
There was a problem hiding this comment.
commented code can be deleted.
pkg/neg/controller.go
Outdated
| @@ -19,6 +19,7 @@ package neg | |||
| import ( | |||
| context2 "context" | |||
There was a problem hiding this comment.
This no longer needs to be imported as context2
pkg/neg/controller_test.go
Outdated
| @@ -20,6 +20,7 @@ import ( | |||
| context2 "context" | |||
There was a problem hiding this comment.
this no longer needs to be imported as context2
pkg/neg/controller.go
Outdated
| @@ -19,6 +19,7 @@ package neg | |||
| import ( | |||
| context2 "context" | |||
| "fmt" | |||
| svcnegclient "k8s.io/ingress-gce/pkg/svcneg/client/clientset/versioned" | |||
There was a problem hiding this comment.
move this to the second import block
| svcNegClient svcnegclient.Interface, | ||
| destinationRuleClient dynamic.NamespaceableResourceInterface, | ||
| kubeSystemUID types.UID, | ||
| ingressInformer cache.SharedIndexInformer, |
There was a problem hiding this comment.
why not encapsulate informers and clients into separate structs?
There was a problem hiding this comment.
I can follow up with a PR to refactor this into a separate NegControllerContext.
|
Done. Ready for another round. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: freehan, swetharepakula 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 |
cc @swetharepakula