Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Clayton Gonsalves <clayton.gonsalves@reddit.com>
  • Loading branch information
clayton-gonsalves committed Oct 6, 2023
1 parent 5f09076 commit ee4aa23
Show file tree
Hide file tree
Showing 16 changed files with 188 additions and 147 deletions.
17 changes: 9 additions & 8 deletions apis/projectcontour/v1alpha1/contourconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,18 @@ type ContourConfigurationSpec struct {
// Tracing defines properties for exporting trace data to OpenTelemetry.
Tracing *TracingConfig `json:"tracing,omitempty"`

// FeatureFlags defines toggle for new contour features.
// FeatureFlags defines toggle to enable new contour features.
// available toggles are
// useEndpointSlices - configures contour to fetch endpoint data
// from k8s endpoint slices. defaults to false and reading endpoint
// data from the k8s endpoints.
FeatureFlags FeatureFlags `json:"featureFlags,omitempty"`
}

// FeatureFlags defines the set of feature flags
// to toggle new contour features.
type FeatureFlags []string

// XDSServerType is the type of xDS server implementation.
type XDSServerType string

Expand Down Expand Up @@ -862,10 +870,3 @@ type ContourConfigurationList struct {
metav1.ListMeta `json:"metadata,omitempty"`
Items []ContourConfiguration `json:"items"`
}

type FeatureFlags struct {
// useEndpointSlice configures contour to fetch endpoint data
// from k8s endpoint slices. defaults to false and reading endpoint
// data from the k8s endpoints.
UseEndpointSlice bool `json:"useEndpointSlice,omitempty"`
}
23 changes: 23 additions & 0 deletions apis/projectcontour/v1alpha1/contourconfig_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,18 @@ import (
"fmt"
"strconv"

"golang.org/x/exp/slices"
"k8s.io/apimachinery/pkg/util/sets"
)

const (
featureFlagUseEndpointSlices string = "useEndpointSlices"
)

var featureFlagsMap = map[string]bool{
featureFlagUseEndpointSlices: true,
}

