-
Notifications
You must be signed in to change notification settings - Fork 727
feat(provisioner): ContourDeployment loadBalancerSourceRanges and loadBalancerClass #7665
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
base: main
Are you sure you want to change the base?
Changes from all commits
fbfc84f
df76deb
74297e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -332,6 +332,21 @@ type NetworkPublishing struct { | |||||||||||||||||
| // | ||||||||||||||||||
| // +optional | ||||||||||||||||||
| ServiceAnnotations map[string]string `json:"serviceAnnotations,omitempty"` | ||||||||||||||||||
|
|
||||||||||||||||||
| // LoadBalancerSourceRanges is an optional list of IP ranges in CIDR form | ||||||||||||||||||
| // which are allowed to access the LoadBalancer type Envoy Service. | ||||||||||||||||||
| // This maps to Service.spec.loadBalancerSourceRanges. | ||||||||||||||||||
| // Only applies when Type is LoadBalancerService. | ||||||||||||||||||
| // | ||||||||||||||||||
| // +optional | ||||||||||||||||||
| LoadBalancerSourceRanges []string `json:"loadBalancerSourceRanges,omitempty"` | ||||||||||||||||||
|
|
||||||||||||||||||
| // LoadBalancerClass is the class of the load balancer implementation | ||||||||||||||||||
| // to use for the Envoy Service when Type is LoadBalancerService. | ||||||||||||||||||
| // This maps to Service.spec.loadBalancerClass. | ||||||||||||||||||
|
Comment on lines
+344
to
+346
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||
| // | ||||||||||||||||||
| // +optional | ||||||||||||||||||
| LoadBalancerClass *string `json:"loadBalancerClass,omitempty"` | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // NetworkPublishingType is a way to publish network endpoints. | ||||||||||||||||||
|
|
||||||||||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Adds ContourDeployment.spec.envoy.networkPublishing.loadBalancerSourceRanges and loadBalancerClass so the Gateway provisioner can configure Envoy LoadBalancer Service source ranges and class. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -308,6 +308,13 @@ func (r *gatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct | |||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| contourModel.Spec.NetworkPublishing.Envoy.ServiceAnnotations = networkPublishing.ServiceAnnotations | ||||||||||||||||||
|
|
||||||||||||||||||
| if len(networkPublishing.LoadBalancerSourceRanges) > 0 { | ||||||||||||||||||
| contourModel.Spec.NetworkPublishing.Envoy.LoadBalancerSourceRanges = networkPublishing.LoadBalancerSourceRanges | ||||||||||||||||||
| } | ||||||||||||||||||
| if networkPublishing.LoadBalancerClass != nil { | ||||||||||||||||||
| contourModel.Spec.NetworkPublishing.Envoy.LoadBalancerClass = networkPublishing.LoadBalancerClass | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+312
to
+317
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can simplify
Suggested change
|
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Node placement | ||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -187,6 +187,16 @@ func LoadBalancerServiceChanged(current, expected *core_v1.Service) (*core_v1.Se | |
| changed = true | ||
| } | ||
|
|
||
| if !apiequality.Semantic.DeepEqual(current.Spec.LoadBalancerSourceRanges, expected.Spec.LoadBalancerSourceRanges) { | ||
| updated.Spec.LoadBalancerSourceRanges = expected.Spec.LoadBalancerSourceRanges | ||
| changed = true | ||
| } | ||
|
|
||
| if !apiequality.Semantic.DeepEqual(current.Spec.LoadBalancerClass, expected.Spec.LoadBalancerClass) { | ||
| updated.Spec.LoadBalancerClass = expected.Spec.LoadBalancerClass | ||
| changed = true | ||
| } | ||
|
|
||
|
Comment on lines
+195
to
+199
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment on
So since we cannot update this, the inequality should be ignored, or what do you think? |
||
| if !changed { | ||
| return nil, false | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -297,6 +297,16 @@ func DesiredEnvoyService(contour *model.Contour) *core_v1.Service { | |
| maps.Copy(svc.Annotations, contour.Spec.NetworkPublishing.Envoy.ServiceAnnotations) | ||
| } | ||
|
|
||
| // Optional LoadBalancer Service fields from ContourDeployment networkPublishing. | ||
| if epType == model.LoadBalancerServicePublishingType { | ||
| if len(contour.Spec.NetworkPublishing.Envoy.LoadBalancerSourceRanges) > 0 { | ||
| svc.Spec.LoadBalancerSourceRanges = contour.Spec.NetworkPublishing.Envoy.LoadBalancerSourceRanges | ||
| } | ||
| if contour.Spec.NetworkPublishing.Envoy.LoadBalancerClass != nil { | ||
| svc.Spec.LoadBalancerClass = contour.Spec.NetworkPublishing.Envoy.LoadBalancerClass | ||
| } | ||
| } | ||
|
Comment on lines
+300
to
+308
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets move these under load balancer type switch epType {
case model.LoadBalancerServicePublishingType:
// all loadbalancer type related fields set here |
||
|
|
||
| return svc | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
We could add This field may be ignored if the cloud-provider does not support it