Multiple Pod IPs

.changelog/201.txt

@@ -0,0 +1,3 @@
+```release-note:note
+Gateway IP address assignment logic updated to include the case when multiple different pod IPs exist
+```

internal/k8s/gatewayclient/gatewayclient.go

@@ -58,7 +58,7 @@ type Client interface {
 
 	// general utilities
 
-	PodWithLabels(ctx context.Context, labels map[string]string) (*core.Pod, error)
+	PodsWithLabels(ctx context.Context, labels map[string]string) ([]core.Pod, error)
 
 	// status updates
 
@@ -153,26 +153,22 @@ func (g *gatewayClient) GatewayClassInUse(ctx context.Context, gc *gateway.Gatew
 	return false, nil
 }
 
-func (g *gatewayClient) PodWithLabels(ctx context.Context, labels map[string]string) (*core.Pod, error) {
+func (g *gatewayClient) PodsWithLabels(ctx context.Context, labels map[string]string) ([]core.Pod, error) {
 	list := &core.PodList{}
 	if err := g.List(ctx, list, client.MatchingLabels(labels)); err != nil {
 		return nil, NewK8sError(err)
 	}
 
-	// if we only have a single item, return it
-	if len(list.Items) == 1 {
-		return &list.Items[0], nil
-	}
+	items := []core.Pod{}
 
-	// we could potentially have two pods based off of one in the process of deletion
-	// return the first with a zero deletion timestamp
+	// return all pods that don't have a deletion timestamp
 	for _, pod := range list.Items {
 		if pod.DeletionTimestamp.IsZero() {
-			return &pod, nil
+			items = append(items, pod)
 		}
 	}
 
-	return nil, nil
+	return items, nil
 }
 
 func (g *gatewayClient) DeploymentForGateway(ctx context.Context, gw *gateway.Gateway) (*apps.Deployment, error) {

internal/k8s/gatewayclient/gatewayclient_test.go

@@ -360,17 +360,17 @@ func TestGatewayClassInUse(t *testing.T) {
 	require.NoError(t, err)
 	require.False(t, used)
 }
-func TestPodWithLabelsNoItems(t *testing.T) {
+func TestPodsWithLabelsNoItems(t *testing.T) {
 	gatewayclient := NewTestClient(nil)
 
-	pod, err := gatewayclient.PodWithLabels(context.Background(), map[string]string{
+	pods, err := gatewayclient.PodsWithLabels(context.Background(), map[string]string{
 		"label": "test",
 	})
 	require.NoError(t, err)
-	require.Nil(t, pod)
+	require.Equal(t, 0, len(pods))
 }
 
-func TestPodWithLabelsOneItem(t *testing.T) {
+func TestPodsWithLabelsOneItem(t *testing.T) {
 	labels := map[string]string{
 		"label": "test",
 	}
@@ -382,12 +382,12 @@ func TestPodWithLabelsOneItem(t *testing.T) {
 		}},
 	})
 
-	pod, err := gatewayclient.PodWithLabels(context.Background(), labels)
+	pod, err := gatewayclient.PodsWithLabels(context.Background(), labels)
 	require.NoError(t, err)
 	require.NotNil(t, pod)
 }
 
-func TestPodWithLabelsMultipleItems(t *testing.T) {
+func TestPodsWithLabelsMultipleItems(t *testing.T) {
 	labels := map[string]string{
 		"label": "test",
 	}
@@ -412,7 +412,8 @@ func TestPodWithLabelsMultipleItems(t *testing.T) {
 		}},
 	})
 
-	pod, err := gatewayclient.PodWithLabels(context.Background(), labels)
+	pods, err := gatewayclient.PodsWithLabels(context.Background(), labels)
 	require.NoError(t, err)
-	require.NotNil(t, pod)
+	require.NotNil(t, pods)
+	require.Equal(t, 2, len(pods))
 }

internal/k8s/reconciler/gateway.go

@@ -18,6 +18,7 @@ import (
 	"github.com/hashicorp/consul-api-gateway/internal/store"
 	apigwv1alpha1 "github.com/hashicorp/consul-api-gateway/pkg/apis/v1alpha1"
 	"github.com/hashicorp/go-hclog"
+	"golang.org/x/exp/slices"
 )
 
 type K8sGateway struct {
@@ -151,7 +152,7 @@ func (g *K8sGateway) validateListenerConflicts() {
 func (g *K8sGateway) validateGatewayIP(ctx context.Context) error {
 	service := g.serviceBuilder.Build()
 	if service == nil {
-		return g.assignGatewayIPFromPod(ctx)
+		return g.assignGatewayIPFromPods(ctx)
 	}
 
 	switch service.Spec.Type {
@@ -221,22 +222,26 @@ func (g *K8sGateway) assignGatewayIPFromService(ctx context.Context, service *co
 	return nil
 }
 
-// assignGatewayIPFromPod retrieves the internal IP for the Pod and assigns
+// assignGatewayIPFromPods retrieves the internal IP for the Pods and assigns
 // it to the Gateway.
-func (g *K8sGateway) assignGatewayIPFromPod(ctx context.Context) error {
-	pod, err := g.client.PodWithLabels(ctx, utils.LabelsForGateway(g.gateway))
+func (g *K8sGateway) assignGatewayIPFromPods(ctx context.Context) error {
+	pods, err := g.client.PodsWithLabels(ctx, utils.LabelsForGateway(g.gateway))
 	if err != nil {
 		return err
 	}
 
-	if pod == nil {
-		g.status.Scheduled.NotReconciled = errors.New("pod not found")
+	if len(pods) == 0 {
+		g.status.Scheduled.NotReconciled = errors.New("pods not found")
 		return nil
 	}
 
-	if pod.Status.PodIP != "" {
-		g.serviceReady = true
-		g.addresses = append(g.addresses, pod.Status.PodIP)
+	for _, pod := range pods {
+		if pod.Status.PodIP != "" {
+			g.serviceReady = true
+			if !slices.Contains(g.addresses, pod.Status.PodIP) {
+				g.addresses = append(g.addresses, pod.Status.PodIP)
+			}
+		}
 	}
 
 	return nil
@@ -248,31 +253,38 @@ func (g *K8sGateway) assignGatewayIPFromPod(ctx context.Context) error {
 // work by the practitioner such as port-forwarding or opening firewall rules to make
 // it externally accessible.
 func (g *K8sGateway) assignGatewayIPFromPodHost(ctx context.Context) error {
-	pod, err := g.client.PodWithLabels(ctx, utils.LabelsForGateway(g.gateway))
+	pods, err := g.client.PodsWithLabels(ctx, utils.LabelsForGateway(g.gateway))
 	if err != nil {
 		return err
 	}
 
-	if pod == nil {
-		g.status.Scheduled.NotReconciled = errors.New("pod not found")
+	if len(pods) == 0 {
+		g.status.Scheduled.NotReconciled = errors.New("pods not found")
 		return nil
 	}
 
-	if pod.Status.HostIP != "" {
-		g.serviceReady = true
-		g.addresses = append(g.addresses, pod.Status.HostIP)
+	for _, pod := range pods {
+		if pod.Status.HostIP != "" {
+			g.serviceReady = true
+
+			if !slices.Contains(g.addresses, pod.Status.HostIP) {
+				g.addresses = append(g.addresses, pod.Status.HostIP)
+			}
+		}
 	}
 
 	return nil
 }
 
 func (g *K8sGateway) validatePods(ctx context.Context) error {
-	pod, err := g.client.PodWithLabels(ctx, utils.LabelsForGateway(g.gateway))
+	pods, err := g.client.PodsWithLabels(ctx, utils.LabelsForGateway(g.gateway))
 	if err != nil {
 		return err
 	}
 
-	g.validatePodConditions(pod)
+	for _, pod := range pods {
+		g.validatePodConditions(&pod)
+	}
 
 	return nil
 }

internal/k8s/reconciler/gateway_test.go

@@ -54,15 +54,15 @@ func TestGatewayValidate(t *testing.T) {
 		},
 	})
 	client.EXPECT().GetSecret(gomock.Any(), gomock.Any()).Return(nil, nil)
-	client.EXPECT().PodWithLabels(gomock.Any(), gomock.Any()).Return(nil, nil).Times(2)
+	client.EXPECT().PodsWithLabels(gomock.Any(), gomock.Any()).Return(nil, nil).Times(2)
 	require.NoError(t, gateway.Validate(context.Background()))
 
 	expected := errors.New("expected")
-	client.EXPECT().PodWithLabels(gomock.Any(), gomock.Any()).Return(nil, nil).Times(2)
+	client.EXPECT().PodsWithLabels(gomock.Any(), gomock.Any()).Return(nil, nil).Times(2)
 	client.EXPECT().GetSecret(gomock.Any(), gomock.Any()).Return(nil, expected)
 	require.True(t, errors.Is(gateway.Validate(context.Background()), expected))
 
-	client.EXPECT().PodWithLabels(gomock.Any(), gomock.Any()).Return(nil, expected).Times(1)
+	client.EXPECT().PodsWithLabels(gomock.Any(), gomock.Any()).Return(nil, expected).Times(1)
 	require.True(t, errors.Is(gateway.Validate(context.Background()), expected))
 }
 
@@ -163,7 +163,7 @@ func TestGatewayValidateGatewayIP(t *testing.T) {
 			if tc.expectedIPFromSvc {
 				client.EXPECT().GetService(gomock.Any(), gomock.Any()).Return(svc, nil)
 			} else {
-				client.EXPECT().PodWithLabels(gomock.Any(), gomock.Any()).Return(pod, nil)
+				client.EXPECT().PodsWithLabels(gomock.Any(), gomock.Any()).Return([]core.Pod{*pod}, nil)
 			}
 			assert.NoError(t, gateway.validateGatewayIP(context.Background()))
 
@@ -206,7 +206,7 @@ func TestGatewayValidate_ListenerProtocolConflicts(t *testing.T) {
 		},
 		Client: client,
 	})
-	client.EXPECT().PodWithLabels(gomock.Any(), gomock.Any()).Return(nil, nil).Times(2)
+	client.EXPECT().PodsWithLabels(gomock.Any(), gomock.Any()).Return(nil, nil).Times(2)
 	require.NoError(t, gateway.Validate(context.Background()))
 	require.Equal(t, ListenerConditionReasonProtocolConflict, gateway.listeners["1"].status.Conflicted.Condition(0).Reason)
 	require.Equal(t, ListenerConditionReasonProtocolConflict, gateway.listeners["2"].status.Conflicted.Condition(0).Reason)
@@ -247,7 +247,7 @@ func TestGatewayValidate_ListenerHostnameConflicts(t *testing.T) {
 		},
 		Client: client,
 	})
-	client.EXPECT().PodWithLabels(gomock.Any(), gomock.Any()).Return(nil, nil).Times(2)
+	client.EXPECT().PodsWithLabels(gomock.Any(), gomock.Any()).Return(nil, nil).Times(2)
 	require.NoError(t, gateway.Validate(context.Background()))
 	require.Equal(t, ListenerConditionReasonHostnameConflict, gateway.listeners["1"].status.Conflicted.Condition(0).Reason)
 	require.Equal(t, ListenerConditionReasonHostnameConflict, gateway.listeners["2"].status.Conflicted.Condition(0).Reason)
@@ -278,63 +278,64 @@ func TestGatewayValidate_Pods(t *testing.T) {
 	})
 
 	// Pod has no/unknown status
-	client.EXPECT().PodWithLabels(gomock.Any(), gomock.Any()).Return(&core.Pod{
+	client.EXPECT().PodsWithLabels(gomock.Any(), gomock.Any()).Return([]core.Pod{{
 		Status: core.PodStatus{},
-	}, nil).Times(2)
+	}}, nil).Times(2)
 	require.NoError(t, gateway.Validate(context.Background()))
 	require.Equal(t, GatewayConditionReasonUnknown, gateway.status.Scheduled.Condition(0).Reason)
 
 	// Pod has pending status
-	client.EXPECT().PodWithLabels(gomock.Any(), gomock.Any()).Return(&core.Pod{
+	client.EXPECT().PodsWithLabels(gomock.Any(), gomock.Any()).Return([]core.Pod{{
 		Status: core.PodStatus{
 			Phase: core.PodPending,
 		},
-	}, nil).Times(2)
+	}}, nil).Times(2)
 	require.NoError(t, gateway.Validate(context.Background()))
 	require.Equal(t, GatewayConditionReasonNotReconciled, gateway.status.Scheduled.Condition(0).Reason)
 
 	// Pod is marked as unschedulable
-	client.EXPECT().PodWithLabels(gomock.Any(), gomock.Any()).Return(&core.Pod{
-		Status: core.PodStatus{
-			Phase: core.PodPending,
-			Conditions: []core.PodCondition{{
-				Type:   core.PodScheduled,
-				Status: core.ConditionFalse,
-				Reason: "Unschedulable",
-			}},
-		},
-	}, nil).Times(2)
+	client.EXPECT().PodsWithLabels(gomock.Any(), gomock.Any()).Return([]core.Pod{
+		{
+			Status: core.PodStatus{
+				Phase: core.PodPending,
+				Conditions: []core.PodCondition{{
+					Type:   core.PodScheduled,
+					Status: core.ConditionFalse,
+					Reason: "Unschedulable",
+				}},
+			},
+		}}, nil).Times(2)
 	require.NoError(t, gateway.Validate(context.Background()))
 	assert.Equal(t, GatewayConditionReasonNoResources, gateway.status.Scheduled.Condition(0).Reason)
 
 	// Pod has running status and is marked ready
-	client.EXPECT().PodWithLabels(gomock.Any(), gomock.Any()).Return(&core.Pod{
+	client.EXPECT().PodsWithLabels(gomock.Any(), gomock.Any()).Return([]core.Pod{{
 		Status: core.PodStatus{
 			Phase: core.PodRunning,
 			Conditions: []core.PodCondition{{
 				Type:   core.PodReady,
 				Status: core.ConditionTrue,
 			}},
 		},
-	}, nil).Times(2)
+	}}, nil).Times(2)
 	require.NoError(t, gateway.Validate(context.Background()))
 	assert.True(t, gateway.podReady)
 
 	// Pod has succeeded status
-	client.EXPECT().PodWithLabels(gomock.Any(), gomock.Any()).Return(&core.Pod{
+	client.EXPECT().PodsWithLabels(gomock.Any(), gomock.Any()).Return([]core.Pod{{
 		Status: core.PodStatus{
 			Phase: core.PodSucceeded,
 		},
-	}, nil).Times(2)
+	}}, nil).Times(2)
 	require.NoError(t, gateway.Validate(context.Background()))
 	assert.Equal(t, GatewayConditionReasonPodFailed, gateway.status.Scheduled.Condition(0).Reason)
 
 	// Pod has failed status
-	client.EXPECT().PodWithLabels(gomock.Any(), gomock.Any()).Return(&core.Pod{
+	client.EXPECT().PodsWithLabels(gomock.Any(), gomock.Any()).Return([]core.Pod{{
 		Status: core.PodStatus{
 			Phase: core.PodFailed,
 		},
-	}, nil).Times(2)
+	}}, nil).Times(2)
 	require.NoError(t, gateway.Validate(context.Background()))
 	assert.Equal(t, GatewayConditionReasonPodFailed, gateway.status.Scheduled.Condition(0).Reason)
 }

internal/k8s/reconciler/manager_test.go

@@ -87,10 +87,10 @@ func TestUpsertGateway(t *testing.T) {
 	require.NotEmpty(t, inner.Annotations[annotationConfig])
 
 	// validation
-	client.EXPECT().PodWithLabels(gomock.Any(), gomock.Any()).Return(nil, expected)
+	client.EXPECT().PodsWithLabels(gomock.Any(), gomock.Any()).Return(nil, expected)
 	require.Equal(t, expected, manager.UpsertGateway(context.Background(), inner))
 
-	client.EXPECT().PodWithLabels(gomock.Any(), gomock.Any()).Return(nil, nil).Times(2)
+	client.EXPECT().PodsWithLabels(gomock.Any(), gomock.Any()).Return(nil, nil).Times(2)
 	store.EXPECT().UpsertGateway(gomock.Any(), gomock.Any())
 	require.NoError(t, manager.UpsertGateway(context.Background(), inner))
 }