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

Make cluster stats names more parseable #10439

Merged
merged 14 commits into from
Dec 5, 2024
24 changes: 14 additions & 10 deletions .github/workflows/pr-kubernetes-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,36 +55,40 @@ jobs:
# NOTE: We use the GitHub action step time (as opposed to the `go test` time), because it is easier to capture

test:
# Nov 14, 2024: 22 minutes
# Dec 4, 2024: 22 minutes
- cluster-name: 'cluster-one'
go-test-args: '-v -timeout=25m'
go-test-run-regex: '^TestK8sGateway$$/^RouteDelegation$$|^TestGlooctlGlooGatewayEdgeGateway$$|^TestGlooctlK8sGateway$$'

# Nov 14, 2024: 22 minutes
# Dec 4, 2024: 23 minutes
- cluster-name: 'cluster-two'
go-test-args: '-v -timeout=25m'
go-test-run-regex: '^TestK8sGatewayIstioRevision$$|^TestRevisionIstioRegression$$|^TestK8sGateway$$/^Deployer$$|^TestK8sGateway$$/^RouteOptions$$|^TestK8sGateway$$/^VirtualHostOptions$$|^TestK8sGateway$$/^Upstreams$$|^TestK8sGateway$$/^HeadlessSvc$$|^TestK8sGateway$$/^PortRouting$$|^TestK8sGatewayMinimalDefaultGatewayParameters$$|^TestK8sGateway$$/^DirectResponse$$|^TestK8sGateway$$/^HttpListenerOptions$$|^TestK8sGateway$$/^ListenerOptions$$|^TestK8sGateway$$/^GlooAdminServer$$'

# Nov 14, 2024: 23 minutes
# Dec 4, 2024: 24 minutes
- cluster-name: 'cluster-three'
go-test-args: '-v -timeout=30m'
go-test-run-regex: '(^TestK8sGatewayIstioAutoMtls$$|^TestAutomtlsIstioEdgeApisGateway$$|^TestIstioEdgeApiGateway$$|^TestIstioRegression$$)'

# Nov 14, 2024: 21 minutes
# Dec 4, 2024: 21 minutes
- cluster-name: 'cluster-four'
go-test-args: '-v -timeout=30m'
go-test-run-regex: '(^TestK8sGatewayIstio$$|^TestGlooGatewayEdgeGateway$$|^TestGlooctlIstioInjectEdgeApiGateway$$)'

# Nov 14, 2024: 30 minutes
# TODO (sheidkamp) rebalance tests
# Dec 4, 2024: 24 minutes
- cluster-name: 'cluster-five'
go-test-args: '-v -timeout=35m'
go-test-run-regex: '^TestFullEnvoyValidation$$|^TestValidationStrict$$|^TestValidationAlwaysAccept$$|^TestTransformationValidationDisabled$$|^TestGloomtlsGatewayEdgeGateway$$|^TestWatchNamespaceSelector$$'
go-test-args: '-v -timeout=30m'
go-test-run-regex: '^TestFullEnvoyValidation$$|^TestValidationStrict$$|^TestValidationAlwaysAccept$$|^TestTransformationValidationDisabled$$'

# Nov 14, 2024: 23 minutes
# Dec 4, 2024: 26 minutes
- cluster-name: 'cluster-six'
go-test-args: '-v -timeout=30m'
go-test-run-regex: '^TestDiscoveryWatchlabels$$|^TestK8sGatewayNoValidation$$|^TestHelm$$|^TestHelmSettings$$|^TestK8sGatewayAws$$|^TestK8sGateway$$/^HTTPRouteServices$$|^TestK8sGateway$$/^TCPRouteServices$$|^TestZeroDowntimeRollout$$'

# Dec 4, 2024: 13 minutes
- cluster-name: 'cluster-seven'
go-test-args: '-v -timeout=25m'
go-test-run-regex: '^TestDiscoveryWatchlabels$$|^TestK8sGatewayNoValidation$$|^TestHelm$$|^TestHelmSettings$$|^TestK8sGatewayAws$$|^TestK8sGateway$$/^CRDCategories$$|^TestK8sGateway$$/^HTTPRouteServices$$|^TestK8sGateway$$/^TCPRouteServices$$|^TestZeroDowntimeRollout$$'
go-test-run-regex: '^TestK8sGateway$$/^CRDCategories$$|^TestK8sGateway$$/^Metrics$$|^TestGloomtlsGatewayEdgeGateway$$|^TestWatchNamespaceSelector$$'

# In our PR tests, we run the suite of tests using the upper ends of versions that we claim to support
# The versions should mirror: https://docs.solo.io/gloo-edge/latest/reference/support/
Expand Down
10 changes: 10 additions & 0 deletions changelog/v1.19.0-beta1/cluster-stats-names.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
changelog:
- type: BREAKING_CHANGE
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure if this should be considered breaking or not, given that it only affects stats names

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, dashboards and alerts can break, so I'd say it is a breaking change.

issueLink: https://github.com/solo-io/solo-projects/issues/7105
resolvesIssue: false
description: >-
When using the Kubernetes Gateway API and routing to Kubernetes Services or Kubernetes Upstreams, the
envoy cluster stats names are now in a new more parseable format that allows us to extract info about
the Service: `upstreamName_upstreamNs_svcNs_svcName_svcPort` (underscore-separated). Note that as long
as Kubernetes Gateway integration is enabled (i.e. Gloo Gateway is installed with `kubeGateway.enabled=true`),
both Edge and Kubernetes Gateway proxies will use the new stats name format for these clusters.
4 changes: 2 additions & 2 deletions pkg/utils/envoyutils/admincli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ This is the Go client that should be used whenever communicating with the Envoy
### Philosophy
We expose methods that return a [Command](/pkg/utils/cmdutils/cmd.go) which can be run by the calling code. Any methods that fit this structure, should end in `Cmd`:
```go
func StatsCmd(ctx context.Context) cmdutils.Cmd {}
func StatsCmd(ctx context.Context, queryParams map[string]string) cmdutils.Cmd {}
```

