From fdb48414e00249d3ef96646fb9c1bd48ee8acf7b Mon Sep 17 00:00:00 2001 From: DanStough Date: Thu, 14 Sep 2023 11:11:43 -0400 Subject: [PATCH] feat(control-plane): v2 backoff on missing namespace --- control-plane/connect-inject/common/common.go | 21 ++++++++- .../connect-inject/common/common_test.go | 46 ++++++++++++++++++- .../endpointsv2/endpoints_controller.go | 7 +++ .../controllers/pod/pod_controller.go | 19 +++++++- .../pod/pod_controller_ent_test.go | 6 +++ 5 files changed, 95 insertions(+), 4 deletions(-) diff --git a/control-plane/connect-inject/common/common.go b/control-plane/connect-inject/common/common.go index 1e7cf5415c..76507eebe2 100644 --- a/control-plane/connect-inject/common/common.go +++ b/control-plane/connect-inject/common/common.go @@ -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: @@ -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 +} diff --git a/control-plane/connect-inject/common/common_test.go b/control-plane/connect-inject/common/common_test.go index 6cbbab5b88..a401923010 100644 --- a/control-plane/connect-inject/common/common_test.go +++ b/control-plane/connect-inject/common/common_test.go @@ -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) { @@ -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) + }) + } +} diff --git a/control-plane/connect-inject/controllers/endpointsv2/endpoints_controller.go b/control-plane/connect-inject/controllers/endpointsv2/endpoints_controller.go index dca7dad950..d18de35701 100644 --- a/control-plane/connect-inject/controllers/endpointsv2/endpoints_controller.go +++ b/control-plane/connect-inject/controllers/endpointsv2/endpoints_controller.go @@ -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) } diff --git a/control-plane/connect-inject/controllers/pod/pod_controller.go b/control-plane/connect-inject/controllers/pod/pod_controller.go index d21e3d663a..9b0e828e61 100644 --- a/control-plane/connect-inject/controllers/pod/pod_controller.go +++ b/control-plane/connect-inject/controllers/pod/pod_controller.go @@ -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) } @@ -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) } } @@ -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, }, diff --git a/control-plane/connect-inject/controllers/pod/pod_controller_ent_test.go b/control-plane/connect-inject/controllers/pod/pod_controller_ent_test.go index 802cb9d910..56fb362d9d 100644 --- a/control-plane/connect-inject/controllers/pod/pod_controller_ent_test.go +++ b/control-plane/connect-inject/controllers/pod/pod_controller_ent_test.go @@ -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 }