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

Backport of Refactor proxy list command, ensuring api-gateway Pods are included into release/1.1.x #4427

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: 3 additions & 0 deletions .changelog/4426.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
cli: fix issue where the `consul-k8s proxy list` command does not include API gateways.
```
97 changes: 64 additions & 33 deletions cli/cmd/proxy/list/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,22 @@ import (
"encoding/json"
"errors"
"fmt"
"sort"
"strings"
"sync"

"github.com/hashicorp/consul-k8s/cli/common"
"github.com/hashicorp/consul-k8s/cli/common/flag"
"github.com/hashicorp/consul-k8s/cli/common/terminal"
"github.com/posener/complete"
"golang.org/x/exp/maps"
helmCLI "helm.sh/helm/v3/pkg/cli"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/validation"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes"

"github.com/hashicorp/consul-k8s/cli/common"
"github.com/hashicorp/consul-k8s/cli/common/flag"
"github.com/hashicorp/consul-k8s/cli/common/terminal"
)

const (
Expand Down Expand Up @@ -205,36 +209,65 @@ func (c *ListCommand) namespace() string {
}
}

// fetchPods fetches all pods in flagNamespace which run Consul proxies.
// fetchPods fetches all pods in flagNamespace which run Consul proxies,
// making sure to return each pod only once even if multiple label selectors may
// return the same pod. The pods in the resulting list are grouped by proxy type
// and then sorted by namespace + name within each group.
func (c *ListCommand) fetchPods() ([]v1.Pod, error) {
var pods []v1.Pod
var (
apiGateways = make(map[types.NamespacedName]v1.Pod)
ingressGateways = make(map[types.NamespacedName]v1.Pod)
meshGateways = make(map[types.NamespacedName]v1.Pod)
terminatingGateways = make(map[types.NamespacedName]v1.Pod)
sidecars = make(map[types.NamespacedName]v1.Pod)
)

// Map target map for each proxy type. Note that some proxy types
// require multiple selectors and thus target the same map.
proxySelectors := []struct {
Target map[types.NamespacedName]v1.Pod
Selector string
}{
{Target: apiGateways, Selector: "component=api-gateway, gateway.consul.hashicorp.com/managed=true"},
{Target: apiGateways, Selector: "api-gateway.consul.hashicorp.com/managed=true"}, // Legacy API gateways
{Target: ingressGateways, Selector: "component=ingress-gateway, chart=consul-helm"},
{Target: meshGateways, Selector: "component=mesh-gateway, chart=consul-helm"},
{Target: terminatingGateways, Selector: "component=terminating-gateway, chart=consul-helm"},
{Target: sidecars, Selector: "consul.hashicorp.com/connect-inject-status=injected"},
}

// Fetch all pods in the namespace with labels matching the gateway component names.
gatewaypods, err := c.kubernetes.CoreV1().Pods(c.namespace()).List(c.Ctx, metav1.ListOptions{
LabelSelector: "component in (ingress-gateway, mesh-gateway, terminating-gateway), chart=consul-helm",
})
if err != nil {
return nil, err
for _, selector := range proxySelectors {
pods, err := c.kubernetes.CoreV1().Pods(c.namespace()).List(c.Ctx, metav1.ListOptions{
LabelSelector: selector.Selector,
})
if err != nil {
return nil, err
}

for _, pod := range pods.Items {
name := types.NamespacedName{Namespace: pod.Namespace, Name: pod.Name}
selector.Target[name] = pod
}
}
pods = append(pods, gatewaypods.Items...)

// Fetch all pods in the namespace with a label indicating they are an API gateway.
apigatewaypods, err := c.kubernetes.CoreV1().Pods(c.namespace()).List(c.Ctx, metav1.ListOptions{
LabelSelector: "api-gateway.consul.hashicorp.com/managed=true",
})
if err != nil {
return nil, err
// Collect all proxies into a single list of Pods, ordered by proxy type.
// Within each proxy type subgroup, order by namespace and then name for output readability.
var pods []v1.Pod
var podSources = []map[types.NamespacedName]v1.Pod{
apiGateways, ingressGateways, meshGateways, terminatingGateways, sidecars,
}
pods = append(pods, apigatewaypods.Items...)
for _, podSource := range podSources {
names := maps.Keys(podSource)

// Fetch all pods in the namespace with a label indicating they are a service networked by Consul.
sidecarpods, err := c.kubernetes.CoreV1().Pods(c.namespace()).List(c.Ctx, metav1.ListOptions{
LabelSelector: "consul.hashicorp.com/connect-inject-status=injected",
})
if err != nil {
return nil, err
// Insert Pods ordered by their NamespacedName which amounts to "<namespace>/<name>".
sort.SliceStable(names, func(i, j int) bool {
return strings.Compare(names[i].String(), names[j].String()) < 0
})

for _, name := range names {
pods = append(pods, podSource[name])
}
}
pods = append(pods, sidecarpods.Items...)

return pods, nil
}
Expand All @@ -250,12 +283,6 @@ func (c *ListCommand) output(pods []v1.Pod) {
return
}

if c.flagAllNamespaces {
c.UI.Output("Namespace: all namespaces\n")
} else {
c.UI.Output("Namespace: %s\n", c.namespace())
}

var tbl *terminal.Table
if c.flagAllNamespaces {
tbl = terminal.NewTable("Namespace", "Name", "Type")
Expand All @@ -266,7 +293,7 @@ func (c *ListCommand) output(pods []v1.Pod) {
for _, pod := range pods {
var proxyType string

// Get the type for ingress, mesh, and terminating gateways.
// Get the type for api, ingress, mesh, and terminating gateways + sidecars.
switch pod.Labels["component"] {
case "ingress-gateway":
proxyType = "Ingress Gateway"
Expand Down Expand Up @@ -302,6 +329,10 @@ func (c *ListCommand) output(pods []v1.Pod) {
c.UI.Output(string(jsonSt))
}
} else {
if !c.flagAllNamespaces {
c.UI.Output("Namespace: %s\n", c.namespace())
}

c.UI.Table(tbl)
}
}
34 changes: 23 additions & 11 deletions cli/cmd/proxy/list/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,24 @@ package list
import (
"bytes"
"context"
"encoding/json"
"flag"
"fmt"
"io"
"os"
"testing"

"github.com/hashicorp/consul-k8s/cli/common"
cmnFlag "github.com/hashicorp/consul-k8s/cli/common/flag"
"github.com/hashicorp/consul-k8s/cli/common/terminal"
"github.com/hashicorp/go-hclog"
"github.com/posener/complete"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"

"github.com/hashicorp/consul-k8s/cli/common"
cmnFlag "github.com/hashicorp/consul-k8s/cli/common/flag"
"github.com/hashicorp/consul-k8s/cli/common/terminal"
)

func TestFlagParsing(t *testing.T) {
Expand Down Expand Up @@ -229,6 +231,9 @@ func TestFetchPods(t *testing.T) {
}
}

// TestListCommandOutput tests the output of the list command. The output must
// contain the expected regular expressions and not contain the not expected
// regular expressions.
func TestListCommandOutput(t *testing.T) {
// These regular expressions must be present in the output.
expected := []string{
Expand Down Expand Up @@ -319,10 +324,10 @@ func TestListCommandOutput(t *testing.T) {
}
}

// TestListCommandOutputInJsonFormat tests the output of the list command when
// the output format is set to JSON. The proxies must be presented in the appropriate
// order and format.
func TestListCommandOutputInJsonFormat(t *testing.T) {
// These regular expressions must be present in the output.
expected := ".*Name.*mesh-gateway.*\n.*Namespace.*consul.*\n.*Type.*Mesh Gateway.*\n.*\n.*\n.*Name.*terminating-gateway.*\n.*Namespace.*consul.*\n.*Type.*Terminating Gateway.*\n.*\n.*\n.*Name.*ingress-gateway.*\n.*Namespace.*default.*\n.*Type.*Ingress Gateway.*\n.*\n.*\n.*Name.*api-gateway.*\n.*Namespace.*consul.*\n.*Type.*API Gateway.*\n.*\n.*\n.*Name.*pod1.*\n.*Namespace.*default.*\n.*Type.*Sidecar.*"
notExpected := "default.*dont-fetch.*Sidecar"

pods := []v1.Pod{
{
Expand Down Expand Up @@ -390,12 +395,19 @@ func TestListCommandOutputInJsonFormat(t *testing.T) {
out := c.Run([]string{"-A", "-o", "json"})
require.Equal(t, 0, out)

actual := buf.String()

require.Regexp(t, expected, actual)
for _, expression := range notExpected {
require.NotRegexp(t, expression, actual)
var actual []struct {
Name string `json:"Name"`
Namespace string `json:"Namespace"`
Type string `json:"Type"`
}
require.NoErrorf(t, json.Unmarshal(buf.Bytes(), &actual), "failed to parse json output: %s", buf.String())

require.Len(t, actual, 5)
assert.Equal(t, "api-gateway", actual[0].Name)
assert.Equal(t, "ingress-gateway", actual[1].Name)
assert.Equal(t, "mesh-gateway", actual[2].Name)
assert.Equal(t, "terminating-gateway", actual[3].Name)
assert.Equal(t, "pod1", actual[4].Name)
}

func TestNoPodsFound(t *testing.T) {
Expand Down
Loading