From 4aec9170dc769cb5e428b06cf7fb866c9799ef28 Mon Sep 17 00:00:00 2001 From: Matt Moore Date: Sun, 16 Dec 2018 21:49:23 +0000 Subject: [PATCH 1/2] Move the resource test under conformance. This was where I'd originally wanted this, but realized it had somehow ended up under e2e. This also removes the pod checks, which reach around the Knative API surface. --- test/{e2e => conformance}/resources_test.go | 90 +++++++-------------- 1 file changed, 29 insertions(+), 61 deletions(-) rename test/{e2e => conformance}/resources_test.go (57%) diff --git a/test/e2e/resources_test.go b/test/conformance/resources_test.go similarity index 57% rename from test/e2e/resources_test.go rename to test/conformance/resources_test.go index 2ea995be7e32..79d5a483c8eb 100644 --- a/test/e2e/resources_test.go +++ b/test/conformance/resources_test.go @@ -16,7 +16,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package e2e +package conformance import ( "fmt" @@ -29,18 +29,15 @@ import ( "github.com/knative/pkg/test/spoof" "github.com/knative/serving/test" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestCustomResourcesLimits(t *testing.T) { - clients = Setup(t) + clients := setup(t) //add test case specific name to its own logger - logger = logging.GetContextLogger("TestCustomResourcesLimits") - - var imagePath = test.ImagePath("bloatingcow") + logger := logging.GetContextLogger("TestCustomResourcesLimits") logger.Infof("Creating a new Route and Configuration") resources := corev1.ResourceRequirements{ @@ -51,12 +48,19 @@ func TestCustomResourcesLimits(t *testing.T) { corev1.ResourceMemory: resource.MustParse("350Mi"), }, } - names, err := CreateRouteAndConfig(clients, logger, imagePath, &test.Options{ContainerResources: resources}) + + names := test.ResourceNames{ + Service: test.AppendRandomString("test-resource-limits-", logger), + Image: "bloatingcow", + } + + _, err := test.CreateRunLatestServiceReady(logger, clients, &names, &test.Options{ContainerResources: resources}) if err != nil { - t.Fatalf("Failed to create Route and Configuration: %v", err) + t.Fatalf("Failed to create initial Service %v: %v", names.Service, err) } - test.CleanupOnInterrupt(func() { TearDown(clients, names, logger) }, logger) - defer TearDown(clients, names, logger) + + test.CleanupOnInterrupt(func() { tearDown(clients, names) }, logger) + defer tearDown(clients, names) logger.Infof("When the Revision can have traffic routed to it, the Route is marked as Ready.") if err := test.WaitForRouteState(clients.ServingClient, names.Route, test.IsRouteReady, "RouteIsReady"); err != nil { @@ -69,43 +73,7 @@ func TestCustomResourcesLimits(t *testing.T) { } domain := route.Status.Domain - logger.Info("Waiting for pod list to contain desired resource configuration") - err = pkgTest.WaitForPodListState( - clients.KubeClient, - func(p *corev1.PodList) (bool, error) { - for _, pod := range p.Items { - if strings.HasPrefix(pod.Name, names.Config) { - for _, c := range pod.Spec.Containers { - if c.Name == "user-container" { - want := corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceMemory: resource.MustParse("350Mi"), - }, - Requests: corev1.ResourceList{ - corev1.ResourceMemory: resource.MustParse("350Mi"), - corev1.ResourceCPU: resource.MustParse("400m"), // Default value - }, - } - - if !equality.Semantic.DeepEqual(c.Resources, want) { - return false, fmt.Errorf("invalid resource configuration for pod %v. Want: %+v, got: %+v", pod.Name, want, c.Resources) - } - return true, nil - } - } - } - } - return false, nil - }, - "WaitForAvailablePods", test.ServingNamespace) - if err != nil { - logger.Fatalf(`Waiting for Pod.List to have pods with custom resources: %v`, err) - } - - logger.Info("pods are running with the desired configuration.") - want := "Moo!" - _, err = pkgTest.WaitForEndpointState( clients.KubeClient, logger, @@ -114,7 +82,21 @@ func TestCustomResourcesLimits(t *testing.T) { "ResourceTestServesText", test.ServingFlags.ResolvableDomain) if err != nil { - t.Fatalf("The endpoint for Route %s at domain %s didn't serve the expected text \"%s\": %v", names.Route, domain, helloWorldExpectedOutput, err) + t.Fatalf("The endpoint for Route %s at domain %s didn't serve the expected text \"%s\": %v", names.Route, domain, want, err) + } + + sendPostRequest := func(resolvableDomain bool, domain string, query string) (*spoof.Response, error) { + logger.Infof("The domain of request is %s and its query is %s", domain, query) + client, err := pkgTest.NewSpoofingClient(clients.KubeClient, logger, domain, resolvableDomain) + if err != nil { + return nil, err + } + + req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("http://%s?%s", domain, query), nil) + if err != nil { + return nil, err + } + return client.Do(req) } pokeCowForMB := func(mb int) error { @@ -141,17 +123,3 @@ func TestCustomResourcesLimits(t *testing.T) { t.Fatalf("We shouldn't have got a response from bloating cow with %d MBs of Memory: %v", 500, err) } } - -func sendPostRequest(resolvableDomain bool, domain string, query string) (*spoof.Response, error) { - logger.Infof("The domain of request is %s and its query is %s", domain, query) - client, err := pkgTest.NewSpoofingClient(clients.KubeClient, logger, domain, resolvableDomain) - if err != nil { - return nil, err - } - - req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("http://%s?%s", domain, query), nil) - if err != nil { - return nil, err - } - return client.Do(req) -} From ea31c12babf844d3bc67bd5208576b5dda240e35 Mon Sep 17 00:00:00 2001 From: Matt Moore Date: Mon, 17 Dec 2018 21:35:23 +0000 Subject: [PATCH 2/2] Make better use of the test facilities. --- test/conformance/resources_test.go | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/test/conformance/resources_test.go b/test/conformance/resources_test.go index 79d5a483c8eb..feb5b4d57b56 100644 --- a/test/conformance/resources_test.go +++ b/test/conformance/resources_test.go @@ -30,7 +30,6 @@ import ( "github.com/knative/serving/test" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestCustomResourcesLimits(t *testing.T) { @@ -54,24 +53,14 @@ func TestCustomResourcesLimits(t *testing.T) { Image: "bloatingcow", } - _, err := test.CreateRunLatestServiceReady(logger, clients, &names, &test.Options{ContainerResources: resources}) - if err != nil { - t.Fatalf("Failed to create initial Service %v: %v", names.Service, err) - } - test.CleanupOnInterrupt(func() { tearDown(clients, names) }, logger) defer tearDown(clients, names) - logger.Infof("When the Revision can have traffic routed to it, the Route is marked as Ready.") - if err := test.WaitForRouteState(clients.ServingClient, names.Route, test.IsRouteReady, "RouteIsReady"); err != nil { - t.Fatalf("The Route %s was not marked as Ready to serve traffic: %v", names.Route, err) - } - - route, err := clients.ServingClient.Routes.Get(names.Route, metav1.GetOptions{}) + objects, err := test.CreateRunLatestServiceReady(logger, clients, &names, &test.Options{ContainerResources: resources}) if err != nil { - t.Fatalf("Error fetching Route %s: %v", names.Route, err) + t.Fatalf("Failed to create initial Service %v: %v", names.Service, err) } - domain := route.Status.Domain + domain := objects.Route.Status.Domain want := "Moo!" _, err = pkgTest.WaitForEndpointState(