-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add webook and external eps support dualstack #774
base: main
Are you sure you want to change the base?
Conversation
8f5a5ae
to
8c10923
Compare
Unit test code should be submitted together with the logic code. |
@@ -202,6 +209,7 @@ func startEndPointsControllers(mgr manager.Manager) error { | |||
APIServerExternalSyncController := endpointscontroller.APIServerExternalSyncController{ | |||
Client: mgr.GetClient(), | |||
EventRecorder: mgr.GetEventRecorderFor(constants.GlobalNodeControllerName), | |||
KubeClient: kubeClient, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a conflict between Client and KubeClient, and can they be reused?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Client is a client used for execution in the VC, while the KubeClient is a client used for execution in the host cluster. These two are used in different scenarios and do not conflict with each other
Namespace string | ||
Name string | ||
Vc *v1alpha1.VirtualCluster | ||
RootClientSet kubernetes.Interface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no special necessity for the definition of this Resource struct, and does the name of the struct not reflect its purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since information from the Resource struct is needed, a struct is defined here and placed in the common package for use by multiple Go files. However, the struct name can be changed to better reflect its purpose.
pkg/kubenest/controller/endpoints.sync.controller/apiserver_external_sync_controller.go
Outdated
Show resolved
Hide resolved
pkg/kubenest/controller/endpoints.sync.controller/apiserver_external_sync_controller.go
Outdated
Show resolved
Hide resolved
pkg/kubenest/controller/endpoints.sync.controller/apiserver_external_sync_controller.go
Outdated
Show resolved
Hide resolved
pkg/kubenest/controller/endpoints.sync.controller/apiserver_external_sync_controller.go
Show resolved
Hide resolved
165dfc4
to
0fc2eb5
Compare
Signed-off-by: qiuwei <[email protected]>
0fc2eb5
to
071f49a
Compare
What type of PR is this?
/kind feature
What does this PR do?
add webook for virtualcluster cr and external eps support dualstack
Which issue(s) does this PR fix?
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?