-
Notifications
You must be signed in to change notification settings - Fork 155
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
Deploy envoy-agent for tunneling expose strategy #6445
Deploy envoy-agent for tunneling expose strategy #6445
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: irozzo-1A 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 |
@@ -151,6 +156,10 @@ func DeploymentCreator(data userclusterControllerData, openshift bool) reconcili | |||
args = append(args, "-node-labels", labelArgsValue) | |||
} | |||
|
|||
//if data.Cluster().Spec.ExposeStrategy == kubermaticv1.ExposeStrategyTunneling { |
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.
clean-up this
@@ -107,6 +110,8 @@ func main() { | |||
flag.StringVar(&runOp.clusterURL, "cluster-url", "", "Cluster URL") | |||
flag.StringVar(&runOp.dnsClusterIP, "dns-cluster-ip", "", "KubeDNS service IP for the cluster") | |||
flag.IntVar(&runOp.openvpnServerPort, "openvpn-server-port", 0, "OpenVPN server port") | |||
flag.IntVar(&runOp.kasSecurePort, "kas-secure-port", 6443, "Secure KAS port") | |||
flag.Var(&runOp.tunnelingAgentIP, "tunneling-agent-ip", "If specified the tunneling agent will bind to this IP address, otherwise it will not be deployed.") |
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.
Shouldn't we put a default value (192.168.30.10, for the tech preview)? We still need a feature-gate flag to enable the new expose strategy right?
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.
The value is provided by seed-controller-manager
when the tunneling strategy is enabled, 192.168.30.10
is the default indeed. We have no feature gate on the user-controller-manager
at the moment, we are using the tunnelingAgentIP
presence/absence to decide whether we should deploy the agents or not.
@@ -212,7 +214,7 @@ func (r *Reconciler) ensureNamespaceExists(ctx context.Context, cluster *kuberma | |||
// GetServiceCreators returns all service creators that are currently in use | |||
func GetServiceCreators(data *resources.TemplateData) []reconciling.NamedServiceCreatorGetter { | |||
creators := []reconciling.NamedServiceCreatorGetter{ | |||
apiserver.ServiceCreator(data.Cluster().Spec.ExposeStrategy, data.Cluster().Address.InternalName), | |||
apiserver.ServiceCreator(data.Cluster().Spec.ExposeStrategy, data.Cluster().Address.ExternalName), |
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 to understand why this has been set to the ExternalName
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 is the hostname we are using for SNI
pkg/controller/user-cluster-controller-manager/resources/resources/envoy-agent/daemonset.go
Outdated
Show resolved
Hide resolved
lgtm minus the failing tests and SA comment. |
fa31f19
to
37eeb55
Compare
0ef6414
to
99b5da2
Compare
Co-authored-by: irozzo-1A <iacopo@kubermatic.com> Co-authored-by: youssefazrak <yazrak.tech@gmail.com>
99b5da2
to
484043c
Compare
/lgtm |
LGTM label has been added. Git tree hash: a896352df55c5a126b5f62fb8c41e20d6b91d1e9
|
This PR introduces the deployment of the tunneling agent when tunneling expose strategy is used.
host
scope through init containers.What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #6392, #6445
Special notes for your reviewer:
This is not intended to be prod ready.
Documentation:
Does this PR introduce a user-facing change?: