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

[1.15] cli: Infer gloo deploy name && optimize glooctl check #9724

Merged
merged 6 commits into from
Jul 9, 2024
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
13 changes: 13 additions & 0 deletions changelog/v1.15.30/infer-gloo-deploy-name.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
changelog:
- type: FIX
issueLink: https://github.com/solo-io/gloo/issues/9163
resolvesIssue: false
description: Infer the gloo deployment name in cases where the deployment name is not the default `gloo`. The gloo deployment is identified by the `gloo=gloo` label.
- type: FIX
issueLink: https://github.com/solo-io/gloo/issues/9673
resolvesIssue: true
description: Optimizes the `glooctl check` command by reducing the time taken to check resources by almost half in large environments consisting of over 500 namespaces
- type: FIX
issueLink: https://github.com/solo-io/gloo/issues/9644
resolvesIssue: false
description: Fix a bug where the service and function names of a discovered gRPC service are not printed when running glooctl get upstreams
11 changes: 7 additions & 4 deletions projects/gloo/cli/pkg/cmd/check/gloo_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import (

"github.com/solo-io/gloo/pkg/cliutil"
"github.com/solo-io/gloo/projects/gloo/cli/pkg/cmd/options"
"github.com/solo-io/gloo/projects/gloo/cli/pkg/helpers"
"github.com/solo-io/gloo/projects/gloo/pkg/defaults"
v1 "k8s.io/api/apps/v1"
)

const (
glooDeployment = "gloo"
rateLimitDeployment = "rate-limit"
glooStatsPath = "/metrics"

Expand All @@ -28,6 +28,9 @@ var (
return fmt.Sprintf("Gloo has detected that the data plane is out of sync. The following types of resources have not been accepted: %v. "+
"Gloo will not be able to process any other configuration updates until these errors are resolved.", resourceNames)
}

// Initialize the custom deployment name that is overwritten later on in the `CheckResources` function
customGlooDeploymentName = helpers.GlooDeploymentName
Copy link
Contributor

Choose a reason for hiding this comment

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

)

