-
Notifications
You must be signed in to change notification settings - Fork 231
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: Migrate compute target TCP proxy(global) to direct controller #3047
feat: Migrate compute target TCP proxy(global) to direct controller #3047
Conversation
83e27b3
to
6afd43c
Compare
ffa6f3a
to
19487b8
Compare
b29cc10
to
3acffb1
Compare
3acffb1
to
20a4c85
Compare
20a4c85
to
15f87db
Compare
} | ||
|
||
--- | ||
|
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.
Looks like the real GCP using the TF-based controller. Could you also upload the one using direct controller? (please do that in a new git-commit)
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.
This log is from ComputeHealthCheck so it's still using TF controller.
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.
suggest to start developing a shared functionality for compute resource. It looks many compute resource would need this global/regional handling.
"sigs.k8s.io/controller-runtime/pkg/client" | ||
) | ||
|
||
func ResolveComputeBackendService(ctx context.Context, reader client.Reader, src client.Object, ref *refs.ComputeBackendServiceRef) (*refs.ComputeBackendServiceRef, error) { |
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.
suggest moving this to API so it can be shared by other services.
/lgtm One comment about code refactoring. Otherwise looks good |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yuwenma 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 |
3954a2d
into
GoogleCloudPlatform:master
Change description
Prerequisite for b/246759060
Migrate global compute target TCP proxy to direct controller. Will support regional in a followup PR.
Tests you have done
make ready-pr
to ensure this PR is ready for review.