Updated tests for allocation changes

cmd/allocator/main.go

@@ -412,7 +412,11 @@ type serviceHandler struct {
 // Allocate implements the Allocate gRPC method definition
 func (h *serviceHandler) Allocate(ctx context.Context, in *pb.AllocationRequest) (*pb.AllocationResponse, error) {
 	logger.WithField("request", in).Infof("allocation request received.")
-	gsa := converters.ConvertAllocationRequestToGSA(in)
+	gsa, err := converters.ConvertAllocationRequestToGSA(in)
+	if err != nil {
+		logger.WithField("request", in).WithError(err).Info("Failed to convert request to GSA")
+		return nil, err
+	}
 	resultObj, err := h.allocationCallback(gsa)
 	if err != nil {
 		logger.WithField("gsa", gsa).WithError(err).Info("allocation failed")

pkg/allocation/converters/converter.go

@@ -16,6 +16,8 @@
 package converters
 
 import (
+	"errors"
+
 	pb "agones.dev/agones/pkg/allocation/go"
 	"agones.dev/agones/pkg/apis"
 	agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
@@ -26,9 +28,9 @@ import (
 )
 
 // ConvertAllocationRequestToGSA converts AllocationRequest to GameServerAllocation V1 (GSA)
-func ConvertAllocationRequestToGSA(in *pb.AllocationRequest) *allocationv1.GameServerAllocation {
+func ConvertAllocationRequestToGSA(in *pb.AllocationRequest) (*allocationv1.GameServerAllocation, error) {
 	if in == nil {
-		return nil
+		return nil, nil
 	}
 
 	gsa := &allocationv1.GameServerAllocation{
@@ -50,17 +52,26 @@ func ConvertAllocationRequestToGSA(in *pb.AllocationRequest) *allocationv1.GameS
 		}
 	}
 
-	if in.GetMetaPatch() != nil {
+	// Accept both metadata (preferred) and metapatch until metapatch is fully removed.
+	metadata := in.GetMetadata()
+	if metadata == nil {
+		metadata = in.GetMetaPatch()
+	} else if in.GetMetaPatch() != nil {
+		// If both are set, reject the call.
+		return nil, errors.New("Allocation request cannot have both metadata and metapatch fields set")
+	}
+
+	if metadata != nil {
 		gsa.Spec.MetaPatch = allocationv1.MetaPatch{
-			Labels:      in.GetMetaPatch().GetLabels(),
-			Annotations: in.GetMetaPatch().GetAnnotations(),
+			Labels:      metadata.GetLabels(),
+			Annotations: metadata.GetAnnotations(),
 		}
 	}
 
 	if ls := convertLabelSelectorToInternalLabelSelector(in.GetRequiredGameServerSelector()); ls != nil {
 		gsa.Spec.Required = *ls
 	}
-	return gsa
+	return gsa, nil
 }
 
 // ConvertGSAToAllocationRequest converts AllocationRequest to GameServerAllocation V1 (GSA)
@@ -77,7 +88,7 @@ func ConvertGSAToAllocationRequest(in *allocationv1.GameServerAllocation) *pb.Al
 			Enabled: in.Spec.MultiClusterSetting.Enabled,
 		},
 		RequiredGameServerSelector: convertInternalLabelSelectorToLabelSelector(&in.Spec.Required),
-		MetaPatch: &pb.MetaPatch{
+		Metadata: &pb.MetaPatch{
 			Labels:      in.Spec.MetaPatch.Labels,
 			Annotations: in.Spec.MetaPatch.Annotations,
 		},

pkg/allocation/converters/converter_test.go

@@ -15,6 +15,7 @@
 package converters
 
 import (
+	"errors"
 	"testing"
 
 	pb "agones.dev/agones/pkg/allocation/go"
@@ -33,6 +34,7 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 		in                 *pb.AllocationRequest
 		want               *allocationv1.GameServerAllocation
 		skipConvertFromGSA bool
+		wantError          error
 	}{
 		{
 			name: "all fields are set",
@@ -64,7 +66,7 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 					},
 				},
 				Scheduling: pb.AllocationRequest_Packed,
-				MetaPatch: &pb.MetaPatch{
+				Metadata: &pb.MetaPatch{
 					Labels: map[string]string{
 						"i": "j",
 					},
@@ -117,7 +119,7 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 				RequiredGameServerSelector:   &pb.LabelSelector{},
 				PreferredGameServerSelectors: []*pb.LabelSelector{},
 				Scheduling:                   pb.AllocationRequest_Distributed,
-				MetaPatch:                    &pb.MetaPatch{},
+				Metadata:                     &pb.MetaPatch{},
 			},
 			want: &allocationv1.GameServerAllocation{
 				ObjectMeta: metav1.ObjectMeta{
@@ -147,22 +149,74 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) {
 			in:   nil,
 			want: nil,
 		},
+		{
+			name: "accepts deprecated metapatch field",
+			in: &pb.AllocationRequest{
+				Namespace:                    "",
+				MultiClusterSetting:          &pb.MultiClusterSetting{},
+				RequiredGameServerSelector:   &pb.LabelSelector{},
+				PreferredGameServerSelectors: []*pb.LabelSelector{},
+				Scheduling:                   pb.AllocationRequest_Distributed,
+				MetaPatch: &pb.MetaPatch{
+					Labels: map[string]string{
+						"a": "b",
+					},
+				},
+			},
+			want: &allocationv1.GameServerAllocation{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "",
+				},
+				Spec: allocationv1.GameServerAllocationSpec{
+					MultiClusterSetting: allocationv1.MultiClusterSetting{
+						Enabled: false,
+					},
+					Scheduling: apis.Distributed,
+					MetaPatch: allocationv1.MetaPatch{
+						Labels: map[string]string{
+							"a": "b",
+						},
+					},
+				},
+			},
+			skipConvertFromGSA: true,
+		},
+		{
+			name: "Rejects both metadata and metapatch field",
+			in: &pb.AllocationRequest{
+				Namespace:                    "",
+				MultiClusterSetting:          &pb.MultiClusterSetting{},
+				RequiredGameServerSelector:   &pb.LabelSelector{},
+				PreferredGameServerSelectors: []*pb.LabelSelector{},
+				Scheduling:                   pb.AllocationRequest_Distributed,
+				Metadata: &pb.MetaPatch{
+					Labels: map[string]string{
+						"a": "b",
+					},
+				},
+				MetaPatch: &pb.MetaPatch{
+					Labels: map[string]string{
+						"c": "d",
+					},
+				},
+			},
+			want:               nil,
+			skipConvertFromGSA: true,
+			wantError:          errors.New("Allocation request cannot have both metadata and metapatch fields set"),
+		},
 	}
 	for _, tc := range tests {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			out := ConvertAllocationRequestToGSA(tc.in)
-			if !assert.Equal(t, tc.want, out) {
-				t.Errorf("mismatch with want after conversion: \"%s\"", tc.name)
-			}
+			out, err := ConvertAllocationRequestToGSA(tc.in)
+			assert.Equal(t, tc.wantError, err, "\"%s\" error: got %v, want %v", tc.name, err, tc.wantError)
+			assert.Equal(t, tc.want, out, "mismatch with want after conversion: \"%s\"", tc.name)
 
 			if !tc.skipConvertFromGSA {
 				gsa := ConvertGSAToAllocationRequest(tc.want)
-				if !assert.Equal(t, tc.in, gsa) {
-					t.Errorf("mismatch with input after double conversion \"%s\"", tc.name)
-				}
+				assert.Equal(t, tc.in, gsa, "mismatch with input after double conversion \"%s\"", tc.name)
 			}
 		})
 	}
@@ -192,7 +246,7 @@ func TestConvertGSAToAllocationRequestEmpty(t *testing.T) {
 				MultiClusterSetting:        &pb.MultiClusterSetting{},
 				RequiredGameServerSelector: &pb.LabelSelector{},
 				Scheduling:                 pb.AllocationRequest_Distributed,
-				MetaPatch:                  &pb.MetaPatch{},
+				Metadata:                   &pb.MetaPatch{},
 			},
 		}, {
 			name: "empty object",
@@ -204,7 +258,7 @@ func TestConvertGSAToAllocationRequestEmpty(t *testing.T) {
 			want: &pb.AllocationRequest{
 				MultiClusterSetting:        &pb.MultiClusterSetting{},
 				RequiredGameServerSelector: &pb.LabelSelector{},
-				MetaPatch:                  &pb.MetaPatch{},
+				Metadata:                   &pb.MetaPatch{},
 			},
 		},
 	}