func ResourcesSyncedOverXds(stats, deploymentName string) bool {
Expand Down Expand Up @@ -83,7 +86,7 @@ func checkXdsMetrics(ctx context.Context, opts *options.Options, deployments *v1
printer.AppendCheck("Warning: checking xds with port forwarding is disabled\n")
return nil
}
stats, portFwdCmd, err := cliutil.PortForwardGet(ctx, opts.Metadata.GetNamespace(), "deploy/"+glooDeployment,
stats, portFwdCmd, err := cliutil.PortForwardGet(ctx, opts.Metadata.GetNamespace(), "deploy/"+customGlooDeploymentName,
localPort, adminPort, false, glooStatsPath)
if err != nil {
return err
Expand All @@ -94,12 +97,12 @@ func checkXdsMetrics(ctx context.Context, opts *options.Options, deployments *v1
}

if strings.TrimSpace(stats) == "" {
err := fmt.Sprint(errMessage+": could not find any metrics at", glooStatsPath, "endpoint of the "+glooDeployment+" deployment")
err := fmt.Sprint(errMessage+": could not find any metrics at", glooStatsPath, "endpoint of the "+customGlooDeploymentName+" deployment")
fmt.Println(err)
return fmt.Errorf(err)
}

if !ResourcesSyncedOverXds(stats, glooDeployment) {
if !ResourcesSyncedOverXds(stats, customGlooDeploymentName) {
fmt.Println(errMessage)
return fmt.Errorf(errMessage)
}
Expand Down
22 changes: 13 additions & 9 deletions projects/gloo/cli/pkg/cmd/check/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ func CheckResources(opts *options.Options) error {
multiErr = multierror.Append(multiErr, err)
}
}
// Fetch the gloo deployment name even if check deployments is disabled as it is used in other checks
customGlooDeploymentName, err = helpers.GetGlooDeploymentName(opts.Top.Ctx, opts.Metadata.GetNamespace())
if err != nil {
multiErr = multierror.Append(multiErr, err)
}

if included := doesNotContain(opts.Top.CheckName, "pods"); included {
err := checkPods(ctx, opts)
Expand Down Expand Up @@ -391,8 +396,8 @@ func checkUpstreams(ctx context.Context, opts *options.Options, namespaces []str
printer.AppendCheck("Checking upstreams... ")
var knownUpstreams []string
var multiErr *multierror.Error
client, err := helpers.UpstreamClient(ctx, namespaces)
for _, ns := range namespaces {
client, err := helpers.UpstreamClient(ctx, []string{ns})
if err != nil {
multiErr = multierror.Append(multiErr, err)
continue
Expand Down Expand Up @@ -435,8 +440,8 @@ func checkUpstreams(ctx context.Context, opts *options.Options, namespaces []str
func checkUpstreamGroups(ctx context.Context, opts *options.Options, namespaces []string) error {
printer.AppendCheck("Checking upstream groups... ")
var multiErr *multierror.Error
upstreamGroupClient, err := helpers.UpstreamGroupClient(ctx, namespaces)
for _, ns := range namespaces {
upstreamGroupClient, err := helpers.UpstreamGroupClient(ctx, []string{ns})
if err != nil {
multiErr = multierror.Append(multiErr, err)
continue
Expand Down Expand Up @@ -482,8 +487,8 @@ func checkAuthConfigs(ctx context.Context, opts *options.Options, namespaces []s
printer.AppendCheck("Checking auth configs... ")
var knownAuthConfigs []string
var multiErr *multierror.Error
authConfigClient, err := helpers.AuthConfigClient(ctx, namespaces)
for _, ns := range namespaces {
authConfigClient, err := helpers.AuthConfigClient(ctx, []string{ns})
if err != nil {
multiErr = multierror.Append(multiErr, err)
continue
Expand Down Expand Up @@ -527,9 +532,8 @@ func checkRateLimitConfigs(ctx context.Context, opts *options.Options, namespace
printer.AppendCheck("Checking rate limit configs... ")
var knownConfigs []string
var multiErr *multierror.Error
rlcClient, err := helpers.RateLimitConfigClient(ctx, namespaces)
for _, ns := range namespaces {

rlcClient, err := helpers.RateLimitConfigClient(ctx, []string{ns})
if err != nil {
if isCrdNotFoundErr(ratelimit.RateLimitConfigCrd, err) {
// Just warn. If the CRD is required, the check would have failed on the crashing gloo/gloo-ee pod.
Expand Down Expand Up @@ -567,8 +571,8 @@ func checkVirtualHostOptions(ctx context.Context, opts *options.Options, namespa
printer.AppendCheck("Checking VirtualHostOptions... ")
var knownVhOpts []string
var multiErr *multierror.Error
vhoptClient, err := helpers.VirtualHostOptionClient(ctx, namespaces)
for _, ns := range namespaces {
vhoptClient, err := helpers.VirtualHostOptionClient(ctx, []string{ns})
if err != nil {
if isCrdNotFoundErr(gatewayv1.VirtualHostOptionCrd, err) {
// Just warn. If the CRD is required, the check would have failed on the crashing gloo/gloo-ee pod.
Expand Down Expand Up @@ -615,8 +619,8 @@ func checkRouteOptions(ctx context.Context, opts *options.Options, namespaces []
printer.AppendCheck("Checking RouteOptions... ")
var knownRouteOpts []string
var multiErr *multierror.Error
routeOptionClient, err := helpers.RouteOptionClient(ctx, namespaces)
for _, ns := range namespaces {
routeOptionClient, err := helpers.RouteOptionClient(ctx, []string{ns})
if err != nil {
if isCrdNotFoundErr(gatewayv1.RouteOptionCrd, err) {
// Just warn. If the CRD is required, the check would have failed on the crashing gloo/gloo-ee pod.
Expand Down Expand Up @@ -663,8 +667,8 @@ func checkVirtualServices(ctx context.Context, opts *options.Options, namespaces
printer.AppendCheck("Checking virtual services... ")
var multiErr *multierror.Error

virtualServiceClient, err := helpers.VirtualServiceClient(ctx, namespaces)
for _, ns := range namespaces {
virtualServiceClient, err := helpers.VirtualServiceClient(ctx, []string{ns})
if err != nil {
multiErr = multierror.Append(multiErr, err)
continue
Expand Down Expand Up @@ -803,8 +807,8 @@ func checkVirtualServices(ctx context.Context, opts *options.Options, namespaces
func checkGateways(ctx context.Context, opts *options.Options, namespaces []string) error {
printer.AppendCheck("Checking gateways... ")
var multiErr *multierror.Error
gatewayClient, err := helpers.GatewayClient(ctx, namespaces)
for _, ns := range namespaces {
gatewayClient, err := helpers.GatewayClient(ctx, []string{ns})
if err != nil {
multiErr = multierror.Append(multiErr, err)
continue
Expand Down
7 changes: 6 additions & 1 deletion projects/gloo/cli/pkg/common/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,11 @@ func getProxiesFromK8s(name string, opts *options.Options) (gloov1.ProxyList, er
// if name is empty, return all proxies
func getProxiesFromGrpc(name string, namespace string, opts *options.Options, proxyEndpointPort string) (gloov1.ProxyList, error) {

glooDeploymentName, err := helpers.GetGlooDeploymentName(opts.Top.Ctx, opts.Metadata.GetNamespace())
if err != nil {
return nil, err
}

options := []grpc.CallOption{
// Some proxies can become very large and exceed the default 100Mb limit
// For this reason we want remove the limit but will settle for a limit of MaxInt32
Expand All @@ -198,7 +203,7 @@ func getProxiesFromGrpc(name string, namespace string, opts *options.Options, pr
return nil, err
}
localPort := strconv.Itoa(freePort)
portFwdCmd, err := cliutil.PortForward(opts.Metadata.GetNamespace(), "deployment/gloo",
portFwdCmd, err := cliutil.PortForward(opts.Metadata.GetNamespace(), "deployment/"+glooDeploymentName,
localPort, proxyEndpointPort, opts.Top.Verbose)
if portFwdCmd.Process != nil {
defer portFwdCmd.Process.Release()
Expand Down
42 changes: 42 additions & 0 deletions projects/gloo/cli/pkg/helpers/clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ var (
lock sync.Mutex
)

const (
GlooDeploymentName = "gloo"
)

// iterates over all the factory overrides, returning the first non-nil
// mem > consul
// if none set, return nil (callers will default to Kube CRD)
Expand Down Expand Up @@ -149,6 +153,44 @@ func KubeClientWithKubecontext(kubecontext string) (kubernetes.Interface, error)
return clientset, nil
}

func GetGlooDeploymentName(ctx context.Context, namespace string) (string, error) {
client, err := KubeClient()
if err != nil {
errMessage := "error getting KubeClient"
fmt.Println(errMessage)
return "", fmt.Errorf(errMessage+": %v", err)
}
_, err = client.CoreV1().Namespaces().Get(ctx, namespace, metav1.GetOptions{})
if err != nil {
errMessage := "Gloo namespace does not exist"
fmt.Println(errMessage)
return "", fmt.Errorf(errMessage+": %v", err)
}
deployments, err := client.AppsV1().Deployments(namespace).List(ctx, metav1.ListOptions{
LabelSelector: "gloo=gloo",
})
if err != nil {
return "", err
}
if len(deployments.Items) == 1 {
return deployments.Items[0].Name, nil
}
errMessage := "Unable to find the gloo deployment"
// if there are multiple we can reasonably use the default variant
for _, d := range deployments.Items {
if d.Name != GlooDeploymentName {
// At least 1 deployment exists, in case we dont find default update our error message
errMessage = "too many deployments labeled gloo=gloo; cannot decide which to target"
continue
}
// TODO: (nfuden) Remove this, while we should generally avoid println in our formatted output we already have alot of these
fmt.Println("multiple gloo labeled apps found, defaulting to", GlooDeploymentName)
return GlooDeploymentName, nil
}
fmt.Println(errMessage)
return "", fmt.Errorf(errMessage+": %v", err)
}

func MustGetNamespaces(ctx context.Context) []string {
ns, err := GetNamespaces(ctx)
if err != nil {
Expand Down
44 changes: 41 additions & 3 deletions projects/gloo/cli/pkg/printers/upstream.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ import (
"os"
"sort"

"github.com/golang/protobuf/protoc-gen-go/descriptor"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protodesc"
"google.golang.org/protobuf/reflect/protoreflect"

"github.com/solo-io/gloo/projects/gloo/cli/pkg/xdsinspection"
plugins "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options"
"github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/aws/ec2"
Expand Down Expand Up @@ -50,7 +55,6 @@ func UpstreamTable(xdsDump *xdsinspection.XdsDump, upstreams []*v1.Upstream, w i
table.Append([]string{"", "", "", line})
}
}

}

table.SetAlignment(tablewriter.ALIGN_LEFT)
Expand Down Expand Up @@ -216,13 +220,47 @@ func linesForServiceSpec(serviceSpec *plugins.ServiceSpec) []string {
add(fmt.Sprintf(" - %v", fn))
}
}
case *plugins.ServiceSpec_GrpcJsonTranscoder:
add("gRPC service:")
descriptorBin := plug.GrpcJsonTranscoder.GetProtoDescriptorBin()
for _, grpcService := range plug.GrpcJsonTranscoder.GetServices() {
add(fmt.Sprintf(" %v", grpcService))
methodDescriptors := getMethodDescriptors(grpcService, descriptorBin)
for i := 0; i < methodDescriptors.Len(); i++ {
add(fmt.Sprintf(" - %v", methodDescriptors.Get(i).Name()))
}
}
}

return spec
}

func getMethodDescriptors(service string, descriptorSet []byte) protoreflect.MethodDescriptors {
fds := &descriptor.FileDescriptorSet{}
err := proto.Unmarshal(descriptorSet, fds)
if err != nil {
fmt.Println("unable to unmarshal descriptor")
return nil
}
files, err := protodesc.NewFiles(fds)
if err != nil {
fmt.Println("unable to create proto registry files")
return nil
}
descriptor, err := files.FindDescriptorByName(protoreflect.FullName(service))
if err != nil {
fmt.Println("unable to fin descriptor")
return nil
}
serviceDescriptor, ok := descriptor.(protoreflect.ServiceDescriptor)
if !ok {
fmt.Println("unable to decode service descriptor")
return nil
}
return serviceDescriptor.Methods()
}

// stringifyKey for a resource likely could be done more nicely with spew
// or a better accessor but minimall this avoids panicing on nested references to nils
// or a better accessor but minimal this avoids panicing on nested references to nils
func stringifyKey(plausiblyNilRef *core.ResourceRef) string {

if plausiblyNilRef == nil {
Expand Down
36 changes: 36 additions & 0 deletions projects/gloo/cli/pkg/printers/upstream_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
package printers

import (
"bytes"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1"
"github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options"
"github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/grpc_json"
"github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/kubernetes"
"github.com/solo-io/solo-kit/pkg/api/v1/resources/core"
)

var _ = Describe("UpstreamTable", func() {
Expand All @@ -13,4 +19,34 @@ var _ = Describe("UpstreamTable", func() {
UpstreamTable(nil, []*v1.Upstream{us}, GinkgoWriter)
}).NotTo(Panic())
})

It("prints grpc upstream function names", func() {
us := &v1.Upstream{
Metadata: &core.Metadata{
Name: "test-us",
},
UpstreamType: &v1.Upstream_Kube{
Kube: &kubernetes.UpstreamSpec{
ServiceName: "test",
ServiceNamespace: "gloo-system",
ServiceSpec: &options.ServiceSpec{
PluginType: &options.ServiceSpec_GrpcJsonTranscoder{
GrpcJsonTranscoder: &grpc_json.GrpcJsonTranscoder{
DescriptorSet: &grpc_json.GrpcJsonTranscoder_ProtoDescriptorBin{
ProtoDescriptorBin: []byte{10, 230, 1, 10, 16, 104, 101, 108, 108, 111, 119, 111, 114, 108, 100, 46, 112, 114, 111, 116, 111, 18, 10, 104, 101, 108, 108, 111, 119, 111, 114, 108, 100, 34, 28, 10, 12, 72, 101, 108, 108, 111, 82, 101, 113, 117, 101, 115, 116, 18, 12, 10, 4, 110, 97, 109, 101, 24, 1, 32, 1, 40, 9, 34, 29, 10, 10, 72, 101, 108, 108, 111, 82, 101, 112, 108, 121, 18, 15, 10, 7, 109, 101, 115, 115, 97, 103, 101, 24, 1, 32, 1, 40, 9, 50, 73, 10, 7, 71, 114, 101, 101, 116, 101, 114, 18, 62, 10, 8, 83, 97, 121, 72, 101, 108, 108, 111, 18, 24, 46, 104, 101, 108, 108, 111, 119, 111, 114, 108, 100, 46, 72, 101, 108, 108, 111, 82, 101, 113, 117, 101, 115, 116, 26, 22, 46, 104, 101, 108, 108, 111, 119, 111, 114, 108, 100, 46, 72, 101, 108, 108, 111, 82, 101, 112, 108, 121, 34, 0, 66, 54, 10, 27, 105, 111, 46, 103, 114, 112, 99, 46, 101, 120, 97, 109, 112, 108, 101, 115, 46, 104, 101, 108, 108, 111, 119, 111, 114, 108, 100, 66, 15, 72, 101, 108, 108, 111, 87, 111, 114, 108, 100, 80, 114, 111, 116, 111, 80, 1, 162, 2, 3, 72, 76, 87, 98, 6, 112, 114, 111, 116, 111, 51},
},
Services: []string{"helloworld.Greeter"},
},
},
},
},
},
}

var out bytes.Buffer
UpstreamTable(nil, []*v1.Upstream{us}, &out)
// The `SayHello` method exists in the ProtoDescriptorBin. This should be printed when listing upstreams.
// Since there is only one service, it is safe to assume that this method belongs to it
Expect(out.String()).To(ContainSubstring("- SayHello"))
})
})
8 changes: 7 additions & 1 deletion projects/gloo/cli/pkg/xdsinspection/get_last_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
structpb "github.com/golang/protobuf/ptypes/struct"
"github.com/rotisserie/eris"
_ "github.com/solo-io/gloo/projects/envoyinit/hack/filter_types"
"github.com/solo-io/gloo/projects/gloo/cli/pkg/helpers"
"github.com/solo-io/gloo/projects/gloo/pkg/defaults"
"github.com/solo-io/go-utils/contextutils"
"go.uber.org/zap"
Expand All @@ -40,14 +41,19 @@ const (
// According to gloo
func GetGlooXdsDump(ctx context.Context, proxyName, namespace string, verboseErrors bool) (*XdsDump, error) {

glooDeploymentName, err := helpers.GetGlooDeploymentName(ctx, namespace)
if err != nil {
return nil, err
}

xdsPort := strconv.Itoa(int(defaults.GlooXdsPort))
// If gloo is in MTLS mode
glooMtlsCheck := exec.Command("kubectl", "get", "configmap", envoySidecarConfig, "-n", namespace)
if err := glooMtlsCheck.Run(); err == nil {
xdsPort = strconv.Itoa(int(defaults.GlooMtlsModeXdsPort))
}
portFwd := exec.Command("kubectl", "port-forward", "-n", namespace,
"deployment/gloo", xdsPort)
"deployment/"+glooDeploymentName, xdsPort)
mergedPortForwardOutput := bytes.NewBuffer([]byte{})
portFwd.Stdout = mergedPortForwardOutput
portFwd.Stderr = mergedPortForwardOutput
Expand Down
Loading
Loading