Skip to content

Commit

Permalink
[NET-2880] Add PrioritizeByLocality to ProxyDefaults CRD (#2784)
Browse files Browse the repository at this point in the history
Add `PrioritizeByLocality` to `ProxyDefaults` CRD

In addition to service resolver, add this field to the CRD for proxy
defaults for parity with Consul config options.
  • Loading branch information
zalimeni authored Aug 17, 2023
1 parent 8a5eff0 commit 48184c6
Show file tree
Hide file tree
Showing 10 changed files with 120 additions and 72 deletions.
3 changes: 0 additions & 3 deletions .changelog/2357.txt

This file was deleted.

3 changes: 3 additions & 0 deletions .changelog/2784.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:feature
Add the `PrioritizeByLocality` field to the `ServiceResolver` and `ProxyDefaults` CRDs.
```
9 changes: 9 additions & 0 deletions charts/consul/templates/crd-proxydefaults.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,15 @@ spec:
type: string
type: array
type: object
prioritizeByLocality:
description: PrioritizeByLocality contains the configuration for
locality aware routing.
properties:
mode:
description: Mode specifies the behavior of PrioritizeByLocality
routing. Valid values are "", "none", and "failover".
type: string
type: object
meshGateway:
description: MeshGateway controls the default mesh gateway configuration
for this service.
Expand Down
27 changes: 16 additions & 11 deletions control-plane/api/v1alpha1/proxydefaults_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ type ProxyDefaultsSpec struct {
EnvoyExtensions EnvoyExtensions `json:"envoyExtensions,omitempty"`
// FailoverPolicy specifies the exact mechanism used for failover.
FailoverPolicy *FailoverPolicy `json:"failoverPolicy,omitempty"`
// PrioritizeByLocality controls whether the locality of services within the
// local partition will be used to prioritize connectivity.
PrioritizeByLocality *PrioritizeByLocality `json:"prioritizeByLocality,omitempty"`
}

func (in *ProxyDefaults) GetObjectMeta() metav1.ObjectMeta {
Expand Down Expand Up @@ -179,17 +182,18 @@ func (in *ProxyDefaults) SetLastSyncedTime(time *metav1.Time) {
func (in *ProxyDefaults) ToConsul(datacenter string) capi.ConfigEntry {
consulConfig := in.convertConfig()
return &capi.ProxyConfigEntry{
Kind: in.ConsulKind(),
Name: in.ConsulName(),
MeshGateway: in.Spec.MeshGateway.toConsul(),
Expose: in.Spec.Expose.toConsul(),
Config: consulConfig,
TransparentProxy: in.Spec.TransparentProxy.toConsul(),
MutualTLSMode: in.Spec.MutualTLSMode.toConsul(),
AccessLogs: in.Spec.AccessLogs.toConsul(),
EnvoyExtensions: in.Spec.EnvoyExtensions.toConsul(),
FailoverPolicy: in.Spec.FailoverPolicy.toConsul(),
Meta: meta(datacenter),
Kind: in.ConsulKind(),
Name: in.ConsulName(),
MeshGateway: in.Spec.MeshGateway.toConsul(),
Expose: in.Spec.Expose.toConsul(),
Config: consulConfig,
TransparentProxy: in.Spec.TransparentProxy.toConsul(),
MutualTLSMode: in.Spec.MutualTLSMode.toConsul(),
AccessLogs: in.Spec.AccessLogs.toConsul(),
EnvoyExtensions: in.Spec.EnvoyExtensions.toConsul(),
FailoverPolicy: in.Spec.FailoverPolicy.toConsul(),
PrioritizeByLocality: in.Spec.PrioritizeByLocality.toConsul(),
Meta: meta(datacenter),
}
}

Expand Down Expand Up @@ -228,6 +232,7 @@ func (in *ProxyDefaults) Validate(_ common.ConsulMeta) error {
allErrs = append(allErrs, in.Spec.Expose.validate(path.Child("expose"))...)
allErrs = append(allErrs, in.Spec.EnvoyExtensions.validate(path.Child("envoyExtensions"))...)
allErrs = append(allErrs, in.Spec.FailoverPolicy.validate(path.Child("failoverPolicy"))...)
allErrs = append(allErrs, in.Spec.PrioritizeByLocality.validate(path.Child("prioritizeByLocality"))...)

if len(allErrs) > 0 {
return apierrors.NewInvalid(
Expand Down
25 changes: 25 additions & 0 deletions control-plane/api/v1alpha1/proxydefaults_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ func TestProxyDefaults_MatchesConsul(t *testing.T) {
Mode: "sequential",
Regions: []string{"us-west-1"},
},
PrioritizeByLocality: &PrioritizeByLocality{
Mode: "failover",
},
},
},
Theirs: &capi.ProxyConfigEntry{
Expand Down Expand Up @@ -161,6 +164,9 @@ func TestProxyDefaults_MatchesConsul(t *testing.T) {
Mode: "sequential",
Regions: []string{"us-west-1"},
},
PrioritizeByLocality: &capi.ServiceResolverPrioritizeByLocality{
Mode: "failover",
},
},
Matches: true,
},
Expand Down Expand Up @@ -318,6 +324,9 @@ func TestProxyDefaults_ToConsul(t *testing.T) {
Mode: "sequential",
Regions: []string{"us-west-1"},
},
PrioritizeByLocality: &PrioritizeByLocality{
Mode: "none",
},
},
},
Exp: &capi.ProxyConfigEntry{
Expand Down Expand Up @@ -382,6 +391,9 @@ func TestProxyDefaults_ToConsul(t *testing.T) {
Mode: "sequential",
Regions: []string{"us-west-1"},
},
PrioritizeByLocality: &capi.ServiceResolverPrioritizeByLocality{
Mode: "none",
},
Meta: map[string]string{
common.SourceKey: common.SourceValue,
common.DatacenterKey: "datacenter",
Expand Down Expand Up @@ -652,6 +664,19 @@ func TestProxyDefaults_Validate(t *testing.T) {
},
expectedErrMsg: `proxydefaults.consul.hashicorp.com "global" is invalid: spec.failoverPolicy.mode: Invalid value: "wrong-mode": must be one of "", "sequential", "order-by-locality"`,
},
"prioritize by locality invalid": {
input: &ProxyDefaults{
ObjectMeta: metav1.ObjectMeta{
Name: "global",
},
Spec: ProxyDefaultsSpec{
PrioritizeByLocality: &PrioritizeByLocality{
Mode: "wrong-mode",
},
},
},
expectedErrMsg: `proxydefaults.consul.hashicorp.com "global" is invalid: spec.prioritizeByLocality.mode: Invalid value: "wrong-mode": must be one of "", "none", "failover"`,
},
"multi-error": {
input: &ProxyDefaults{
ObjectMeta: metav1.ObjectMeta{
Expand Down
38 changes: 1 addition & 37 deletions control-plane/api/v1alpha1/serviceresolver_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,7 @@ type ServiceResolverSpec struct {
LoadBalancer *LoadBalancer `json:"loadBalancer,omitempty"`
// PrioritizeByLocality controls whether the locality of services within the
// local partition will be used to prioritize connectivity.
PrioritizeByLocality *ServiceResolverPrioritizeByLocality `json:"prioritizeByLocality,omitempty"`
}

type ServiceResolverPrioritizeByLocality struct {
// Mode specifies the type of prioritization that will be performed
// when selecting nodes in the local partition.
// Valid values are: "" (default "none"), "none", and "failover".
Mode string `json:"mode,omitempty"`
PrioritizeByLocality *PrioritizeByLocality `json:"prioritizeByLocality,omitempty"`
}

type ServiceResolverRedirect struct {
Expand Down Expand Up @@ -536,16 +529,6 @@ func (in *ServiceResolverFailover) toConsul() *capi.ServiceResolverFailover {
}
}

func (in *ServiceResolverPrioritizeByLocality) toConsul() *capi.ServiceResolverPrioritizeByLocality {
if in == nil {
return nil
}

return &capi.ServiceResolverPrioritizeByLocality{
Mode: in.Mode,
}
}

func (in ServiceResolverFailoverTarget) toConsul() capi.ServiceResolverFailoverTarget {
return capi.ServiceResolverFailoverTarget{
Service: in.Service,
Expand Down Expand Up @@ -655,25 +638,6 @@ func (in *ServiceResolverFailover) isEmpty() bool {
return in.Service == "" && in.ServiceSubset == "" && in.Namespace == "" && len(in.Datacenters) == 0 && len(in.Targets) == 0 && in.Policy == nil && in.SamenessGroup == ""
}

func (in *ServiceResolverPrioritizeByLocality) validate(path *field.Path) field.ErrorList {
var errs field.ErrorList

if in == nil {
return nil
}

switch in.Mode {
case "":
case "none":
case "failover":
default:
asJSON, _ := json.Marshal(in)
errs = append(errs, field.Invalid(path, string(asJSON),
"mode must be one of '', 'none', or 'failover'"))
}
return errs
}

func (in *ServiceResolverFailover) validate(path *field.Path, consulMeta common.ConsulMeta) field.ErrorList {
var errs field.ErrorList
if in.isEmpty() {
Expand Down
10 changes: 5 additions & 5 deletions control-plane/api/v1alpha1/serviceresolver_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func TestServiceResolver_MatchesConsul(t *testing.T) {
Datacenter: "redirect_datacenter",
Peer: "redirect_peer",
},
PrioritizeByLocality: &ServiceResolverPrioritizeByLocality{
PrioritizeByLocality: &PrioritizeByLocality{
Mode: "failover",
},
Failover: map[string]ServiceResolverFailover{
Expand Down Expand Up @@ -285,7 +285,7 @@ func TestServiceResolver_ToConsul(t *testing.T) {
Datacenter: "redirect_datacenter",
Partition: "redirect_partition",
},
PrioritizeByLocality: &ServiceResolverPrioritizeByLocality{
PrioritizeByLocality: &PrioritizeByLocality{
Mode: "none",
},
Failover: map[string]ServiceResolverFailover{
Expand Down Expand Up @@ -898,20 +898,20 @@ func TestServiceResolver_Validate(t *testing.T) {
"spec.failover[failB].namespace: Invalid value: \"namespace-b\": Consul Enterprise namespaces must be enabled to set failover.namespace",
},
},
"prioritize by locality none": {
"prioritize by locality invalid": {
input: &ServiceResolver{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
Spec: ServiceResolverSpec{
PrioritizeByLocality: &ServiceResolverPrioritizeByLocality{
PrioritizeByLocality: &PrioritizeByLocality{
Mode: "bad",
},
},
},
namespacesEnabled: false,
expectedErrMsgs: []string{
"mode must be one of '', 'none', or 'failover'",
"serviceresolver.consul.hashicorp.com \"foo\" is invalid: spec.prioritizeByLocality.mode: Invalid value: \"bad\": must be one of \"\", \"none\", \"failover\"",
},
},
}
Expand Down
31 changes: 31 additions & 0 deletions control-plane/api/v1alpha1/shared_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,37 @@ func (in *FailoverPolicy) validate(path *field.Path) field.ErrorList {
return errs
}

// PrioritizeByLocality controls whether the locality of services within the
// local partition will be used to prioritize connectivity.
type PrioritizeByLocality struct {
// Mode specifies the type of prioritization that will be performed
// when selecting nodes in the local partition.
// Valid values are: "" (default "none"), "none", and "failover".
Mode string `json:"mode,omitempty"`
}

func (in *PrioritizeByLocality) toConsul() *capi.ServiceResolverPrioritizeByLocality {
if in == nil {
return nil
}

return &capi.ServiceResolverPrioritizeByLocality{
Mode: in.Mode,
}
}

func (in *PrioritizeByLocality) validate(path *field.Path) field.ErrorList {
var errs field.ErrorList
if in == nil {
return nil
}
modes := []string{"", "none", "failover"}
if !sliceContains(modes, in.Mode) {
errs = append(errs, field.Invalid(path.Child("mode"), in.Mode, notInSliceMessage(modes)))
}
return errs
}

func notInSliceMessage(slice []string) string {
return fmt.Sprintf(`must be one of "%s"`, strings.Join(slice, `", "`))
}
Expand Down
37 changes: 21 additions & 16 deletions control-plane/api/v1alpha1/zz_generated.deepcopy.go

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
Expand Up @@ -156,6 +156,15 @@ spec:
type: string
type: array
type: object
prioritizeByLocality:
description: PrioritizeByLocality contains the configuration for
locality aware routing.
properties:
mode:
description: Mode specifies the behavior of PrioritizeByLocality
routing. Valid values are "", "none", and "failover".
type: string
type: object
meshGateway:
description: MeshGateway controls the default mesh gateway configuration
for this service.
Expand Down

0 comments on commit 48184c6

Please sign in to comment.