[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5328: Delivery Service/server assignment safeties

[GitBox <gi...@apache.org>]

ocket8888 commented on a change in pull request #5328:
URL: https://github.com/apache/trafficcontrol/pull/5328#discussion_r535726946



##########
File path: traffic_ops/testing/api/v3/deliveryserviceservers_test.go
##########
@@ -17,21 +17,339 @@ package v3
 
 import (
 	"errors"
+	"fmt"
 	"net/http"
 	"net/url"
 	"strconv"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/apache/trafficcontrol/lib/go-tc"
 	"github.com/apache/trafficcontrol/lib/go-util"
 )
 
+func TryToRemoveLastServerInDeliveryService(t *testing.T) {
+	cdns, _, err := TOSession.GetCDNsWithHdr(nil)
+	if err != nil {
+		t.Fatalf("Could not fetch CDNs: %v", err)
+	}
+	if len(cdns) < 1 {
+		t.Fatal("Need at least one CDN to test removing the last server from a Delivery Service")
+	}
+
+	cdnID := cdns[0].ID
+
+	user, _, err := TOSession.GetUserCurrentWithHdr(nil)
+	if err != nil {
+		t.Fatalf("Could not fetch current user: %v", err)
+	}
+	if user == nil {
+		t.Fatalf("Current user is null")
+	}
+	if user.TenantID == nil {
+		t.Fatalf("Current user has null Tenant ID")
+	}
+
+	types, _, err := TOSession.GetTypesWithHdr(nil, "deliveryservice")
+	if err != nil {
+		t.Fatalf("Could not fetch Types: %v", err)
+	}
+	if len(types) < 1 {
+		t.Fatal("Need at least one Type with 'useInTable' of 'deliveryservice' to create a Delivery Service")
+	}
+	dsTypeID := types[0].ID
+
+	xmlid := fmt.Sprintf("test-%d", time.Now().Unix())
+	ds := tc.DeliveryServiceNullableV30{
+		DeliveryServiceNullableV15: tc.DeliveryServiceNullableV15{
+			DeliveryServiceNullableV14: tc.DeliveryServiceNullableV14{
+				DeliveryServiceNullableV13: tc.DeliveryServiceNullableV13{
+					DeliveryServiceNullableV12: tc.DeliveryServiceNullableV12{
+						DeliveryServiceNullableV11: tc.DeliveryServiceNullableV11{
+							Active:                   new(bool),
+							AnonymousBlockingEnabled: new(bool),
+							CDNID:                    &cdnID,
+							DisplayName:              util.StrPtr("test"),
+							DSCP:                     new(int),
+							GeoLimit:                 new(int),
+							GeoProvider:              new(int),
+							InitialDispersion:        new(int),
+							IPV6RoutingEnabled:       new(bool),
+							LogsEnabled:              new(bool),
+							MissLat:                  new(float64),
+							MissLong:                 new(float64),
+							OrgServerFQDN:            util.StrPtr("https://example.com"),
+							Protocol:                 new(int),
+							QStringIgnore:            new(int),
+							RangeRequestHandling:     new(int),
+							RegionalGeoBlocking:      new(bool),
+							Signed:                   false,
+							TenantID:                 user.TenantID,
+							TypeID:                   &dsTypeID,
+							XMLID:                    &xmlid,
+						},
+					},
+				},
+			},
+		},
+	}
+	*ds.InitialDispersion = 1
+	*ds.Active = true
+	ds, _, err = TOSession.CreateDeliveryServiceV30(ds)
+	if err != nil {
+		t.Fatalf("Failed to create Delivery Service: %v", err)
+	}
+	if ds.ID == nil {
+		t.Fatal("Delivery Service had null/undefined ID after creation")
+	}
+
+	cacheGroups, _, err := TOSession.GetCacheGroupsNullableWithHdr(nil)
+	if err != nil {
+		t.Fatalf("Could not fetch Cache Groups: %v", err)
+	}
+	if len(cacheGroups) < 1 {
+		t.Fatal("Need at least one Cache Group for testing")
+	}
+	if cacheGroups[0].ID == nil {
+		t.Fatal("Got a Cache Group for testing with null or undefined ID")
+	}
+	cacheGroupID := *cacheGroups[0].ID
+
+	profile := tc.Profile{
+		CDNID:       cdnID,
+		Description: "test",
+		Name:        xmlid,
+		Type:        "ATS_PROFILE",
+	}
+	alerts, _, err := TOSession.CreateProfile(profile)
+	t.Logf("Alerts from creating Profile: %s", strings.Join(alerts.ToStrings(), ", "))
+	if err != nil {
+		t.Fatalf("Failed to create new Profile in CDN #%d: %v", cdnID, err)
+	}
+	profiles, _, err := TOSession.GetProfileByNameWithHdr(profile.Name, nil)
+	if err != nil {
+		t.Fatalf("Could not fetch Profile '%s' after creation: %v", profile.Name, err)
+	}
+	if len(profiles) != 1 {
+		t.Fatalf("Expected exactly one Profile named '%s'; got: %d", profile.Name, len(profiles))
+	}
+	profile = profiles[0]
+
+	physLocs, _, err := TOSession.GetPhysLocationsWithHdr(nil, nil)
+	if err != nil {
+		t.Fatalf("Could not fetch Physical Locations: %v", err)
+	}
+	if len(physLocs) < 1 {
+		t.Fatal("Need at least one Physical Location")
+	}
+	physLoc := physLocs[0]
+
+	statuses, _, err := TOSession.GetStatusesWithHdr(nil)
+	if err != nil {
+		t.Fatalf("Could not fetch Statuses: %v", err)
+	}
+	if len(statuses) < 1 {
+		t.Fatal("Need at least one Status")
+	}
+
+	var statusID int
+	var badStatusID int
+	found := false
+	foundBad := false
+	for _, status := range statuses {
+		if status.Name == "ONLINE" || status.Name == "REPORTED" {
+			statusID = status.ID
+			found = true
+			if foundBad {
+				break
+			}
+		} else {
+			badStatusID = status.ID
+			foundBad = true
+			if found {
+				break
+			}
+		}
+	}
+	if !found || !foundBad {
+		t.Fatal("Need at least one status with the name 'ONLINE' or 'REPORTED' and at least one status with neither of those names")
+	}
+
+	types, _, err = TOSession.GetTypesWithHdr(nil, "server")
+	if err != nil {
+		t.Fatalf("Could not fetch Types: %v", err)
+	}
+	if len(types) < 1 {
+		t.Fatal("Need at least one Type with 'useInTable' of 'server' to create a server")
+	}
+	serverTypeID := types[0].ID
+
+	server := tc.ServerV30{
+		CommonServerProperties: tc.CommonServerProperties{
+			CachegroupID:   &cacheGroupID,
+			CDNID:          &cdnID,
+			DomainName:     &xmlid,
+			HostName:       &xmlid,
+			HTTPSPort:      util.IntPtr(443),
+			PhysLocationID: &physLoc.ID,
+			ProfileID:      &profile.ID,
+			RevalPending:   new(bool),
+			StatusID:       &statusID,
+			TCPPort:        util.IntPtr(80),
+			TypeID:         &serverTypeID,
+			UpdPending:     new(bool),
+		},
+		Interfaces: []tc.ServerInterfaceInfo{
+			{
+				Name: "eth0",
+				IPAddresses: []tc.ServerIPAddress{
+					{
+						Address:        "198.51.100.1",
+						ServiceAddress: true,
+					},
+				},
+			},
+		},
+	}
+	alerts, _, err = TOSession.CreateServerWithHdr(server, nil)
+	if err != nil {
+		t.Fatalf("Failed to create server: %v - alerts: %s", err, strings.Join(alerts.ToStrings(), ", "))
+	}
+	params := url.Values{}
+	params.Set("hostName", *server.HostName)
+	servers, _, err := TOSession.GetServersWithHdr(&params, nil)
+	if err != nil {
+		t.Fatalf("Could not fetch server after creation: %v", err)
+	}
+	if len(servers.Response) != 1 {
+		t.Fatalf("Expected exactly 1 server with hostname '%s'; got: %d", *server.HostName, len(servers.Response))
+	}
+	server = servers.Response[0]
+	if server.ID == nil {
+		t.Fatal("Server had null/undefined ID after creation")
+	}

Review comment:
       Because then I have to depend on the structure of `tc-fixtures` and changes therein could break the test. Like how `AssignTestDeliveryService` takes the first server and first DS in the fixture data and assigns them to one another, then if I put mine in as the first ds and server then I have to go change _that_ test to use some other hard-coded data. And If I put my ds and server last in the fixture data, then any time someone adds or remove a DS or server from the fixture data this test breaks.
   
   This is very annoying, but it just seemed way less brittle than hard-coding a position in the fixture data.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org