From 1878227d5acb37febe9846eea880554508247c3c Mon Sep 17 00:00:00 2001 From: David Jumani Date: Tue, 2 Jul 2024 16:38:58 -0400 Subject: [PATCH 1/5] cli: Infer gloo deployment name --- projects/gloo/cli/pkg/cmd/check/gloo_stats.go | 9 ++++--- projects/gloo/cli/pkg/cmd/check/root.go | 7 +++++ projects/gloo/cli/pkg/common/get.go | 7 ++++- projects/gloo/cli/pkg/helpers/clients.go | 27 +++++++++++++++++++ .../cli/pkg/xdsinspection/get_last_config.go | 8 +++++- 5 files changed, 53 insertions(+), 5 deletions(-) diff --git a/projects/gloo/cli/pkg/cmd/check/gloo_stats.go b/projects/gloo/cli/pkg/cmd/check/gloo_stats.go index 0df6768873c..d245d938e00 100644 --- a/projects/gloo/cli/pkg/cmd/check/gloo_stats.go +++ b/projects/gloo/cli/pkg/cmd/check/gloo_stats.go @@ -27,6 +27,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 + customGlooDeploymentName = glooDeployment ) func ResourcesSyncedOverXds(stats, deploymentName string) bool { @@ -78,7 +81,7 @@ func checkXdsMetrics(opts *options.Options, glooNamespace string, deployments *v localPort := strconv.Itoa(freePort) adminPort := strconv.Itoa(int(defaults.GlooAdminPort)) // stats is the string containing all stats from /stats/prometheus - stats, portFwdCmd, err := cliutil.PortForwardGet(opts.Top.Ctx, glooNamespace, "deploy/"+glooDeployment, + stats, portFwdCmd, err := cliutil.PortForwardGet(opts.Top.Ctx, glooNamespace, "deploy/"+customGlooDeploymentName, localPort, adminPort, false, glooStatsPath) if err != nil { return err @@ -89,12 +92,12 @@ func checkXdsMetrics(opts *options.Options, glooNamespace string, deployments *v } 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) } diff --git a/projects/gloo/cli/pkg/cmd/check/root.go b/projects/gloo/cli/pkg/cmd/check/root.go index 72df95d6d7f..acfc6aa835d 100644 --- a/projects/gloo/cli/pkg/cmd/check/root.go +++ b/projects/gloo/cli/pkg/cmd/check/root.go @@ -113,6 +113,13 @@ 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) + // Fallback to the default deployment name if unable to fetch the custom one + customGlooDeploymentName = glooDeployment + } if included := doesNotContain(opts.Top.CheckName, "pods"); included { err := checkPods(ctx, opts) diff --git a/projects/gloo/cli/pkg/common/get.go b/projects/gloo/cli/pkg/common/get.go index 7d595f473ba..c9088458624 100644 --- a/projects/gloo/cli/pkg/common/get.go +++ b/projects/gloo/cli/pkg/common/get.go @@ -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 @@ -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() diff --git a/projects/gloo/cli/pkg/helpers/clients.go b/projects/gloo/cli/pkg/helpers/clients.go index 99d92612af4..36e42c62ef8 100644 --- a/projects/gloo/cli/pkg/helpers/clients.go +++ b/projects/gloo/cli/pkg/helpers/clients.go @@ -139,6 +139,33 @@ func KubeClient() (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 { + errMessage := "Unable to find the gloo deployment" + fmt.Println(errMessage) + return "", fmt.Errorf(errMessage+": %v", err) + } + return deployments.Items[0].Name, nil +} + func MustGetNamespaces(ctx context.Context) []string { ns, err := GetNamespaces(ctx) if err != nil { diff --git a/projects/gloo/cli/pkg/xdsinspection/get_last_config.go b/projects/gloo/cli/pkg/xdsinspection/get_last_config.go index c83a7012df7..e564af2d58c 100644 --- a/projects/gloo/cli/pkg/xdsinspection/get_last_config.go +++ b/projects/gloo/cli/pkg/xdsinspection/get_last_config.go @@ -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" @@ -38,6 +39,11 @@ const ( 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) @@ -45,7 +51,7 @@ func GetGlooXdsDump(ctx context.Context, proxyName, namespace string, verboseErr 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 From 588250e4a37e3d771b33fc8795ff2118cefa96c2 Mon Sep 17 00:00:00 2001 From: David Jumani Date: Tue, 2 Jul 2024 16:41:18 -0400 Subject: [PATCH 2/5] adding changelog --- changelog/v1.13.39/infer-gloo-deploy-name.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/v1.13.39/infer-gloo-deploy-name.yaml diff --git a/changelog/v1.13.39/infer-gloo-deploy-name.yaml b/changelog/v1.13.39/infer-gloo-deploy-name.yaml new file mode 100644 index 00000000000..5c3d5fd8ea3 --- /dev/null +++ b/changelog/v1.13.39/infer-gloo-deploy-name.yaml @@ -0,0 +1,5 @@ +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. From 43e2a668eccfc567e967d4f66a4e09ea6f7421fb Mon Sep 17 00:00:00 2001 From: David Jumani Date: Tue, 2 Jul 2024 17:18:06 -0400 Subject: [PATCH 3/5] fix tests --- projects/gloo/cli/pkg/cmd/check/root_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/projects/gloo/cli/pkg/cmd/check/root_test.go b/projects/gloo/cli/pkg/cmd/check/root_test.go index 349aabb4455..03d428faf16 100644 --- a/projects/gloo/cli/pkg/cmd/check/root_test.go +++ b/projects/gloo/cli/pkg/cmd/check/root_test.go @@ -56,6 +56,9 @@ var _ = Describe("Root", func() { ObjectMeta: metav1.ObjectMeta{ Name: appName, Namespace: "gloo-system", + Labels: map[string]string{ + "gloo": "gloo", + }, }, Spec: appsv1.DeploymentSpec{}, }, metav1.CreateOptions{}) @@ -95,6 +98,9 @@ var _ = Describe("Root", func() { ObjectMeta: metav1.ObjectMeta{ Name: appName, Namespace: "gloo-system", + Labels: map[string]string{ + "gloo": "gloo", + }, }, Spec: appsv1.DeploymentSpec{}, }, metav1.CreateOptions{}) @@ -176,6 +182,9 @@ var _ = Describe("Root", func() { ObjectMeta: metav1.ObjectMeta{ Name: appName, Namespace: myNs, + Labels: map[string]string{ + "gloo": "gloo", + }, }, Spec: appsv1.DeploymentSpec{}, }, metav1.CreateOptions{}) @@ -221,6 +230,9 @@ var _ = Describe("Root", func() { ObjectMeta: metav1.ObjectMeta{ Name: appName, Namespace: "gloo-system", + Labels: map[string]string{ + "gloo": "gloo", + }, }, Spec: appsv1.DeploymentSpec{}, }, metav1.CreateOptions{}) From d7653e028ef3b33cf1af4bf0920c9a544811ab58 Mon Sep 17 00:00:00 2001 From: David Jumani Date: Wed, 3 Jul 2024 09:44:47 -0400 Subject: [PATCH 4/5] Update projects/gloo/cli/pkg/helpers/clients.go Co-authored-by: Nathan Fudenberg --- projects/gloo/cli/pkg/helpers/clients.go | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/projects/gloo/cli/pkg/helpers/clients.go b/projects/gloo/cli/pkg/helpers/clients.go index 36e42c62ef8..b29a2587007 100644 --- a/projects/gloo/cli/pkg/helpers/clients.go +++ b/projects/gloo/cli/pkg/helpers/clients.go @@ -158,12 +158,25 @@ func GetGlooDeploymentName(ctx context.Context, namespace string) (string, error if err != nil { return "", err } - if len(deployments.Items) != 1 { - errMessage := "Unable to find the gloo deployment" - fmt.Println(errMessage) - return "", fmt.Errorf(errMessage+": %v", 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 deployment.Items{ + if d.Name != glooDeployment{ + // At least 1 deployment exists, in case we dont find default update our error message + errMessage = "too many app=gloo deployments, 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", glooDeployment) + return glooDeployment, nil + } + fmt.Println(errMessage) + return "", fmt.Errorf(errMessage+": %v", err) } - return deployments.Items[0].Name, nil + } func MustGetNamespaces(ctx context.Context) []string { From 70338368b494d6d48e0a46d4c3525f4f5911c1e6 Mon Sep 17 00:00:00 2001 From: David Jumani Date: Wed, 3 Jul 2024 10:02:04 -0400 Subject: [PATCH 5/5] fix syntax --- projects/gloo/cli/pkg/cmd/check/gloo_stats.go | 4 ++-- projects/gloo/cli/pkg/cmd/check/root.go | 2 -- projects/gloo/cli/pkg/helpers/clients.go | 14 ++++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/projects/gloo/cli/pkg/cmd/check/gloo_stats.go b/projects/gloo/cli/pkg/cmd/check/gloo_stats.go index d245d938e00..d7b27b831da 100644 --- a/projects/gloo/cli/pkg/cmd/check/gloo_stats.go +++ b/projects/gloo/cli/pkg/cmd/check/gloo_stats.go @@ -7,12 +7,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" @@ -29,7 +29,7 @@ var ( } // Initialize the custom deployment name that is overwritten later on - customGlooDeploymentName = glooDeployment + customGlooDeploymentName = helpers.GlooDeploymentName ) func ResourcesSyncedOverXds(stats, deploymentName string) bool { diff --git a/projects/gloo/cli/pkg/cmd/check/root.go b/projects/gloo/cli/pkg/cmd/check/root.go index acfc6aa835d..6a150d45538 100644 --- a/projects/gloo/cli/pkg/cmd/check/root.go +++ b/projects/gloo/cli/pkg/cmd/check/root.go @@ -117,8 +117,6 @@ func CheckResources(opts *options.Options) error { customGlooDeploymentName, err = helpers.GetGlooDeploymentName(opts.Top.Ctx, opts.Metadata.GetNamespace()) if err != nil { multiErr = multierror.Append(multiErr, err) - // Fallback to the default deployment name if unable to fetch the custom one - customGlooDeploymentName = glooDeployment } if included := doesNotContain(opts.Top.CheckName, "pods"); included { diff --git a/projects/gloo/cli/pkg/helpers/clients.go b/projects/gloo/cli/pkg/helpers/clients.go index b29a2587007..dae59402df9 100644 --- a/projects/gloo/cli/pkg/helpers/clients.go +++ b/projects/gloo/cli/pkg/helpers/clients.go @@ -45,6 +45,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) @@ -163,20 +167,18 @@ func GetGlooDeploymentName(ctx context.Context, namespace string) (string, error } errMessage := "Unable to find the gloo deployment" // if there are multiple we can reasonably use the default variant - for _, d := range deployment.Items{ - if d.Name != glooDeployment{ + 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 app=gloo deployments, 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", glooDeployment) - return glooDeployment, nil + 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 {