diff --git a/internal/controller/nginx/agent/command.go b/internal/controller/nginx/agent/command.go index af4a254a47..fdafa31fbc 100644 --- a/internal/controller/nginx/agent/command.go +++ b/internal/controller/nginx/agent/command.go @@ -17,7 +17,6 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "sigs.k8s.io/controller-runtime/pkg/client" @@ -26,6 +25,7 @@ import ( agentgrpc "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/grpc" grpcContext "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/grpc/context" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/grpc/messenger" + nginxTypes "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/types" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/status" ) @@ -77,7 +77,7 @@ func (cs *commandService) CreateConnection( return nil, errors.New("empty connection request") } - gi, ok := grpcContext.FromContext(ctx) + grpcInfo, ok := grpcContext.FromContext(ctx) if !ok { return nil, agentgrpc.ErrStatusInvalidConnection } @@ -86,8 +86,9 @@ func (cs *commandService) CreateConnection( podName := resource.GetContainerInfo().GetHostname() cs.logger.Info(fmt.Sprintf("Creating connection for nginx pod: %s", podName)) - owner, _, err := cs.getPodOwner(podName) - if err != nil { + name, depType := getAgentDeploymentNameAndType(resource.GetInstances()) + if name == (types.NamespacedName{}) || depType == "" { + err := errors.New("agent labels missing") response := &pb.CreateConnectionResponse{ Response: &pb.CommandResponse{ Status: pb.CommandResponse_COMMAND_STATUS_ERROR, @@ -96,15 +97,15 @@ func (cs *commandService) CreateConnection( }, } cs.logger.Error(err, "error getting pod owner") - return response, grpcStatus.Errorf(codes.Internal, "error getting pod owner %s", err.Error()) + return response, grpcStatus.Errorf(codes.InvalidArgument, "error getting pod owner: %s", err.Error()) } conn := agentgrpc.Connection{ - Parent: owner, - PodName: podName, + ParentName: name, + ParentType: depType, InstanceID: getNginxInstanceID(resource.GetInstances()), } - cs.connTracker.Track(gi.UUID, conn) + cs.connTracker.Track(grpcInfo.UUID, conn) return &pb.CreateConnectionResponse{ Response: &pb.CommandResponse{ @@ -126,27 +127,31 @@ func (cs *commandService) CreateConnection( func (cs *commandService) Subscribe(in pb.CommandService_SubscribeServer) error { ctx := in.Context() - gi, ok := grpcContext.FromContext(ctx) + grpcInfo, ok := grpcContext.FromContext(ctx) if !ok { return agentgrpc.ErrStatusInvalidConnection } - defer cs.connTracker.RemoveConnection(gi.UUID) + defer cs.connTracker.RemoveConnection(grpcInfo.UUID) // wait for the agent to report itself and nginx - conn, deployment, err := cs.waitForConnection(ctx, gi) + conn, deployment, err := cs.waitForConnection(ctx, grpcInfo) if err != nil { cs.logger.Error(err, "error waiting for connection") return err } - defer deployment.RemovePodStatus(conn.PodName) + defer deployment.RemovePodStatus(grpcInfo.UUID) - cs.logger.Info(fmt.Sprintf("Successfully connected to nginx agent %s", conn.PodName)) + cs.logger.Info( + "Successfully connected to nginx agent", + conn.ParentType, conn.ParentName, + "uuid", grpcInfo.UUID, + ) msgr := messenger.New(in) go msgr.Run(ctx) // apply current config before starting event loop - if err := cs.setInitialConfig(ctx, deployment, conn, msgr); err != nil { + if err := cs.setInitialConfig(ctx, &grpcInfo, deployment, conn, msgr); err != nil { return err } @@ -188,7 +193,7 @@ func (cs *commandService) Subscribe(in pb.CommandService_SubscribeServer) error cs.logger.V(1).Info("Sending configuration to agent", "requestType", msg.Type) if err := msgr.Send(ctx, req); err != nil { cs.logger.Error(err, "error sending request to agent") - deployment.SetPodErrorStatus(conn.PodName, err) + deployment.SetPodErrorStatus(grpcInfo.UUID, err) channels.ResponseCh <- struct{}{} return grpcStatus.Error(codes.Internal, err.Error()) @@ -198,8 +203,8 @@ func (cs *commandService) Subscribe(in pb.CommandService_SubscribeServer) error // Only broadcast operations should signal ResponseCh for coordination. pendingBroadcastRequest = &msg case err = <-msgr.Errors(): - cs.logger.Error(err, "connection error", "pod", conn.PodName) - deployment.SetPodErrorStatus(conn.PodName, err) + cs.logger.Error(err, "connection error", conn.ParentType, conn.ParentName, "uuid", grpcInfo.UUID) + deployment.SetPodErrorStatus(grpcInfo.UUID, err) select { case channels.ResponseCh <- struct{}{}: default: @@ -220,9 +225,9 @@ func (cs *commandService) Subscribe(in pb.CommandService_SubscribeServer) error continue } err := fmt.Errorf("msg: %s; error: %s", res.GetMessage(), res.GetError()) - deployment.SetPodErrorStatus(conn.PodName, err) + deployment.SetPodErrorStatus(grpcInfo.UUID, err) } else { - deployment.SetPodErrorStatus(conn.PodName, nil) + deployment.SetPodErrorStatus(grpcInfo.UUID, nil) } // Signal broadcast completion only for tracked broadcast operations. @@ -231,7 +236,11 @@ func (cs *commandService) Subscribe(in pb.CommandService_SubscribeServer) error pendingBroadcastRequest = nil channels.ResponseCh <- struct{}{} } else { - cs.logger.V(1).Info("Received response for non-broadcast request (likely initial config)", "pod", conn.PodName) + cs.logger.V(1).Info( + "Received response for non-broadcast request (likely initial config)", + conn.ParentType, conn.ParentName, + "uuid", grpcInfo.UUID, + ) } } } @@ -239,7 +248,7 @@ func (cs *commandService) Subscribe(in pb.CommandService_SubscribeServer) error func (cs *commandService) waitForConnection( ctx context.Context, - gi grpcContext.GrpcInfo, + grpcInfo grpcContext.GrpcInfo, ) (*agentgrpc.Connection, *Deployment, error) { ticker := time.NewTicker(time.Second) defer ticker.Stop() @@ -258,9 +267,9 @@ func (cs *commandService) waitForConnection( case <-timer.C: return nil, nil, err case <-ticker.C: - if conn := cs.connTracker.GetConnection(gi.UUID); conn.Ready() { + if conn := cs.connTracker.GetConnection(grpcInfo.UUID); conn.Ready() { // connection has been established, now ensure that the deployment exists in the store - if deployment := cs.nginxDeployments.Get(conn.Parent); deployment != nil { + if deployment := cs.nginxDeployments.Get(conn.ParentName); deployment != nil { return &conn, deployment, nil } err = deploymentStoreErr @@ -274,6 +283,7 @@ func (cs *commandService) waitForConnection( // setInitialConfig gets the initial configuration for this connection and applies it. func (cs *commandService) setInitialConfig( ctx context.Context, + grpcInfo *grpcContext.GrpcInfo, deployment *Deployment, conn *agentgrpc.Connection, msgr messenger.Messenger, @@ -281,23 +291,22 @@ func (cs *commandService) setInitialConfig( deployment.FileLock.Lock() defer deployment.FileLock.Unlock() - _, pod, err := cs.getPodOwner(conn.PodName) - if err != nil { - cs.logAndSendErrorStatus(deployment, conn, err) - - return grpcStatus.Error(codes.Internal, err.Error()) - } - if err := cs.validatePodImageVersion(pod, deployment.imageVersion); err != nil { - cs.logAndSendErrorStatus(deployment, conn, err) + if err := cs.validatePodImageVersion(conn.ParentName, conn.ParentType, deployment.imageVersion); err != nil { + cs.logAndSendErrorStatus(grpcInfo, deployment, conn, err) return grpcStatus.Errorf(codes.FailedPrecondition, "nginx image version validation failed: %s", err.Error()) } fileOverviews, configVersion := deployment.GetFileOverviews() - cs.logger.Info("Sending initial configuration to agent", "pod", conn.PodName, "configVersion", configVersion) + cs.logger.Info( + "Sending initial configuration to agent", + conn.ParentType, conn.ParentName, + "uuid", grpcInfo.UUID, + "configVersion", configVersion, + ) if err := msgr.Send(ctx, buildRequest(fileOverviews, conn.InstanceID, configVersion)); err != nil { - cs.logAndSendErrorStatus(deployment, conn, err) + cs.logAndSendErrorStatus(grpcInfo, deployment, conn, err) return grpcStatus.Error(codes.Internal, err.Error()) } @@ -321,7 +330,7 @@ func (cs *commandService) setInitialConfig( true, // poll immediately func(ctx context.Context) (bool, error) { if err := msgr.Send(ctx, buildPlusAPIRequest(action, conn.InstanceID)); err != nil { - cs.logAndSendErrorStatus(deployment, conn, err) + cs.logAndSendErrorStatus(grpcInfo, deployment, conn, err) return false, grpcStatus.Error(codes.Internal, err.Error()) } @@ -350,7 +359,7 @@ func (cs *commandService) setInitialConfig( cancel() } // send the status (error or nil) to the status queue - cs.logAndSendErrorStatus(deployment, conn, errors.Join(errs...)) + cs.logAndSendErrorStatus(grpcInfo, deployment, conn, errors.Join(errs...)) return nil } @@ -393,16 +402,25 @@ func (cs *commandService) waitForInitialConfigApply( // the full Deployment error status to the status queue. This ensures that any other Pod errors that already // exist on the Deployment are not overwritten. // If the error is nil, then we just enqueue the nil value and don't log it, which indicates success. -func (cs *commandService) logAndSendErrorStatus(deployment *Deployment, conn *agentgrpc.Connection, err error) { +func (cs *commandService) logAndSendErrorStatus( + grpcInfo *grpcContext.GrpcInfo, + deployment *Deployment, + conn *agentgrpc.Connection, + err error, +) { if err != nil { cs.logger.Error(err, "error sending request to agent") } else { - cs.logger.Info("Successfully configured nginx for new subscription", "pod", conn.PodName) + cs.logger.Info( + "Successfully configured nginx for new subscription", + conn.ParentType, conn.ParentName, + "uuid", grpcInfo.UUID, + ) } - deployment.SetPodErrorStatus(conn.PodName, err) + deployment.SetPodErrorStatus(grpcInfo.UUID, err) queueObj := &status.QueueObject{ - Deployment: conn.Parent, + Deployment: conn.ParentName, Error: deployment.GetConfigurationStatus(), UpdateType: status.UpdateAll, } @@ -454,99 +472,56 @@ func buildPlusAPIRequest(action *pb.NGINXPlusAction, instanceID string) *pb.Mana } } -func (cs *commandService) getPodOwner(podName string) (types.NamespacedName, *v1.Pod, error) { - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - var pods v1.PodList - listOpts := &client.ListOptions{ - FieldSelector: fields.SelectorFromSet(fields.Set{"metadata.name": podName}), - } - if err := cs.k8sReader.List(ctx, &pods, listOpts); err != nil { - return types.NamespacedName{}, nil, fmt.Errorf("error listing pods: %w", err) - } - - if len(pods.Items) == 0 { - return types.NamespacedName{}, nil, fmt.Errorf("no pods found with name %q", podName) - } - - if len(pods.Items) > 1 { - return types.NamespacedName{}, nil, fmt.Errorf("should only be one pod with name %q", podName) - } - pod := &pods.Items[0] - - podOwnerRefs := pod.GetOwnerReferences() - if len(podOwnerRefs) != 1 { - tooManyOwnersError := "expected one owner reference of the nginx Pod, got %d" - return types.NamespacedName{}, nil, fmt.Errorf(tooManyOwnersError, len(podOwnerRefs)) - } - - if podOwnerRefs[0].Kind != "ReplicaSet" && podOwnerRefs[0].Kind != "DaemonSet" { - err := fmt.Errorf("expected pod owner reference to be ReplicaSet or DaemonSet, got %s", podOwnerRefs[0].Kind) - return types.NamespacedName{}, nil, err - } - - if podOwnerRefs[0].Kind == "DaemonSet" { - return types.NamespacedName{Namespace: pod.Namespace, Name: podOwnerRefs[0].Name}, pod, nil - } - - var replicaSet appsv1.ReplicaSet - var replicaSetErr error - if err := wait.PollUntilContextCancel( - ctx, - 500*time.Millisecond, - true, /* poll immediately */ - func(ctx context.Context) (bool, error) { - if err := cs.k8sReader.Get( - ctx, - types.NamespacedName{Namespace: pod.Namespace, Name: podOwnerRefs[0].Name}, - &replicaSet, - ); err != nil { - replicaSetErr = err - return false, nil //nolint:nilerr // error is returned at the end - } - - return true, nil - }, - ); err != nil { - return types.NamespacedName{}, nil, fmt.Errorf("failed to get nginx Pod's ReplicaSet: %w", replicaSetErr) - } - - replicaOwnerRefs := replicaSet.GetOwnerReferences() - if len(replicaOwnerRefs) != 1 { - err := fmt.Errorf("expected one owner reference of the nginx ReplicaSet, got %d", len(replicaOwnerRefs)) - return types.NamespacedName{}, nil, err - } - - return types.NamespacedName{Namespace: pod.Namespace, Name: replicaOwnerRefs[0].Name}, pod, nil -} - // validatePodImageVersion checks if the pod's nginx container image version matches the expected version // from its deployment. Returns an error if versions don't match. func (cs *commandService) validatePodImageVersion( - pod *v1.Pod, + parent types.NamespacedName, + parentType string, expectedImage string, ) error { - var podNginxImage string + var nginxImage string + var found bool + + getNginxContainerImage := func(containers []v1.Container) (string, bool) { + for _, c := range containers { + if c.Name == "nginx" { + return c.Image, true + } + } + return "", false + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() - for _, container := range pod.Spec.Containers { - if container.Name == "nginx" { - podNginxImage = container.Image - break + switch parentType { + case nginxTypes.DaemonSetType: + ds := &appsv1.DaemonSet{} + if err := cs.k8sReader.Get(ctx, parent, ds); err != nil { + return fmt.Errorf("failed to get DaemonSet %s: %w", parent.String(), err) } + nginxImage, found = getNginxContainerImage(ds.Spec.Template.Spec.Containers) + case nginxTypes.DeploymentType: + deploy := &appsv1.Deployment{} + if err := cs.k8sReader.Get(ctx, parent, deploy); err != nil { + return fmt.Errorf("failed to get Deployment %s: %w", parent.String(), err) + } + nginxImage, found = getNginxContainerImage(deploy.Spec.Template.Spec.Containers) + default: + return fmt.Errorf("unknown parentType: %s", parentType) } - if podNginxImage == "" { - return fmt.Errorf("nginx container not found in pod %q", pod.Name) + + if !found { + return fmt.Errorf("nginx container not found in %s %q", parentType, parent.Name) } - // Compare images - if podNginxImage != expectedImage { - return fmt.Errorf("nginx image version mismatch: pod has %q but expected %q", podNginxImage, expectedImage) + if nginxImage != expectedImage { + return fmt.Errorf("nginx image version mismatch: has %q but expected %q", nginxImage, expectedImage) } - cs.logger.V(1).Info("Pod nginx image version validated successfully", - "podName", pod.Name, - "image", podNginxImage) + cs.logger.V(1).Info("nginx image version validated successfully", + "parent", parent.String(), + "image", nginxImage) return nil } @@ -562,7 +537,7 @@ func (cs *commandService) UpdateDataPlaneStatus( return nil, errors.New("empty UpdateDataPlaneStatus request") } - gi, ok := grpcContext.FromContext(ctx) + grpcInfo, ok := grpcContext.FromContext(ctx) if !ok { return nil, agentgrpc.ErrStatusInvalidConnection } @@ -572,7 +547,7 @@ func (cs *commandService) UpdateDataPlaneStatus( return nil, grpcStatus.Errorf(codes.InvalidArgument, "request does not contain nginx instanceID") } - cs.connTracker.SetInstanceID(gi.UUID, instanceID) + cs.connTracker.SetInstanceID(grpcInfo.UUID, instanceID) return &pb.UpdateDataPlaneStatusResponse{}, nil } @@ -589,6 +564,35 @@ func getNginxInstanceID(instances []*pb.Instance) string { return "" } +func getAgentDeploymentNameAndType(instances []*pb.Instance) (types.NamespacedName, string) { + var nsName types.NamespacedName + var depType string + + for _, instance := range instances { + instanceType := instance.GetInstanceMeta().GetInstanceType() + if instanceType == pb.InstanceMeta_INSTANCE_TYPE_AGENT { + labels := instance.GetInstanceConfig().GetAgentConfig().GetLabels() + + for _, label := range labels { + fields := label.GetFields() + + if val, ok := fields[nginxTypes.AgentOwnerNameLabel]; ok { + fullName := val.GetStringValue() + parts := strings.SplitN(fullName, "_", 2) + if len(parts) == 2 { + nsName = types.NamespacedName{Namespace: parts[0], Name: parts[1]} + } + } + if val, ok := fields[nginxTypes.AgentOwnerTypeLabel]; ok { + depType = val.GetStringValue() + } + } + } + } + + return nsName, depType +} + // UpdateDataPlaneHealth includes full health information about the data plane as reported by the agent. func (*commandService) UpdateDataPlaneHealth( context.Context, diff --git a/internal/controller/nginx/agent/command_test.go b/internal/controller/nginx/agent/command_test.go index fb14c47ea3..db2fda1cf6 100644 --- a/internal/controller/nginx/agent/command_test.go +++ b/internal/controller/nginx/agent/command_test.go @@ -11,6 +11,7 @@ import ( pb "github.com/nginx/agent/v3/api/grpc/mpi/v1" . "github.com/onsi/gomega" "google.golang.org/grpc" + "google.golang.org/protobuf/types/known/structpb" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -25,6 +26,7 @@ import ( grpcContext "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/grpc/context" agentgrpcfakes "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/grpc/grpcfakes" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/grpc/messenger/messengerfakes" + nginxTypes "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/types" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/status" ) @@ -85,25 +87,19 @@ func createGrpcContextWithCancel() (context.Context, context.CancelFunc) { }), cancel } -func getDefaultPodList() []runtime.Object { - pod := &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-pod", - Namespace: "test", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "ReplicaSet", - Name: "nginx-replicaset", - }, - }, - }, +func getDefaultResources() []runtime.Object { + deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nginx-deployment", + Namespace: "test", + }, + Spec: appsv1.DeploymentSpec{ + Template: v1.PodTemplateSpec{ Spec: v1.PodSpec{ Containers: []v1.Container{ { - Image: "nginx:v1.0.0", Name: "nginx", + Image: "nginx:v1.0.0", }, }, }, @@ -111,20 +107,7 @@ func getDefaultPodList() []runtime.Object { }, } - replicaSet := &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-replicaset", - Namespace: "test", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "Deployment", - Name: "nginx-deployment", - }, - }, - }, - } - - return []runtime.Object{pod, replicaSet} + return []runtime.Object{deployment} } func TestCreateConnection(t *testing.T) { @@ -154,6 +137,33 @@ func TestCreateConnection(t *testing.T) { InstanceType: pb.InstanceMeta_INSTANCE_TYPE_NGINX, }, }, + { + InstanceMeta: &pb.InstanceMeta{ + InstanceType: pb.InstanceMeta_INSTANCE_TYPE_AGENT, + }, + InstanceConfig: &pb.InstanceConfig{ + Config: &pb.InstanceConfig_AgentConfig{ + AgentConfig: &pb.AgentConfig{ + Labels: []*structpb.Struct{ + { + Fields: map[string]*structpb.Value{ + nginxTypes.AgentOwnerNameLabel: { + Kind: &structpb.Value_StringValue{ + StringValue: "test_nginx-deployment", + }, + }, + nginxTypes.AgentOwnerTypeLabel: { + Kind: &structpb.Value_StringValue{ + StringValue: nginxTypes.DeploymentType, + }, + }, + }, + }, + }, + }, + }, + }, + }, }, }, }, @@ -192,7 +202,7 @@ func TestCreateConnection(t *testing.T) { Response: &pb.CommandResponse{ Status: pb.CommandResponse_COMMAND_STATUS_ERROR, Message: "error getting pod owner", - Error: "no pods found with name \"nginx-pod\"", + Error: "agent labels missing", }, }, errString: "error getting pod owner", @@ -208,7 +218,7 @@ func TestCreateConnection(t *testing.T) { var objs []runtime.Object if test.errString != "error getting pod owner" { - objs = getDefaultPodList() + objs = getDefaultResources() } fakeClient, err := createFakeK8sClient(objs...) g.Expect(err).ToNot(HaveOccurred()) @@ -236,8 +246,8 @@ func TestCreateConnection(t *testing.T) { g.Expect(connTracker.TrackCallCount()).To(Equal(1)) expConn := agentgrpc.Connection{ - Parent: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, - PodName: "nginx-pod", + ParentName: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, + ParentType: nginxTypes.DeploymentType, InstanceID: "nginx-id", } @@ -303,13 +313,13 @@ func TestSubscribe(t *testing.T) { connTracker := agentgrpcfakes.FakeConnectionsTracker{} conn := agentgrpc.Connection{ - Parent: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, - PodName: "nginx-pod", + ParentName: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, + ParentType: nginxTypes.DeploymentType, InstanceID: "nginx-id", } connTracker.GetConnectionReturns(conn) - fakeClient, err := createFakeK8sClient(getDefaultPodList()...) + fakeClient, err := createFakeK8sClient(getDefaultResources()...) g.Expect(err).ToNot(HaveOccurred()) store := NewDeploymentStore(&connTracker) @@ -332,7 +342,7 @@ func TestSubscribe(t *testing.T) { broadcaster.SubscribeReturns(subChannels) // set the initial files and actions to be applied by the Subscription - deployment := store.StoreWithBroadcaster(conn.Parent, broadcaster) + deployment := store.StoreWithBroadcaster(conn.ParentName, broadcaster) files := []File{ { Meta: &pb.FileMeta{ @@ -431,7 +441,7 @@ func TestSubscribe(t *testing.T) { g.Eventually(func() map[string]error { return deployment.podStatuses - }).Should(HaveKey("nginx-pod")) + }).Should(HaveKey("1234567")) cancel() @@ -439,7 +449,7 @@ func TestSubscribe(t *testing.T) { return <-errCh }).Should(MatchError(ContainSubstring("context canceled"))) - g.Expect(deployment.podStatuses).ToNot(HaveKey("nginx-pod")) + g.Expect(deployment.podStatuses).ToNot(HaveKey("1234567")) } func TestSubscribe_Reset(t *testing.T) { @@ -448,13 +458,13 @@ func TestSubscribe_Reset(t *testing.T) { connTracker := agentgrpcfakes.FakeConnectionsTracker{} conn := agentgrpc.Connection{ - Parent: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, - PodName: "nginx-pod", + ParentName: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, + ParentType: nginxTypes.DeploymentType, InstanceID: "nginx-id", } connTracker.GetConnectionReturns(conn) - fakeClient, err := createFakeK8sClient(getDefaultPodList()...) + fakeClient, err := createFakeK8sClient(getDefaultResources()...) g.Expect(err).ToNot(HaveOccurred()) store := NewDeploymentStore(&connTracker) @@ -478,7 +488,7 @@ func TestSubscribe_Reset(t *testing.T) { broadcaster.SubscribeReturns(subChannels) // set the initial files to be applied by the Subscription - deployment := store.StoreWithBroadcaster(conn.Parent, broadcaster) + deployment := store.StoreWithBroadcaster(conn.ParentName, broadcaster) files := []File{ { Meta: &pb.FileMeta{ @@ -612,7 +622,6 @@ func TestSetInitialConfig_Errors(t *testing.T) { setup func(msgr *messengerfakes.FakeMessenger, deployment *Deployment) name string errString string - podList []runtime.Object }{ { name: "error sending initial config", @@ -677,7 +686,7 @@ func TestSetInitialConfig_Errors(t *testing.T) { setup: func(_ *messengerfakes.FakeMessenger, deployment *Deployment) { deployment.SetImageVersion("nginx:v2.0.0") }, - errString: "nginx image version mismatch: pod has \"nginx:v1.0.0\" but expected \"nginx:v2.0.0\"", + errString: "nginx image version mismatch: has \"nginx:v1.0.0\" but expected \"nginx:v2.0.0\"", }, } @@ -689,12 +698,7 @@ func TestSetInitialConfig_Errors(t *testing.T) { connTracker := agentgrpcfakes.FakeConnectionsTracker{} msgr := &messengerfakes.FakeMessenger{} - podList := test.podList - if len(podList) == 0 { - podList = getDefaultPodList() - } - - fakeClient, err := createFakeK8sClient(podList...) + fakeClient, err := createFakeK8sClient(getDefaultResources()...) g.Expect(err).ToNot(HaveOccurred()) cs := newCommandService( @@ -707,9 +711,9 @@ func TestSetInitialConfig_Errors(t *testing.T) { ) conn := &agentgrpc.Connection{ - Parent: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, - PodName: "nginx-pod", + ParentName: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, InstanceID: "nginx-id", + ParentType: nginxTypes.DeploymentType, } deployment := newDeployment(&broadcastfakes.FakeBroadcaster{}) @@ -719,7 +723,7 @@ func TestSetInitialConfig_Errors(t *testing.T) { test.setup(msgr, deployment) } - err = cs.setInitialConfig(context.Background(), deployment, conn, msgr) + err = cs.setInitialConfig(context.Background(), &grpcContext.GrpcInfo{}, deployment, conn, msgr) g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring(test.errString)) @@ -727,212 +731,6 @@ func TestSetInitialConfig_Errors(t *testing.T) { } } -func TestGetPodOwner(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - podName string - podList *v1.PodList - replicaSet *appsv1.ReplicaSet - errString string - expected types.NamespacedName - }{ - { - name: "successfully gets pod owner; ReplicaSet", - podName: "nginx-pod", - podList: &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-pod", - Namespace: "test", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "ReplicaSet", - Name: "nginx-replicaset", - }, - }, - }, - }, - }, - }, - replicaSet: &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-replicaset", - Namespace: "test", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "Deployment", - Name: "nginx-deployment", - }, - }, - }, - }, - expected: types.NamespacedName{ - Namespace: "test", - Name: "nginx-deployment", - }, - }, - { - name: "successfully gets pod owner; DaemonSet", - podName: "nginx-pod", - podList: &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-pod", - Namespace: "test", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "DaemonSet", - Name: "nginx-daemonset", - }, - }, - }, - }, - }, - }, - replicaSet: &appsv1.ReplicaSet{}, - expected: types.NamespacedName{ - Namespace: "test", - Name: "nginx-daemonset", - }, - }, - { - name: "error listing pods", - podName: "nginx-pod", - podList: &v1.PodList{}, - replicaSet: &appsv1.ReplicaSet{}, - errString: "no pods found", - }, - { - name: "multiple pods with same name", - podName: "nginx-pod", - podList: &v1.PodList{ - Items: []v1.Pod{ - {ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "nginx-pod"}}, - {ObjectMeta: metav1.ObjectMeta{Namespace: "test2", Name: "nginx-pod"}}, - }, - }, - replicaSet: &appsv1.ReplicaSet{}, - errString: "should only be one pod with name", - }, - { - name: "pod owner reference is not ReplicaSet or DaemonSet", - podName: "nginx-pod", - podList: &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-pod", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "Owner", - Name: "nginx-owner", - }, - }, - }, - }, - }, - }, - replicaSet: &appsv1.ReplicaSet{}, - errString: "expected pod owner reference to be ReplicaSet or DaemonSet", - }, - { - name: "pod has multiple owners", - podName: "nginx-pod", - podList: &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-pod", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "ReplicaSet", - Name: "nginx-replicaset", - }, - { - Kind: "ReplicaSet", - Name: "nginx-replicaset2", - }, - }, - }, - }, - }, - }, - replicaSet: &appsv1.ReplicaSet{}, - errString: "expected one owner reference of the nginx Pod", - }, - { - name: "replicaSet has multiple owners", - podName: "nginx-pod", - podList: &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-pod", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "ReplicaSet", - Name: "nginx-replicaset", - }, - }, - }, - }, - }, - }, - replicaSet: &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-replicaset", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "Deployment", - Name: "nginx-deployment", - }, - { - Kind: "Deployment", - Name: "nginx-deployment2", - }, - }, - }, - }, - errString: "expected one owner reference of the nginx ReplicaSet", - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - t.Parallel() - g := NewWithT(t) - - fakeClient, err := createFakeK8sClient(test.podList, test.replicaSet) - g.Expect(err).ToNot(HaveOccurred()) - - cs := newCommandService( - logr.Discard(), - fakeClient, - NewDeploymentStore(nil), - nil, - status.NewQueue(), - nil, - ) - - owner, _, err := cs.getPodOwner(test.podName) - - if test.errString != "" { - g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).To(ContainSubstring(test.errString)) - g.Expect(owner).To(Equal(types.NamespacedName{})) - return - } - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(owner).To(Equal(test.expected)) - }) - } -} - func TestUpdateDataPlaneStatus(t *testing.T) { t.Parallel() diff --git a/internal/controller/nginx/agent/file.go b/internal/controller/nginx/agent/file.go index 035c96b77b..6f1e05a4d1 100644 --- a/internal/controller/nginx/agent/file.go +++ b/internal/controller/nginx/agent/file.go @@ -56,7 +56,7 @@ func (fs *fileService) GetFile( ctx context.Context, req *pb.GetFileRequest, ) (*pb.GetFileResponse, error) { - gi, ok := grpcContext.FromContext(ctx) + grpcInfo, ok := grpcContext.FromContext(ctx) if !ok { return nil, agentgrpc.ErrStatusInvalidConnection } @@ -65,7 +65,7 @@ func (fs *fileService) GetFile( return nil, status.Error(codes.InvalidArgument, "invalid request") } - contents, err := fs.getFileContents(req, gi.UUID) + contents, err := fs.getFileContents(req, grpcInfo.UUID) if err != nil { return nil, err } @@ -84,7 +84,7 @@ func (fs *fileService) GetFileStream( req *pb.GetFileRequest, server grpc.ServerStreamingServer[pb.FileDataChunk], ) error { - gi, ok := grpcContext.FromContext(server.Context()) + grpcInfo, ok := grpcContext.FromContext(server.Context()) if !ok { return agentgrpc.ErrStatusInvalidConnection } @@ -93,7 +93,7 @@ func (fs *fileService) GetFileStream( return status.Error(codes.InvalidArgument, "invalid request") } - contents, err := fs.getFileContents(req, gi.UUID) + contents, err := fs.getFileContents(req, grpcInfo.UUID) if err != nil { return err } @@ -133,11 +133,11 @@ func (fs *fileService) GetFileStream( func (fs *fileService) getFileContents(req *pb.GetFileRequest, connKey string) ([]byte, error) { conn := fs.connTracker.GetConnection(connKey) - if conn.PodName == "" { + if conn.InstanceID == "" { return nil, status.Errorf(codes.NotFound, "connection not found") } - deployment := fs.nginxDeployments.Get(conn.Parent) + deployment := fs.nginxDeployments.Get(conn.ParentName) if deployment == nil { return nil, status.Errorf(codes.NotFound, "deployment not found in store") } @@ -187,17 +187,17 @@ func (fs *fileService) UpdateOverview( ctx context.Context, req *pb.UpdateOverviewRequest, ) (*pb.UpdateOverviewResponse, error) { - gi, ok := grpcContext.FromContext(ctx) + grpcInfo, ok := grpcContext.FromContext(ctx) if !ok { return &pb.UpdateOverviewResponse{}, agentgrpc.ErrStatusInvalidConnection } - conn := fs.connTracker.GetConnection(gi.UUID) - if conn.PodName == "" { + conn := fs.connTracker.GetConnection(grpcInfo.UUID) + if conn.InstanceID == "" { return &pb.UpdateOverviewResponse{}, status.Errorf(codes.NotFound, "connection not found") } - deployment := fs.nginxDeployments.Get(conn.Parent) + deployment := fs.nginxDeployments.Get(conn.ParentName) if deployment == nil { return &pb.UpdateOverviewResponse{}, status.Errorf(codes.NotFound, "deployment not found in store") } diff --git a/internal/controller/nginx/agent/file_test.go b/internal/controller/nginx/agent/file_test.go index 5e0c52d5bb..d56518ca55 100644 --- a/internal/controller/nginx/agent/file_test.go +++ b/internal/controller/nginx/agent/file_test.go @@ -43,9 +43,8 @@ func TestGetFile(t *testing.T) { connTracker := &agentgrpcfakes.FakeConnectionsTracker{} conn := agentgrpc.Connection{ - PodName: "nginx-pod", InstanceID: "12345", - Parent: deploymentName, + ParentName: deploymentName, } connTracker.GetConnectionReturns(conn) @@ -109,9 +108,8 @@ func TestGetFile_InvalidRequest(t *testing.T) { deploymentName := types.NamespacedName{Name: "nginx-deployment", Namespace: "default"} connTracker := &agentgrpcfakes.FakeConnectionsTracker{} conn := agentgrpc.Connection{ - PodName: "nginx-pod", InstanceID: "12345", - Parent: deploymentName, + ParentName: deploymentName, } connTracker.GetConnectionReturns(conn) @@ -165,9 +163,8 @@ func TestGetFile_DeploymentNotFound(t *testing.T) { connTracker := &agentgrpcfakes.FakeConnectionsTracker{} conn := agentgrpc.Connection{ - PodName: "nginx-pod", InstanceID: "12345", - Parent: deploymentName, + ParentName: deploymentName, } connTracker.GetConnectionReturns(conn) @@ -198,9 +195,8 @@ func TestGetFile_FileNotFound(t *testing.T) { connTracker := &agentgrpcfakes.FakeConnectionsTracker{} conn := agentgrpc.Connection{ - PodName: "nginx-pod", InstanceID: "12345", - Parent: deploymentName, + ParentName: deploymentName, } connTracker.GetConnectionReturns(conn) @@ -234,9 +230,8 @@ func TestGetFileStream(t *testing.T) { connTracker := &agentgrpcfakes.FakeConnectionsTracker{} conn := agentgrpc.Connection{ - PodName: "nginx-pod", InstanceID: "12345", - Parent: deploymentName, + ParentName: deploymentName, } connTracker.GetConnectionReturns(conn) @@ -312,9 +307,8 @@ func TestGetFileStream_InvalidRequest(t *testing.T) { deploymentName := types.NamespacedName{Name: "nginx-deployment", Namespace: "default"} connTracker := &agentgrpcfakes.FakeConnectionsTracker{} conn := agentgrpc.Connection{ - PodName: "nginx-pod", InstanceID: "12345", - Parent: deploymentName, + ParentName: deploymentName, } connTracker.GetConnectionReturns(conn) @@ -369,9 +363,8 @@ func TestUpdateOverview(t *testing.T) { connTracker := &agentgrpcfakes.FakeConnectionsTracker{} conn := agentgrpc.Connection{ - PodName: "nginx-pod", InstanceID: "12345", - Parent: deploymentName, + ParentName: deploymentName, } connTracker.GetConnectionReturns(conn) @@ -504,9 +497,8 @@ func TestUpdateOverview_DeploymentNotFound(t *testing.T) { connTracker := &agentgrpcfakes.FakeConnectionsTracker{} conn := agentgrpc.Connection{ - PodName: "nginx-pod", InstanceID: "12345", - Parent: deploymentName, + ParentName: deploymentName, } connTracker.GetConnectionReturns(conn) diff --git a/internal/controller/nginx/agent/grpc/connections.go b/internal/controller/nginx/agent/grpc/connections.go index 0bae634ccc..3ae38924d6 100644 --- a/internal/controller/nginx/agent/grpc/connections.go +++ b/internal/controller/nginx/agent/grpc/connections.go @@ -21,9 +21,9 @@ type ConnectionsTracker interface { // Connection contains the data about a single nginx agent connection. type Connection struct { - PodName string InstanceID string - Parent types.NamespacedName + ParentType string + ParentName types.NamespacedName } // Ready returns if the connection is ready to be used. In other words, agent diff --git a/internal/controller/nginx/agent/grpc/connections_test.go b/internal/controller/nginx/agent/grpc/connections_test.go index 24280d375f..06e3a47df0 100644 --- a/internal/controller/nginx/agent/grpc/connections_test.go +++ b/internal/controller/nginx/agent/grpc/connections_test.go @@ -16,9 +16,8 @@ func TestGetConnection(t *testing.T) { tracker := agentgrpc.NewConnectionsTracker() conn := agentgrpc.Connection{ - PodName: "pod1", InstanceID: "instance1", - Parent: types.NamespacedName{Namespace: "default", Name: "parent1"}, + ParentName: types.NamespacedName{Namespace: "default", Name: "parent1"}, } tracker.Track("key1", conn) @@ -34,9 +33,8 @@ func TestConnectionIsReady(t *testing.T) { g := NewWithT(t) conn := agentgrpc.Connection{ - PodName: "pod1", InstanceID: "instance1", - Parent: types.NamespacedName{Namespace: "default", Name: "parent1"}, + ParentName: types.NamespacedName{Namespace: "default", Name: "parent1"}, } g.Expect(conn.Ready()).To(BeTrue()) @@ -47,8 +45,7 @@ func TestConnectionIsNotReady(t *testing.T) { g := NewWithT(t) conn := agentgrpc.Connection{ - PodName: "pod1", - Parent: types.NamespacedName{Namespace: "default", Name: "parent1"}, + ParentName: types.NamespacedName{Namespace: "default", Name: "parent1"}, } g.Expect(conn.Ready()).To(BeFalse()) @@ -60,8 +57,7 @@ func TestSetInstanceID(t *testing.T) { tracker := agentgrpc.NewConnectionsTracker() conn := agentgrpc.Connection{ - PodName: "pod1", - Parent: types.NamespacedName{Namespace: "default", Name: "parent1"}, + ParentName: types.NamespacedName{Namespace: "default", Name: "parent1"}, } tracker.Track("key1", conn) @@ -81,9 +77,8 @@ func TestRemoveConnection(t *testing.T) { tracker := agentgrpc.NewConnectionsTracker() conn := agentgrpc.Connection{ - PodName: "pod1", InstanceID: "instance1", - Parent: types.NamespacedName{Namespace: "default", Name: "parent1"}, + ParentName: types.NamespacedName{Namespace: "default", Name: "parent1"}, } tracker.Track("key1", conn) diff --git a/internal/controller/nginx/agent/grpc/interceptor/interceptor.go b/internal/controller/nginx/agent/grpc/interceptor/interceptor.go index b9e6a485f7..b5b8a6c922 100644 --- a/internal/controller/nginx/agent/grpc/interceptor/interceptor.go +++ b/internal/controller/nginx/agent/grpc/interceptor/interceptor.go @@ -84,12 +84,12 @@ func (c ContextSetter) Unary(logger logr.Logger) grpc.UnaryServerInterceptor { // validateConnection checks that the connection is valid and returns a new // context containing information used by the gRPC command/file services. func (c ContextSetter) validateConnection(ctx context.Context) (context.Context, error) { - gi, err := getGrpcInfo(ctx) + grpcInfo, err := getGrpcInfo(ctx) if err != nil { return nil, err } - return c.validateToken(ctx, gi) + return c.validateToken(ctx, grpcInfo) } func getGrpcInfo(ctx context.Context) (*grpcContext.GrpcInfo, error) { @@ -114,11 +114,11 @@ func getGrpcInfo(ctx context.Context) (*grpcContext.GrpcInfo, error) { }, nil } -func (c ContextSetter) validateToken(ctx context.Context, gi *grpcContext.GrpcInfo) (context.Context, error) { +func (c ContextSetter) validateToken(ctx context.Context, grpcInfo *grpcContext.GrpcInfo) (context.Context, error) { tokenReview := &authv1.TokenReview{ Spec: authv1.TokenReviewSpec{ Audiences: []string{c.audience}, - Token: gi.Token, + Token: grpcInfo.Token, }, } @@ -169,5 +169,5 @@ func (c ContextSetter) validateToken(ctx context.Context, gi *grpcContext.GrpcIn return nil, status.Error(codes.Unauthenticated, msg) } - return grpcContext.NewGrpcContext(ctx, *gi), nil + return grpcContext.NewGrpcContext(ctx, *grpcInfo), nil } diff --git a/internal/controller/nginx/agent/grpc/interceptor/interceptor_test.go b/internal/controller/nginx/agent/grpc/interceptor/interceptor_test.go index 68f3392a5b..5987da9194 100644 --- a/internal/controller/nginx/agent/grpc/interceptor/interceptor_test.go +++ b/internal/controller/nginx/agent/grpc/interceptor/interceptor_test.go @@ -253,7 +253,7 @@ func TestValidateToken_PodListOptions(t *testing.T) { testCases := []struct { pod *corev1.Pod - gi *grpcContext.GrpcInfo + grpcInfo *grpcContext.GrpcInfo name string shouldErr bool }{ @@ -269,7 +269,7 @@ func TestValidateToken_PodListOptions(t *testing.T) { }, Status: corev1.PodStatus{Phase: corev1.PodRunning}, }, - gi: &grpcContext.GrpcInfo{Token: "dummy-token"}, + grpcInfo: &grpcContext.GrpcInfo{Token: "dummy-token"}, shouldErr: false, }, { @@ -284,7 +284,7 @@ func TestValidateToken_PodListOptions(t *testing.T) { }, Status: corev1.PodStatus{Phase: corev1.PodRunning}, }, - gi: &grpcContext.GrpcInfo{Token: "dummy-token"}, + grpcInfo: &grpcContext.GrpcInfo{Token: "dummy-token"}, shouldErr: true, }, { @@ -299,7 +299,7 @@ func TestValidateToken_PodListOptions(t *testing.T) { }, Status: corev1.PodStatus{Phase: corev1.PodRunning}, }, - gi: &grpcContext.GrpcInfo{Token: "dummy-token"}, + grpcInfo: &grpcContext.GrpcInfo{Token: "dummy-token"}, shouldErr: true, }, { @@ -312,7 +312,7 @@ func TestValidateToken_PodListOptions(t *testing.T) { }, Status: corev1.PodStatus{Phase: corev1.PodRunning}, }, - gi: &grpcContext.GrpcInfo{Token: "dummy-token"}, + grpcInfo: &grpcContext.GrpcInfo{Token: "dummy-token"}, shouldErr: true, }, { @@ -327,7 +327,7 @@ func TestValidateToken_PodListOptions(t *testing.T) { }, Status: corev1.PodStatus{Phase: corev1.PodPending}, }, - gi: &grpcContext.GrpcInfo{Token: "dummy-token"}, + grpcInfo: &grpcContext.GrpcInfo{Token: "dummy-token"}, shouldErr: true, }, } @@ -352,7 +352,7 @@ func TestValidateToken_PodListOptions(t *testing.T) { patchedClient := &patchClient{fakeClient} csPatched := NewContextSetter(patchedClient, "ngf-audience") - resultCtx, err := csPatched.validateToken(t.Context(), tc.gi) + resultCtx, err := csPatched.validateToken(t.Context(), tc.grpcInfo) if tc.shouldErr { g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring("no running pods")) diff --git a/internal/controller/nginx/types/types.go b/internal/controller/nginx/types/types.go index 6883a4314c..c164e94baa 100644 --- a/internal/controller/nginx/types/types.go +++ b/internal/controller/nginx/types/types.go @@ -4,3 +4,14 @@ const ( // Nginx503Server is used as a backend for services that cannot be resolved (have no IP address). Nginx503Server = "unix:/var/run/nginx/nginx-503-server.sock" ) + +const ( + // AgentOwnerNameLabel is the label key used to store the owner name of the nginx agent. + AgentOwnerNameLabel = "owner-name" + // AgentOwnerTypeLabel is the label key used to store the owner type of the nginx agent. + AgentOwnerTypeLabel = "owner-type" + // DaemonSetType is the value used to represent a DaemonSet owner type. + DaemonSetType = "DaemonSet" + // DeploymentType is the value used to represent a Deployment owner type. + DeploymentType = "Deployment" +) diff --git a/internal/controller/provisioner/objects.go b/internal/controller/provisioner/objects.go index cb4cdb6647..168097b634 100644 --- a/internal/controller/provisioner/objects.go +++ b/internal/controller/provisioner/objects.go @@ -24,6 +24,7 @@ import ( ngfAPIv1alpha2 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha2" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/config" + nginxTypes "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/types" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/graph" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/controller" @@ -451,6 +452,14 @@ func (p *NginxProvisioner) buildNginxConfigMaps( metricsPort = *port } + depType := nginxTypes.DeploymentType + if nProxyCfg != nil && nProxyCfg.Kubernetes != nil && nProxyCfg.Kubernetes.DaemonSet != nil { + depType = nginxTypes.DaemonSetType + } + + p.cfg.AgentLabels[nginxTypes.AgentOwnerNameLabel] = fmt.Sprintf("%s_%s", objectMeta.Namespace, objectMeta.Name) + p.cfg.AgentLabels[nginxTypes.AgentOwnerTypeLabel] = depType + agentFields := map[string]interface{}{ "Plus": p.cfg.Plus, "ServiceName": p.cfg.GatewayPodConfig.ServiceName, @@ -865,10 +874,8 @@ func (p *NginxProvisioner) buildNginxPodTemplateSpec( tokenAudience := fmt.Sprintf("%s.%s.svc", p.cfg.GatewayPodConfig.ServiceName, p.cfg.GatewayPodConfig.Namespace) clusterID := "unknown" - if p.cfg.AgentLabels != nil { - if val, ok := p.cfg.AgentLabels["cluster-id"]; ok { - clusterID = val - } + if val, ok := p.cfg.AgentLabels["cluster-id"]; ok { + clusterID = val } spec := corev1.PodTemplateSpec{ diff --git a/internal/controller/provisioner/objects_test.go b/internal/controller/provisioner/objects_test.go index e9aaa51fef..204b210d21 100644 --- a/internal/controller/provisioner/objects_test.go +++ b/internal/controller/provisioner/objects_test.go @@ -47,6 +47,7 @@ func TestBuildNginxResourceObjects(t *testing.T) { Image: "ngf-image", }, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, baseLabelSelector: metav1.LabelSelector{ MatchLabels: map[string]string{ @@ -253,6 +254,7 @@ func TestBuildNginxResourceObjects_NginxProxyConfig(t *testing.T) { Version: "1.0.0", }, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, baseLabelSelector: metav1.LabelSelector{ MatchLabels: map[string]string{ @@ -494,6 +496,7 @@ func TestBuildNginxResourceObjects_DeploymentReplicasFromHPA(t *testing.T) { Image: "ngf-image", }, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, baseLabelSelector: metav1.LabelSelector{ MatchLabels: map[string]string{"app": "nginx"}, @@ -594,6 +597,7 @@ func TestBuildNginxResourceObjects_Plus(t *testing.T) { SkipVerify: true, }, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, k8sClient: fakeClient, baseLabelSelector: metav1.LabelSelector{ @@ -747,6 +751,7 @@ func TestBuildNginxResourceObjects_DockerSecrets(t *testing.T) { }, NginxDockerSecretNames: []string{dockerTestSecretName, dockerSecretRegistry1Name, dockerSecretRegistry2Name}, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, k8sClient: fakeClient, baseLabelSelector: metav1.LabelSelector{ @@ -838,6 +843,7 @@ func TestBuildNginxResourceObjects_DaemonSet(t *testing.T) { Namespace: ngfNamespace, }, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, k8sClient: fakeClient, baseLabelSelector: metav1.LabelSelector{ @@ -924,6 +930,7 @@ func TestBuildNginxResourceObjects_OpenShift(t *testing.T) { Namespace: ngfNamespace, }, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, k8sClient: fakeClient, baseLabelSelector: metav1.LabelSelector{ @@ -997,6 +1004,7 @@ func TestBuildNginxResourceObjects_DataplaneKeySecret(t *testing.T) { EndpointPort: 443, EndpointTLSSkipVerify: false, }, + AgentLabels: make(map[string]string), }, k8sClient: fakeClient, baseLabelSelector: metav1.LabelSelector{ @@ -1353,6 +1361,7 @@ func TestBuildNginxConfigMaps_WorkerConnections(t *testing.T) { Namespace: "default", ServiceName: "test-service", }, + AgentLabels: make(map[string]string), }, } objectMeta := metav1.ObjectMeta{Name: "test", Namespace: "default"} @@ -1422,6 +1431,8 @@ func TestBuildNginxConfigMaps_AgentFields(t *testing.T) { g.Expect(data).To(ContainSubstring("key1: val1")) g.Expect(data).To(ContainSubstring("key2: val2")) + g.Expect(data).To(ContainSubstring("owner-name: default_test")) + g.Expect(data).To(ContainSubstring("owner-type: Deployment")) g.Expect(data).To(ContainSubstring("host: console.example.com")) g.Expect(data).To(ContainSubstring("port: 443")) g.Expect(data).To(ContainSubstring("skip_verify: false")) @@ -1542,6 +1553,7 @@ func TestBuildNginxResourceObjects_Patches(t *testing.T) { Image: "ngf-image", }, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, baseLabelSelector: metav1.LabelSelector{ MatchLabels: map[string]string{ @@ -1866,6 +1878,7 @@ func TestBuildNginxResourceObjects_InferenceExtension(t *testing.T) { InferenceExtension: true, EndpointPickerDisableTLS: true, EndpointPickerTLSSkipVerify: true, + AgentLabels: make(map[string]string), }, k8sClient: fakeClient, baseLabelSelector: metav1.LabelSelector{ diff --git a/internal/controller/provisioner/provisioner.go b/internal/controller/provisioner/provisioner.go index f7688e9ec1..43d9f5c513 100644 --- a/internal/controller/provisioner/provisioner.go +++ b/internal/controller/provisioner/provisioner.go @@ -146,6 +146,9 @@ func NewNginxProvisioner( cfg.Logger.Error(err, "failed to collect agent labels") } cfg.AgentLabels = agentLabels + if cfg.AgentLabels == nil { + cfg.AgentLabels = make(map[string]string) + } provisioner := &NginxProvisioner{ k8sClient: mgr.GetClient(),