feat(provisioner): ContourDeployment loadBalancerSourceRanges and loadBalancerClass - #7665
Conversation
…lass on ContourDeployment Allow ContourDeployment.spec.envoy.networkPublishing to configure the Envoy Service LoadBalancer fields Service.spec.loadBalancerSourceRanges and Service.spec.loadBalancerClass so Gateway provisioner users can restrict source IPs and select a load balancer implementation. Fixes projectcontour#7334 Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com>
Add unreleased changelog for projectcontour#7665 and regenerate CRDs, example manifests, and API reference docs so check-generate passes. Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com>
1b726e3 to
96edf5e
Compare
Add unreleased changelog for projectcontour#7665 and regenerate CRDs, example manifests, and API reference docs so check-generate passes. Signed-off-by: Vedant Madane <vedantnm@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7665 +/- ##
==========================================
+ Coverage 82.35% 82.37% +0.01%
==========================================
Files 131 131
Lines 15915 15930 +15
==========================================
+ Hits 13107 13122 +15
Misses 2520 2520
Partials 288 288
🚀 New features to boost your workflow:
|
Add unit coverage for ContourDeployment networkPublishing LB fields through equality change detection, DesiredEnvoyService (LB and ClusterIP ignore paths), and gateway controller copy into the Envoy Service. Improves codecov/patch on the ContourDeployment LB feature. Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com>
42fb60a to
74297e0
Compare
|
When will this go live? I’m not sure why the LoadBalancer class is restricting us. It’s forcing us to move away from Contour and use Envoy Gateway instead, in the VMware ecosystem :( |
| if !apiequality.Semantic.DeepEqual(current.Spec.LoadBalancerClass, expected.Spec.LoadBalancerClass) { | ||
| updated.Spec.LoadBalancerClass = expected.Spec.LoadBalancerClass | ||
| changed = true | ||
| } | ||
|
|
There was a problem hiding this comment.
The comment on LoadBalancerClass says (link)
This field can only be set when creating or updating a Service to type 'LoadBalancer'. Once set, it can not be changed. This field will be wiped when a service is updated to a non 'LoadBalancer' type.
So since we cannot update this, the inequality should be ignored, or what do you think?
| if len(networkPublishing.LoadBalancerSourceRanges) > 0 { | ||
| contourModel.Spec.NetworkPublishing.Envoy.LoadBalancerSourceRanges = networkPublishing.LoadBalancerSourceRanges | ||
| } | ||
| if networkPublishing.LoadBalancerClass != nil { | ||
| contourModel.Spec.NetworkPublishing.Envoy.LoadBalancerClass = networkPublishing.LoadBalancerClass | ||
| } |
There was a problem hiding this comment.
I think we can simplify
| if len(networkPublishing.LoadBalancerSourceRanges) > 0 { | |
| contourModel.Spec.NetworkPublishing.Envoy.LoadBalancerSourceRanges = networkPublishing.LoadBalancerSourceRanges | |
| } | |
| if networkPublishing.LoadBalancerClass != nil { | |
| contourModel.Spec.NetworkPublishing.Envoy.LoadBalancerClass = networkPublishing.LoadBalancerClass | |
| } | |
| contourModel.Spec.NetworkPublishing.Envoy.LoadBalancerSourceRanges = networkPublishing.LoadBalancerSourceRanges | |
| contourModel.Spec.NetworkPublishing.Envoy.LoadBalancerClass = networkPublishing.LoadBalancerClass |
| // 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 | ||
| } | ||
| } |
There was a problem hiding this comment.
Lets move these under load balancer type case
switch epType {
case model.LoadBalancerServicePublishingType:
// all loadbalancer type related fields set here| // 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. |
There was a problem hiding this comment.
We could add This field may be ignored if the cloud-provider does not support it
| // 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. | |
| // 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. | |
| // This field may be ignored if the cloud-provider does not support it. | |
| // Only applies when Type is LoadBalancerService. |
| // 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. |
There was a problem hiding this comment.
| // 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. | |
| // 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. | |
| // Once set, it cannot be changed without recreating the Service. | |
| // Only applies when Type is LoadBalancerService. |
tsaarni
left a comment
There was a problem hiding this comment.
@VedantMadane Thanks for contributing! I left some inline comments to check.
Summary
Adds
loadBalancerSourceRangesandloadBalancerClasstoContourDeployment.spec.envoy.networkPublishingso the Gateway provisioner can set the corresponding Envoy Service fields when publishing as a LoadBalancer.Fixes #7334
Motivation
When using Contour Gateway Provisioner with a custom
ContourDeployment, there was no way to configure:Service.spec.loadBalancerSourceRanges(restrict source IPs)Service.spec.loadBalancerClass(select LB implementation)Changes
ContourDeployment/NetworkPublishing): new optional fieldsLoadBalancerServiceTestDesiredEnvoyServiceLoadBalancerFieldsExample