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
add konnectivity proxy sidecar to ingress-operator to ensure it can properly perform in cluster canary healthchecks #1131
Conversation
f0c50c8
to
462b110
Compare
/hold |
90951bb
to
63b6e4e
Compare
After change:
|
/unhold |
/lgtm |
@@ -118,6 +121,18 @@ func ReconcileDeployment(dep *appsv1.Deployment, params Params, apiPort *int32) | |||
{Name: "IMAGE", Value: params.HAProxyRouterImage}, | |||
{Name: "CANARY_IMAGE", Value: params.IngressOperatorImage}, | |||
{Name: "KUBECONFIG", Value: "/etc/kubernetes/kubeconfig"}, | |||
{ | |||
Name: "HTTP_PROXY", |
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.
/hold
This breaks outbound connectivity to cloud provider apis which we need for public clouds.
I don't have a solution for this off-hand, but it is a problem we will have to solve for other components in the context of management cluster with proxy support as well.
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.
Actually it works eventually, probably after the geust cluster has nodes? But this completely negates the advantage of running it in the mgtm cluster
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.
Not sure how it negates it?
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.
You still get the benefit of it rolling out the resources ahead of time since it can talk to the kube-apis? Which is where I believe the real time benefit is: this just solves a fundamental gap that needs to be solved for health checking on the domain route.
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.
@alvaroaleman brought up a great point that this does call cloud provider APIs: therefore those routes should not be proxied this is added on the latest version.
63b6e4e
to
950d5b5
Compare
950d5b5
to
5406963
Compare
/hold cancel |
/hold |
…roperly perform in cluster canary healthchecks Currently the ingress operator fails to properly perform canary health checks in the guest cluster if it does not have direct network access to the ingress subdomain in the guest cluster. This is not a guarentee to have since the management cluster and guest cluster can run in a split network environment. This pr introduces the socks proxy which will allow the ingress operator to proxy these canary healthcheck https requests through konnectivity and ultimately into the guest cluster. This will allow the healthchecks to properly be executed in all environments and prevent Degragaded status reports on the ingress resource which can lead to customer concerns/tickets. Fixes: openshift#1130
5406963
to
28546dc
Compare
Adjusted to .format which is compatible with both curl and go
Can tell it's going direct since it connects direct to the backend IP
|
Whereas on one that is proxied
|
/unhold |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, relyt0925 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 |
@relyt0925: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Currently the ingress operator fails to properly perform canary health checks in the guest cluster if it does not have direct network access to the ingress subdomain in the guest cluster. This is not a guarentee to have since the management
cluster and guest cluster can run in a split network environment. This pr introduces the socks proxy which will allow the ingress operator to proxy these canary healthcheck https requests through konnectivity and ultimately into the guest cluster. This will allow the healthchecks to properly be executed in all environments and prevent Degragaded status reports on the ingress resource which can lead to customer concerns/tickets. Fixes: #1130
Relevant logs:
What this PR does / why we need it:
#1130
Which issue(s) this PR fixes (optional, use
fixes #<issue_number>(, fixes #<issue_number>, ...)
format, where issue_number might be a GitHub issue, or a Jira story:Fixes #
#1130
Checklist