// Validate configuration that is not already covered by CRD validation.
func (c *ContourConfigurationSpec) Validate() error {
// Validation of root configuration fields.
Expand Down Expand Up @@ -215,6 +224,20 @@ func (e *EnvoyTLS) SanitizedCipherSuites() []string {
return validatedCiphers
}

func (f FeatureFlags) Validate() error {
for _, featureFlag := range f {
if _, found := featureFlagsMap[featureFlag]; !found {
return fmt.Errorf("invalid contour configuration, unknown feature flag:%s", featureFlag)
}
}

return nil
}

func (f FeatureFlags) IsEndpointSliceEnabled() bool {
return slices.Contains(f, featureFlagUseEndpointSlices)
}

// Validate ensures that exactly one of ControllerName or GatewayRef are specified.
func (g *GatewayConfig) Validate() error {
if g == nil {
Expand Down
37 changes: 37 additions & 0 deletions apis/projectcontour/v1alpha1/contourconfig_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package v1alpha1_test

import (
"fmt"
"testing"

"github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
Expand Down Expand Up @@ -294,3 +295,39 @@ func TestAccessLogFormatExtensions(t *testing.T) {
}
assert.Empty(t, e3.AccessLogFormatterExtensions())
}

func TestFeatureFlagsValidate(t *testing.T) {
tests := []struct {
name string
flags v1alpha1.FeatureFlags
expected error
}{
{
name: "valid flag",
flags: v1alpha1.FeatureFlags{"useEndpointSlices"},
expected: nil,
},
{
name: "invalid flag",
flags: v1alpha1.FeatureFlags{"invalidFlag"},
expected: fmt.Errorf("invalid contour configuration, unknown feature flag:invalidFlag"),
},
{
name: "mix of valid and invalid flags",
flags: v1alpha1.FeatureFlags{"useEndpointSlices", "invalidFlag"},
expected: fmt.Errorf("invalid contour configuration, unknown feature flag:invalidFlag"),
},
{
name: "empty flags",
flags: v1alpha1.FeatureFlags{},
expected: nil,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := tt.flags.Validate()
assert.Equal(t, tt.expected, err)
})
}
}
18 changes: 13 additions & 5 deletions apis/projectcontour/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions changelogs/unreleased/5745-clayton-gonsalves-minor.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@

This change optionally enables Contour to consume the kubernetes endpointslice API to determine the endpoints to configure Envoy with.
Note: This change is off by default and is gated by the feature flag `useEndpointSlice`.

This feature will be enabled by default in a future version on Contour once it has had sufficient bake time in production environments.
6 changes: 3 additions & 3 deletions cmd/contour/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ func (s *Server) doServe() error {

// Endpoints updates are handled directly by the EndpointsTranslator/EndpointSliceTranslator due to the high update volume.
var endpointHandler EndpointsTranslator
if contourConfiguration.FeatureFlags.UseEndpointSlice {
if contourConfiguration.FeatureFlags.IsEndpointSliceEnabled() {
endpointHandler = xdscache_v3.NewEndpointSliceTranslator(s.log.WithField("context", "endpointstranslator"))
} else {
endpointHandler = xdscache_v3.NewEndpointsTranslator(s.log.WithField("context", "endpointstranslator"))
Expand Down Expand Up @@ -600,7 +600,7 @@ func (s *Server) doServe() error {
}

// Inform on endpoints/endpointSlices.
if contourConfiguration.FeatureFlags.UseEndpointSlice {
if contourConfiguration.FeatureFlags.IsEndpointSliceEnabled() {
if err := informOnResource(&discoveryv1.EndpointSlice{}, &contour.EventRecorder{
Next: endpointHandler,
Counter: contourMetrics.EventHandlerOperations,
Expand All @@ -612,7 +612,7 @@ func (s *Server) doServe() error {
Next: endpointHandler,
Counter: contourMetrics.EventHandlerOperations,
}, s.mgr.GetCache()); err != nil {
s.log.WithError(err).WithField("resource", "endpointslices").Fatal("failed to create informer")
s.log.WithError(err).WithField("resource", "endpoints").Fatal("failed to create informer")
}

Check warning on line 616 in cmd/contour/serve.go

View check run for this annotation

Codecov / codecov/patch

cmd/contour/serve.go#L603-L616

Added lines #L603 - L616 were not covered by tests
}

Expand Down
4 changes: 1 addition & 3 deletions cmd/contour/servecontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -588,9 +588,7 @@ func (ctx *serveContext) convertToContourConfigurationSpec() contour_api_v1alpha
Policy: policy,
Metrics: &contourMetrics,
Tracing: tracingConfig,
FeatureFlags: contour_api_v1alpha1.FeatureFlags{
UseEndpointSlice: ctx.Config.FeatureFlags.UseEndpointSlice,
},
FeatureFlags: ctx.Config.FeatureFlags,
}

xdsServerType := contour_api_v1alpha1.ContourServerType
Expand Down
30 changes: 14 additions & 16 deletions examples/contour/01-crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -449,14 +449,13 @@ spec:
type: object
type: object
featureFlags:
description: FeatureFlags defines toggle for new contour features.
properties:
useEndpointSlice:
description: useEndpointSlice configures contour to fetch endpoint
data from k8s endpoint slices. defaults to false and reading
endpoint data from the k8s endpoints.
type: boolean
type: object
description: FeatureFlags defines toggle to enable new contour features.
available toggles are useEndpointSlices - configures contour to
fetch endpoint data from k8s endpoint slices. defaults to false
and reading endpoint data from the k8s endpoints.
items:
type: string
type: array
gateway:
description: Gateway contains parameters for the gateway-api Gateway
that Contour is configured to serve traffic.
Expand Down Expand Up @@ -3899,14 +3898,13 @@ spec:
type: object
type: object
featureFlags:
description: FeatureFlags defines toggle for new contour features.
properties:
useEndpointSlice:
description: useEndpointSlice configures contour to fetch
endpoint data from k8s endpoint slices. defaults to false
and reading endpoint data from the k8s endpoints.
type: boolean
type: object
description: FeatureFlags defines toggle to enable new contour
features. available toggles are useEndpointSlices - configures
contour to fetch endpoint data from k8s endpoint slices. defaults
to false and reading endpoint data from the k8s endpoints.
items:
type: string
type: array
gateway:
description: Gateway contains parameters for the gateway-api Gateway
that Contour is configured to serve traffic.
Expand Down
30 changes: 14 additions & 16 deletions examples/render/contour-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -668,14 +668,13 @@ spec:
type: object
type: object
featureFlags:
description: FeatureFlags defines toggle for new contour features.
properties:
useEndpointSlice:
description: useEndpointSlice configures contour to fetch endpoint
data from k8s endpoint slices. defaults to false and reading
endpoint data from the k8s endpoints.
type: boolean
type: object
description: FeatureFlags defines toggle to enable new contour features.
available toggles are useEndpointSlices - configures contour to
fetch endpoint data from k8s endpoint slices. defaults to false
and reading endpoint data from the k8s endpoints.
items:
type: string
type: array
gateway:
description: Gateway contains parameters for the gateway-api Gateway
that Contour is configured to serve traffic.
Expand Down Expand Up @@ -4118,14 +4117,13 @@ spec:
type: object
type: object
featureFlags:
description: FeatureFlags defines toggle for new contour features.
properties:
useEndpointSlice:
description: useEndpointSlice configures contour to fetch
endpoint data from k8s endpoint slices. defaults to false
and reading endpoint data from the k8s endpoints.
type: boolean
type: object
description: FeatureFlags defines toggle to enable new contour
features. available toggles are useEndpointSlices - configures
contour to fetch endpoint data from k8s endpoint slices. defaults
to false and reading endpoint data from the k8s endpoints.
items:
type: string
type: array
gateway:
description: Gateway contains parameters for the gateway-api Gateway
that Contour is configured to serve traffic.
Expand Down
30 changes: 14 additions & 16 deletions examples/render/contour-gateway-provisioner.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -463,14 +463,13 @@ spec:
type: object
type: object
featureFlags:
description: FeatureFlags defines toggle for new contour features.
properties:
useEndpointSlice:
description: useEndpointSlice configures contour to fetch endpoint
data from k8s endpoint slices. defaults to false and reading
endpoint data from the k8s endpoints.
type: boolean
type: object
description: FeatureFlags defines toggle to enable new contour features.
available toggles are useEndpointSlices - configures contour to
fetch endpoint data from k8s endpoint slices. defaults to false
and reading endpoint data from the k8s endpoints.
items:
type: string
type: array
gateway:
description: Gateway contains parameters for the gateway-api Gateway
that Contour is configured to serve traffic.
Expand Down Expand Up @@ -3913,14 +3912,13 @@ spec:
type: object
type: object
featureFlags:
description: FeatureFlags defines toggle for new contour features.
properties:
useEndpointSlice:
description: useEndpointSlice configures contour to fetch
endpoint data from k8s endpoint slices. defaults to false
and reading endpoint data from the k8s endpoints.
type: boolean
type: object
description: FeatureFlags defines toggle to enable new contour
features. available toggles are useEndpointSlices - configures
contour to fetch endpoint data from k8s endpoint slices. defaults
to false and reading endpoint data from the k8s endpoints.
items:
type: string
type: array
gateway:
description: Gateway contains parameters for the gateway-api Gateway
that Contour is configured to serve traffic.
Expand Down
30 changes: 14 additions & 16 deletions examples/render/contour-gateway.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -674,14 +674,13 @@ spec:
type: object
type: object
featureFlags:
description: FeatureFlags defines toggle for new contour features.
properties:
useEndpointSlice:
description: useEndpointSlice configures contour to fetch endpoint
data from k8s endpoint slices. defaults to false and reading
endpoint data from the k8s endpoints.
type: boolean
type: object
description: FeatureFlags defines toggle to enable new contour features.
available toggles are useEndpointSlices - configures contour to
fetch endpoint data from k8s endpoint slices. defaults to false
and reading endpoint data from the k8s endpoints.
items:
type: string
type: array
gateway:
description: Gateway contains parameters for the gateway-api Gateway
that Contour is configured to serve traffic.
Expand Down Expand Up @@ -4124,14 +4123,13 @@ spec:
type: object
type: object
featureFlags:
description: FeatureFlags defines toggle for new contour features.
properties:
useEndpointSlice:
description: useEndpointSlice configures contour to fetch
endpoint data from k8s endpoint slices. defaults to false
and reading endpoint data from the k8s endpoints.
type: boolean
type: object
description: FeatureFlags defines toggle to enable new contour
features. available toggles are useEndpointSlices - configures
contour to fetch endpoint data from k8s endpoint slices. defaults
to false and reading endpoint data from the k8s endpoints.
items:
type: string
type: array
gateway:
description: Gateway contains parameters for the gateway-api Gateway
that Contour is configured to serve traffic.
Expand Down
Loading

0 comments on commit ee4aa23

Please sign in to comment.