Skip to content
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

[NET-2880] Add PrioritizeByLocality to ProxyDefaults CRD #2784

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
zalimeni marked this conversation as resolved.
Show resolved Hide resolved
// 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