Skip to content

Commit

Permalink
feat(control-plane): v2 backoff on missing namespace
Browse files Browse the repository at this point in the history
  • Loading branch information
DanStough committed Sep 14, 2023
1 parent 38dc912 commit fdb4841
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 4 deletions.
21 changes: 20 additions & 1 deletion control-plane/connect-inject/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,17 @@ import (
"strings"

mapset "github.com/deckarep/golang-set"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/anypb"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"

"github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants"
pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"

"github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants"
)

// DetermineAndValidatePort behaves as follows:
Expand Down Expand Up @@ -177,3 +180,19 @@ func PortValueFromIntOrString(pod corev1.Pod, port intstr.IntOrString) (uint32,
}
return uint32(portVal), nil
}

// ConsulNamespaceIsNotFound checks the gRPC error code and message to determine if the
// error indicates that the namespace of for a resource does not yet exist.
func ConsulNamespaceIsNotFound(err error) bool {
if err == nil {
return false
}
s, ok := status.FromError(err)
if !ok {
return false
}
if codes.InvalidArgument == s.Code() && strings.Contains(s.Message(), "namespace resource not found") {
return true
}
return false
}
46 changes: 44 additions & 2 deletions control-plane/connect-inject/common/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,24 @@
package common

import (
"fmt"
"testing"

mapset "github.com/deckarep/golang-set"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/testing/protocmp"
"google.golang.org/protobuf/types/known/anypb"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants"
"github.com/hashicorp/consul-k8s/control-plane/namespaces"
pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"
"github.com/hashicorp/consul/proto-public/pbresource"

"github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants"
"github.com/hashicorp/consul-k8s/control-plane/namespaces"
)

func TestCommonDetermineAndValidatePort(t *testing.T) {
Expand Down Expand Up @@ -399,3 +403,41 @@ func TestGetPortProtocol(t *testing.T) {
})
}
}

func Test_ConsulNamespaceIsNotFound(t *testing.T) {
t.Parallel()

cases := []struct {
name string
input error
expectMissingNamespace bool
}{
{
name: "random error",
input: fmt.Errorf("namespace resource not found"),
expectMissingNamespace: false,
},
{
name: "grpc code is not InvalidArgument",
input: status.Error(codes.NotFound, "namespace resource not found"),
expectMissingNamespace: false,
},
{
name: "grpc code is InvalidArgument, but the message is not for namespaces",
input: status.Error(codes.InvalidArgument, "blurg resource not found"),
expectMissingNamespace: false,
},
{
name: "namespace is missing",
input: status.Error(codes.InvalidArgument, "namespace resource not found"),
expectMissingNamespace: true,
},
}

for _, tt := range cases {
t.Run(tt.name, func(t *testing.T) {
actual := ConsulNamespaceIsNotFound(tt.input)
require.Equal(t, tt.expectMissingNamespace, actual)
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,13 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu

//TODO: Maybe check service-enable label here on service/deployments/other pod owners
if err = r.registerService(ctx, resourceClient, service, workloadSelector); err != nil {
// We could be racing with the namespace controller.
// Requeue (which includes backoff) to try again.
if common.ConsulNamespaceIsNotFound(err) {
r.Log.Info("Consul namespace not found; re-queueing request",
"service", service.GetName(), "ns", req.Namespace, "consul-ns", r.getConsulNamespace(req.Namespace))
return ctrl.Result{Requeue: true}, nil
}
errs = multierror.Append(errs, err)
}

Expand Down
19 changes: 18 additions & 1 deletion control-plane/connect-inject/controllers/pod/pod_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,22 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu

if hasBeenInjected(pod) {
if err := r.writeProxyConfiguration(ctx, pod); err != nil {
// We could be racing with the namespace controller.
// Requeue (which includes backoff) to try again.
if common.ConsulNamespaceIsNotFound(err) {
r.Log.Info("Consul namespace not found; re-queueing request",
"pod", req.Name, "ns", req.Namespace, "consul-ns", r.getConsulNamespace(req.Namespace))
return ctrl.Result{Requeue: true}, nil
}
errs = multierror.Append(errs, err)
}

if err := r.writeWorkload(ctx, pod); err != nil {
if common.ConsulNamespaceIsNotFound(err) {
r.Log.Info("Consul namespace not found; re-queueing request",
"pod", req.Name, "ns", req.Namespace, "consul-ns", r.getConsulNamespace(req.Namespace))
return ctrl.Result{Requeue: true}, nil
}
errs = multierror.Append(errs, err)
}

Expand All @@ -158,6 +170,11 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
//}

if err := r.writeHealthStatus(ctx, pod); err != nil {
if common.ConsulNamespaceIsNotFound(err) {
r.Log.Info("Consul namespace not found; re-queueing request",
"pod", req.Name, "ns", req.Namespace, "consul-ns", r.getConsulNamespace(req.Namespace))
return ctrl.Result{Requeue: true}, nil
}
errs = multierror.Append(errs, err)
}
}
Expand Down Expand Up @@ -269,7 +286,7 @@ func (r *Controller) writeProxyConfiguration(ctx context.Context, pod corev1.Pod

req := &pbresource.WriteRequest{
Resource: &pbresource.Resource{
Id: getProxyConfigurationID(pod.GetName(), r.getConsulNamespace(pod.Namespace), r.getPartition()),
Id: getProxyConfigurationID(pod.GetName(), "banana", r.getPartition()),
Metadata: metaFromPod(pod),
Data: data,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,34 +11,40 @@ import "testing"
// Tests creating a Pod object in a non-default NS and Partition with namespaces set to mirroring
func TestReconcileCreatePodWithMirrorNamespaces(t *testing.T) {

//TODO: Add test case to cover Consul namespace missing and check for backoff
}

// TODO(dans)
// Tests updating a Pod object in a non-default NS and Partition with namespaces set to mirroring
func TestReconcileUpdatePodWithMirrorNamespaces(t *testing.T) {

//TODO: Add test case to cover Consul namespace missing and check for backoff
}

// TODO(dans)
// Tests deleting a Pod object in a non-default NS and Partition with namespaces set to mirroring
func TestReconcileDeletePodWithMirrorNamespaces(t *testing.T) {

//TODO: Add test case to cover Consul namespace missing and check for backoff
}

// TODO(dans)
// Tests creating a Pod object in a non-default NS and Partition with namespaces set to a destination
func TestReconcileCreatePodWithDestinationNamespace(t *testing.T) {

//TODO: Add test case to cover Consul namespace missing and check for backoff
}

// TODO(dans)
// Tests updating a Pod object in a non-default NS and Partition with namespaces set to a destination
func TestReconcileUpdatePodWithDestinationNamespace(t *testing.T) {

//TODO: Add test case to cover Consul namespace missing and check for backoff
}

// TODO(dans)
// Tests deleting a Pod object in a non-default NS and Partition with namespaces set to a destination
func TestReconcileDeletePodWithDestinationNamespace(t *testing.T) {

//TODO: Add test case to cover Consul namespace missing and check for backoff
}

0 comments on commit fdb4841

Please sign in to comment.