Skip to content

Commit

Permalink
Add PrioritizeByLocality to ProxyDefaults CRD
Browse files Browse the repository at this point in the history
In addition to service resolver, add this field to the CRD for proxy
defaults for parity with Consul config options.
  • Loading branch information
zalimeni committed Aug 16, 2023
1 parent ab00c03 commit 8320145
Show file tree
Hide file tree
Showing 9 changed files with 116 additions and 70 deletions.
2 changes: 1 addition & 1 deletion .changelog/2357.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
```release-note:feature
Add the `PrioritizeByLocality` field to the `ServiceResolver` CRD.
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
29 changes: 29 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,35 @@ func (in *FailoverPolicy) validate(path *field.Path) field.ErrorList {
return errs
}

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 8320145

Please sign in to comment.