There are also methods that the client exposes which are [syntactic sugar](https://en.wikipedia.org/wiki/Syntactic_sugar) on top of this command API. These methods tend to follow the naming convention: `GetX`:
```go
func GetStats(ctx context.Context) (string, error) {}
func GetStats(ctx context.Context, queryParams map[string]string) (string, error) {}
```
_As a general practice, these methods should return a concrete type, whenever possible._
13 changes: 8 additions & 5 deletions pkg/utils/envoyutils/admincli/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,15 +135,18 @@ func (c *Client) RequestPathCmd(ctx context.Context, path string) cmdutils.Cmd {
}

// StatsCmd returns the cmdutils.Cmd that can be run to request data from the stats endpoint
func (c *Client) StatsCmd(ctx context.Context) cmdutils.Cmd {
return c.RequestPathCmd(ctx, StatsPath)
func (c *Client) StatsCmd(ctx context.Context, queryParams map[string]string) cmdutils.Cmd {
return c.Command(ctx,
curl.WithPath(StatsPath),
curl.WithQueryParameters(queryParams),
)
}

// GetStats returns the data that is available at the stats endpoint
func (c *Client) GetStats(ctx context.Context) (string, error) {
func (c *Client) GetStats(ctx context.Context, queryParams map[string]string) (string, error) {
var outLocation threadsafe.Buffer

err := c.StatsCmd(ctx).WithStdout(&outLocation).Run().Cause()
err := c.StatsCmd(ctx, queryParams).WithStdout(&outLocation).Run().Cause()
if err != nil {
return "", err
}
Expand Down Expand Up @@ -322,7 +325,7 @@ func (c *Client) WriteEnvoyDumpToZip(ctx context.Context, options DumpOptions, z
if err := c.ConfigDumpCmd(ctx, configParams).WithStdout(fileInArchive(zip, "config.json")).Run().Cause(); err != nil {
return err
}
if err := c.StatsCmd(ctx).WithStdout(fileInArchive(zip, "stats.txt")).Run().Cause(); err != nil {
if err := c.StatsCmd(ctx, nil).WithStdout(fileInArchive(zip, "stats.txt")).Run().Cause(); err != nil {
return err
}
if err := c.ClustersCmd(ctx).WithStdout(fileInArchive(zip, "clusters.txt")).Run().Cause(); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/envoyutils/admincli/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ var _ = Describe("Client", func() {
curl.WithoutRetries(),
)

statsCmd := client.StatsCmd(ctx).
statsCmd := client.StatsCmd(ctx, nil).
WithStdout(&outLocation).
WithStderr(&errLocation)

Expand Down
5 changes: 2 additions & 3 deletions projects/discovery/pkg/fds/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (

v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1"
plugins "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options"
"github.com/solo-io/gloo/projects/gloo/pkg/translator"
)

var errorUndetectableUpstream = errors.New("upstream type cannot be detected")
Expand Down Expand Up @@ -115,7 +114,7 @@ func (u *Updater) UpstreamUpdated(upstream *v1.Upstream) {

func (u *Updater) UpstreamAdded(upstream *v1.Upstream) {
// upstream already tracked. ignore.
key := translator.UpstreamToClusterName(upstream.GetMetadata().Ref())
key := upstream.GetMetadata().Ref().Key()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was part of the cleanup i carried over from #10403. in some plugins (etc) we are just using the "cluster name" as key for a map used internally, and i changed those instances to use Ref.Key instead

if _, ok := u.activeUpstreams[key]; ok {
return
}
Expand Down Expand Up @@ -144,7 +143,7 @@ func (u *Updater) UpstreamAdded(upstream *v1.Upstream) {
}

func (u *Updater) UpstreamRemoved(upstream *v1.Upstream) {
key := translator.UpstreamToClusterName(upstream.GetMetadata().Ref())
key := upstream.GetMetadata().Ref().Key()
if upstreamState, ok := u.activeUpstreams[key]; ok {
upstreamState.cancel()
delete(u.activeUpstreams, key)
Expand Down
2 changes: 1 addition & 1 deletion projects/gateway2/proxy_syncer/cla.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func PrioritizeEndpoints(logger *zap.Logger, destrule *DestinationRuleWrapper, e
var priorityInfo *PriorityInfo

if destrule != nil {
trafficPolicy := getTraficPolicy(destrule, ep.Port)
trafficPolicy := getTrafficPolicy(destrule, ep.Port)
localityLb := getLocalityLbSetting(trafficPolicy)
if localityLb != nil {
priorityInfo = getPriorityInfoFromDestrule(localityLb)
Expand Down
2 changes: 1 addition & 1 deletion projects/gateway2/proxy_syncer/destrule.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func getLocalityLbSetting(trafficPolicy *v1alpha3.TrafficPolicy) *v1alpha3.Local
return localityLb
}

func getTraficPolicy(destrule *DestinationRuleWrapper, port uint32) *v1alpha3.TrafficPolicy {
func getTrafficPolicy(destrule *DestinationRuleWrapper, port uint32) *v1alpha3.TrafficPolicy {
trafficPolicy := destrule.Spec.GetTrafficPolicy()
if trafficPolicy == nil {
return nil
Expand Down
2 changes: 1 addition & 1 deletion projects/gateway2/proxy_syncer/upstreams.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func translate(ctx context.Context, settings *gloov1.Settings, translator setup.

func ApplyDestRulesForUpstream(destrule *DestinationRuleWrapper, u *gloov1.Upstream) (*gloov1.Upstream, string) {
if destrule != nil {
trafficPolicy := getTraficPolicy(destrule, ggv2utils.GetPortForUpstream(u))
trafficPolicy := getTrafficPolicy(destrule, ggv2utils.GetPortForUpstream(u))
if outlier := trafficPolicy.GetOutlierDetection(); outlier != nil {
name := krtcollections.GetEndpointClusterName(u)

Expand Down
2 changes: 1 addition & 1 deletion projects/gateway2/setup/ggv2setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func (g *genericStatusReporter) WriteReports(ctx context.Context, resourceErrs r
for resource, report := range resourceErrsCopy {

// check if resource is an internal upstream. if so skip it..
if kubernetes.IsKubeUpstream(resource.GetMetadata().GetName()) {
if kubernetes.IsFakeKubeUpstream(resource.GetMetadata().GetName()) {
continue
}
// check if resource is an internal upstream. Internal upstreams have ':' in their names so
Expand Down
2 changes: 1 addition & 1 deletion projects/gloo/cli/pkg/cmd/gateway/dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func getEnvoyStatsDump(opts *options.Options) error {

defer shutdownFunc()

return adminCli.StatsCmd(opts.Top.Ctx).WithStdout(os.Stdout).Run().Cause()
return adminCli.StatsCmd(opts.Top.Ctx, nil).WithStdout(os.Stdout).Run().Cause()
}

func getEnvoyFullDumpToDisk(opts *options.Options) (string, error) {
Expand Down
5 changes: 2 additions & 3 deletions projects/gloo/pkg/plugins/aws/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/solo-io/gloo/projects/gloo/pkg/plugins"
"github.com/solo-io/gloo/projects/gloo/pkg/plugins/pluginutils"
"github.com/solo-io/gloo/projects/gloo/pkg/plugins/transformation"
"github.com/solo-io/gloo/projects/gloo/pkg/translator"
"github.com/solo-io/gloo/projects/gloo/pkg/upstreams"
"github.com/solo-io/gloo/projects/gloo/pkg/utils"
"github.com/solo-io/go-utils/contextutils"
Expand Down Expand Up @@ -99,7 +98,7 @@ func (p *Plugin) ProcessUpstream(params plugins.Params, in *v1.Upstream, out *en
return nil
}
// even if it failed, route should still be valid
p.recordedUpstreams[translator.UpstreamToClusterName(in.GetMetadata().Ref())] = upstreamSpec.Aws
p.recordedUpstreams[in.GetMetadata().Ref().Key()] = upstreamSpec.Aws

lambdaHostname := getLambdaHostname(upstreamSpec.Aws)

Expand Down Expand Up @@ -208,7 +207,7 @@ func (p *Plugin) ProcessRoute(params plugins.RouteParams, in *v1.Route, out *env
}

// validate that the upstream is one that we have previously recorded as an aws upstream
lambdaSpec, ok := p.recordedUpstreams[translator.UpstreamToClusterName(upstreamRef)]
lambdaSpec, ok := p.recordedUpstreams[upstreamRef.Key()]
if !ok {
if tryingNonExplicitAWSDest {
// skip the lambda plugin as the route was not explicitly set to be an aws route
Expand Down
5 changes: 2 additions & 3 deletions projects/gloo/pkg/plugins/azure/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/solo-io/gloo/projects/gloo/pkg/plugins"
"github.com/solo-io/gloo/projects/gloo/pkg/plugins/pluginutils"
"github.com/solo-io/gloo/projects/gloo/pkg/plugins/transformation"
"github.com/solo-io/gloo/projects/gloo/pkg/translator"
"github.com/solo-io/gloo/projects/gloo/pkg/upstreams"
"github.com/solo-io/gloo/projects/gloo/pkg/utils"
"github.com/solo-io/go-utils/contextutils"
Expand Down Expand Up @@ -64,7 +63,7 @@ func (p *plugin) ProcessUpstream(params plugins.Params, in *v1.Upstream, out *en
return nil
}
azureUpstream := upstreamSpec.Azure
p.recordedUpstreams[translator.UpstreamToClusterName(in.GetMetadata().Ref())] = azureUpstream
p.recordedUpstreams[in.GetMetadata().Ref().Key()] = azureUpstream

// configure Envoy cluster routing info
out.ClusterDiscoveryType = &envoy_config_cluster_v3.Cluster_Type{
Expand Down Expand Up @@ -126,7 +125,7 @@ func (p *plugin) ProcessRoute(params plugins.RouteParams, in *v1.Route, out *env
contextutils.LoggerFrom(p.ctx).Error(err)
return nil, err
}
upstreamSpec, ok := p.recordedUpstreams[translator.UpstreamToClusterName(upstreamRef)]
upstreamSpec, ok := p.recordedUpstreams[upstreamRef.Key()]
if !ok {
// TODO(yuval-k): panic in debug
return nil, errors.Errorf("%v is not an Azure upstream", *upstreamRef)
Expand Down
6 changes: 3 additions & 3 deletions projects/gloo/pkg/plugins/grpc/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (p *plugin) ProcessUpstream(params plugins.Params, in *v1.Upstream, out *en
// If the upstream uses the new API we should record that it exists for use in `ProcessRoute` but not make any changes
_, ok = upstreamType.GetServiceSpec().GetPluginType().(*glooplugins.ServiceSpec_GrpcJsonTranscoder)
if ok {
p.recordedUpstreams[translator.UpstreamToClusterName(in.GetMetadata().Ref())] = in
p.recordedUpstreams[in.GetMetadata().Ref().Key()] = in
return nil
}
grpcWrapper, ok := upstreamType.GetServiceSpec().GetPluginType().(*glooplugins.ServiceSpec_Grpc)
Expand Down Expand Up @@ -116,7 +116,7 @@ func (p *plugin) ProcessUpstream(params plugins.Params, in *v1.Upstream, out *en

addWellKnownProtos(descriptors)

p.recordedUpstreams[translator.UpstreamToClusterName(in.GetMetadata().Ref())] = in
p.recordedUpstreams[in.GetMetadata().Ref().Key()] = in
p.upstreamServices = append(p.upstreamServices, ServicesAndDescriptor{
Descriptors: descriptors,
Spec: grpcSpec,
Expand Down Expand Up @@ -184,7 +184,7 @@ func (p *plugin) ProcessRoute(params plugins.RouteParams, in *v1.Route, out *env
return nil, err
}

upstream := p.recordedUpstreams[translator.UpstreamToClusterName(upstreamRef)]
upstream := p.recordedUpstreams[upstreamRef.Key()]
if upstream == nil {
return nil, errors.New(fmt.Sprintf("upstream %v was not recorded for grpc route", upstreamRef))
}
Expand Down
19 changes: 12 additions & 7 deletions projects/gloo/pkg/plugins/kubernetes/uds_convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,13 @@ import (
"fmt"
"reflect"

"github.com/solo-io/gloo/projects/gloo/pkg/plugins/kubernetes/serviceconverter"
"github.com/solo-io/go-utils/contextutils"

"github.com/solo-io/gloo/projects/gloo/pkg/plugins/utils"

sanitizer "github.com/solo-io/k8s-utils/kubeutils"

v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1"
kubeplugin "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/kubernetes"
"github.com/solo-io/gloo/projects/gloo/pkg/discovery"
"github.com/solo-io/gloo/projects/gloo/pkg/plugins/kubernetes/serviceconverter"
"github.com/solo-io/gloo/projects/gloo/pkg/plugins/utils"
"github.com/solo-io/go-utils/contextutils"
sanitizer "github.com/solo-io/k8s-utils/kubeutils"
"github.com/solo-io/solo-kit/pkg/errors"
"github.com/solo-io/solo-kit/pkg/utils/kubeutils"
corev1 "k8s.io/api/core/v1"
Expand All @@ -34,6 +31,7 @@ type KubeUpstreamConverter struct {
serviceConverters []serviceconverter.ServiceConverter
}

// UpstreamsForService is called by the k8s uds plugin to convert a service to list of upstreams
func (uc *KubeUpstreamConverter) UpstreamsForService(ctx context.Context, svc *corev1.Service) v1.UpstreamList {
var upstreams v1.UpstreamList
for _, port := range svc.Spec.Ports {
Expand All @@ -42,6 +40,9 @@ func (uc *KubeUpstreamConverter) UpstreamsForService(ctx context.Context, svc *c
return upstreams
}

// CreateUpstream is called by both:
// - discovery (when creating an upstream from a k8s service)
// - controller code that converts services to in-memory upstreams
func (uc *KubeUpstreamConverter) CreateUpstream(ctx context.Context, svc *corev1.Service, port corev1.ServicePort) *v1.Upstream {
meta := svc.ObjectMeta
coremeta := kubeutils.FromKubeMeta(meta, false)
Expand Down Expand Up @@ -74,6 +75,10 @@ func (uc *KubeUpstreamConverter) CreateUpstream(ctx context.Context, svc *corev1
return us
}

// UpstreamName creates an upstream name from a k8s Service name/namespace/port.
//
// This function is used in the context of both "real" upstreams written to etcd (e.g. upstreams created by UDS)
// as well as "fake" upstreams (i.e. those upstreams we only create in the in-memory input snapshot).
func UpstreamName(serviceNamespace, serviceName string, servicePort int32) string {
return sanitizer.SanitizeNameV2(fmt.Sprintf("%s-%s-%v", serviceNamespace, serviceName, servicePort))
}
Expand Down
5 changes: 2 additions & 3 deletions projects/gloo/pkg/plugins/rest/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/solo-io/gloo/projects/gloo/pkg/plugins/pluginutils"
"github.com/solo-io/gloo/projects/gloo/pkg/plugins/transformation"
transformutils "github.com/solo-io/gloo/projects/gloo/pkg/plugins/utils/transformation"
"github.com/solo-io/gloo/projects/gloo/pkg/translator"
"github.com/solo-io/gloo/projects/gloo/pkg/upstreams"
"github.com/solo-io/go-utils/contextutils"
"github.com/solo-io/solo-kit/pkg/errors"
Expand Down Expand Up @@ -69,7 +68,7 @@ func (p *plugin) ProcessUpstream(params plugins.Params, in *v1.Upstream, _ *envo
if restServiceSpec.Rest == nil {
return errors.Errorf("%v has an empty rest service spec", in.GetMetadata().Ref())
}
p.recordedUpstreams[translator.UpstreamToClusterName(in.GetMetadata().Ref())] = restServiceSpec
p.recordedUpstreams[in.GetMetadata().Ref().Key()] = restServiceSpec
}
return nil
}
Expand All @@ -92,7 +91,7 @@ func (p *plugin) ProcessRoute(params plugins.RouteParams, in *v1.Route, out *env
contextutils.LoggerFrom(params.Ctx).Error(err)
return nil, err
}
restServiceSpec, ok := p.recordedUpstreams[translator.UpstreamToClusterName(upstreamRef)]
restServiceSpec, ok := p.recordedUpstreams[upstreamRef.Key()]
if !ok {
return nil, errors.Errorf("%s does not have a rest service spec", upstreamRef)
}
Expand Down
8 changes: 7 additions & 1 deletion projects/gloo/pkg/translator/clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"github.com/golang/protobuf/ptypes/wrappers"
"github.com/rotisserie/eris"
"github.com/solo-io/gloo/pkg/utils/api_conversion"
"github.com/solo-io/gloo/pkg/utils/envutils"
"github.com/solo-io/gloo/projects/gloo/constants"
v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1"
v1_options "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options"
"github.com/solo-io/gloo/projects/gloo/pkg/api/v1/ssl"
Expand Down Expand Up @@ -69,7 +71,7 @@ func (t *translatorInstance) computeClusters(
}

// This function is intented to be used when translating a single upstream outside of the context of a full snapshot.
// This happens in GGv2 krt implementation.
// This happens in the kube gateway krt implementation.
func (t *translatorInstance) TranslateCluster(
params plugins.Params,
upstream *v1.Upstream,
Expand Down Expand Up @@ -156,6 +158,10 @@ func (t *translatorInstance) initializeCluster(
DnsRefreshRate: dnsRefreshRate,
PreconnectPolicy: preconnect,
}
// for kube gateway, use new stats name format
if envutils.IsEnvTruthy(constants.GlooGatewayEnableK8sGwControllerEnv) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we do things like this elsewhere but after looking at it I feel like we should be checking these once at the start of a translation rather than checking mid translation where hypothetically some clusters may have this set true and some set false.

Not a problem with this pr more of a thought for our general strategies

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah we have this check and/or the proxy type check sprinkled throughout the codebase and it's a bit messy..but luckily we should be able to rip them out soon!

this is the only place that i saw that created a Cluster (besides some plugins that create Clusters for other purposes but with well-defined names that don't need this alt stat name)

out.AltStatName = UpstreamToClusterStatsName(upstream)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the most important part

}

if sslConfig := upstream.GetSslConfig(); sslConfig != nil {
applyDefaultsToUpstreamSslConfig(sslConfig, t.settings.GetUpstreamOptions())
Expand Down
2 changes: 1 addition & 1 deletion projects/gloo/pkg/translator/translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func (t *translatorInstance) translateClusterSubsystemComponents(params plugins.

upstreamRefKeyToEndpoints := createUpstreamToEndpointsMap(params.Snapshot.Upstreams, params.Snapshot.Endpoints)

// endpoints and listeners are shared between listeners
// endpoints and clusters are shared between listeners
logger.Debugf("computing envoy clusters for proxy: %v", proxy.GetMetadata().GetName())
clusters, clusterToUpstreamMap := t.computeClusters(params, reports, upstreamRefKeyToEndpoints, proxy)
logger.Debugf("computing envoy endpoints for proxy: %v", proxy.GetMetadata().GetName())
Expand Down
Loading