From f954888f77d4bcc10dfbf6eb8c99af00abc6db2c Mon Sep 17 00:00:00 2001 From: Roberto Jimenez Sanchez Date: Fri, 2 Nov 2018 13:34:14 +0100 Subject: [PATCH 1/3] Allow resources in Revision [knative/serving##2099] --- docs/spec/spec.md | 3 + .../serving/v1alpha1/revision_validation.go | 11 +- .../v1alpha1/revision_validation_test.go | 43 ++++- .../v1alpha1/revision/resources/deploy.go | 26 +++- .../revision/resources/deploy_test.go | 110 ++++++++++++- test/configuration.go | 1 + test/crd.go | 3 +- test/e2e/resources_test.go | 147 ++++++++++++++++++ test/test_images/bloatingcow/README.md | 13 ++ test/test_images/bloatingcow/bloatingcow.go | 60 +++++++ 10 files changed, 402 insertions(+), 15 deletions(-) create mode 100644 test/e2e/resources_test.go create mode 100644 test/test_images/bloatingcow/README.md create mode 100644 test/test_images/bloatingcow/bloatingcow.go diff --git a/docs/spec/spec.md b/docs/spec/spec.md index 85c72d102003..fe4870bab8d2 100644 --- a/docs/spec/spec.md +++ b/docs/spec/spec.md @@ -174,6 +174,7 @@ spec: - ... livenessProbe: ... # Optional readinessProbe: ... # Optional + resources: ... # Optional # +optional concurrency strategy. Defaults to Multi. # Deprecated in favor of ContainerConcurrency. @@ -249,6 +250,7 @@ spec: - ... livenessProbe: ... # Optional readinessProbe: ... # Optional + resources: ... # Optional # Name of the service account the code should run as. serviceAccountName: ... @@ -369,6 +371,7 @@ spec: # One of "runLatest", "release", "pinned" (DEPRECATED), or "manual" - ... livenessProbe: ... # Optional readinessProbe: ... # Optional + resources: ... # Optional containerConcurrency: ... # Optional timeoutSeconds: ... serviceAccountName: ... # Name of the service account the code should run as diff --git a/pkg/apis/serving/v1alpha1/revision_validation.go b/pkg/apis/serving/v1alpha1/revision_validation.go index b0ad154dda76..38be9f051fa3 100644 --- a/pkg/apis/serving/v1alpha1/revision_validation.go +++ b/pkg/apis/serving/v1alpha1/revision_validation.go @@ -23,6 +23,7 @@ import ( "github.com/google/go-cmp/cmp" 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" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/validation" @@ -123,9 +124,6 @@ func validateContainer(container corev1.Container) *apis.FieldError { if container.Name != "" { ignoredFields = append(ignoredFields, "name") } - if !equality.Semantic.DeepEqual(container.Resources, corev1.ResourceRequirements{}) { - ignoredFields = append(ignoredFields, "resources") - } if len(container.Ports) > 0 { ignoredFields = append(ignoredFields, "ports") } @@ -206,12 +204,17 @@ func (current *Revision) CheckImmutableFields(og apis.Immutable) *apis.FieldErro return &apis.FieldError{Message: "The provided original was not a Revision"} } - if diff := cmp.Diff(original.Spec, current.Spec); diff != "" { + quantityComparer := cmp.Comparer(func(x, y resource.Quantity) bool { + return x.Cmp(y) == 0 + }) + + if diff := cmp.Diff(original.Spec, current.Spec, quantityComparer); diff != "" { return &apis.FieldError{ Message: "Immutable fields changed (-old +new)", Paths: []string{"spec"}, Details: diff, } } + return nil } diff --git a/pkg/apis/serving/v1alpha1/revision_validation_test.go b/pkg/apis/serving/v1alpha1/revision_validation_test.go index 351020431b6e..a1788c1e960b 100644 --- a/pkg/apis/serving/v1alpha1/revision_validation_test.go +++ b/pkg/apis/serving/v1alpha1/revision_validation_test.go @@ -63,7 +63,7 @@ func TestContainerValidation(t *testing.T) { }, }, }, - want: apis.ErrDisallowedFields("resources"), + want: nil, }, { name: "has ports", c: corev1.Container{ @@ -137,11 +137,6 @@ func TestContainerValidation(t *testing.T) { name: "has numerous problems", c: corev1.Container{ Name: "foo", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceName("cpu"): resource.MustParse("25m"), - }, - }, Ports: []corev1.ContainerPort{{ Name: "http", ContainerPort: 8080, @@ -152,7 +147,7 @@ func TestContainerValidation(t *testing.T) { }}, Lifecycle: &corev1.Lifecycle{}, }, - want: apis.ErrDisallowedFields("name", "resources", "ports", "volumeMounts", "lifecycle"), + want: apis.ErrDisallowedFields("name", "ports", "volumeMounts", "lifecycle"), }} for _, test := range tests { @@ -599,6 +594,40 @@ func TestImmutableFields(t *testing.T) { }, old: ¬ARevision{}, want: &apis.FieldError{Message: "The provided original was not a Revision"}, + }, { + name: "bad (resources image change)", + new: &Revision{ + Spec: RevisionSpec{ + Container: corev1.Container{ + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceName("cpu"): resource.MustParse("50m"), + }, + }, + }, + ConcurrencyModel: "Multi", + }, + }, + old: &Revision{ + Spec: RevisionSpec{ + Container: corev1.Container{ + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceName("cpu"): resource.MustParse("100m"), + }, + }, + }, + ConcurrencyModel: "Multi", + }, + }, + want: &apis.FieldError{ + Message: "Immutable fields changed (-old +new)", + Paths: []string{"spec"}, + Details: `{v1alpha1.RevisionSpec}.Container.Resources.Requests["cpu"]: + -: resource.Quantity{i: resource.int64Amount{value: 100, scale: resource.Scale(-3)}, s: "100m", Format: resource.Format("DecimalSI")} + +: resource.Quantity{i: resource.int64Amount{value: 50, scale: resource.Scale(-3)}, s: "50m", Format: resource.Format("DecimalSI")} +`, + }, }, { name: "bad (container image change)", new: &Revision{ diff --git a/pkg/reconciler/v1alpha1/revision/resources/deploy.go b/pkg/reconciler/v1alpha1/revision/resources/deploy.go index b1dc760f53e2..43d4d3e696fc 100644 --- a/pkg/reconciler/v1alpha1/revision/resources/deploy.go +++ b/pkg/reconciler/v1alpha1/revision/resources/deploy.go @@ -93,12 +93,36 @@ func rewriteUserProbe(p *corev1.Probe) { } } +// applyDefaultResource +// Implements a deep merge for ResourceRequirements +// note: DeepCopyInto cannot be used because it replaces limits or requests instead of merging them +func applyDefaultResources(defaults corev1.ResourceRequirements, out *corev1.ResourceRequirements) { + in := defaults.DeepCopy() + if in.Limits != nil { + in, out := &in.Limits, &out.Limits + for key, val := range *out { + (*in)[key] = val.DeepCopy() + } + (*out) = (*in) + } + if in.Requests != nil { + in, out := &in.Requests, &out.Requests + for key, val := range *out { + (*in)[key] = val.DeepCopy() + } + (*out) = (*in) + } +} + func makePodSpec(rev *v1alpha1.Revision, loggingConfig *logging.Config, observabilityConfig *config.Observability, autoscalerConfig *autoscaler.Config, controllerConfig *config.Controller) *corev1.PodSpec { userContainer := rev.Spec.Container.DeepCopy() // Adding or removing an overwritten corev1.Container field here? Don't forget to // update the validations in pkg/webhook.validateContainer. userContainer.Name = userContainerName - userContainer.Resources = userResources + + // If client provides for some resources, override default values + applyDefaultResources(userResources, &userContainer.Resources) + userContainer.Ports = userPorts userContainer.VolumeMounts = append(userContainer.VolumeMounts, varLogVolumeMount) userContainer.Lifecycle = userLifecycle diff --git a/pkg/reconciler/v1alpha1/revision/resources/deploy_test.go b/pkg/reconciler/v1alpha1/revision/resources/deploy_test.go index f0fb95b3b389..b31011882331 100644 --- a/pkg/reconciler/v1alpha1/revision/resources/deploy_test.go +++ b/pkg/reconciler/v1alpha1/revision/resources/deploy_test.go @@ -870,6 +870,16 @@ func TestMakePodSpec(t *testing.T) { Name: "BAZ", Value: "blah", }}, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("666Mi"), + corev1.ResourceCPU: resource.MustParse("666m"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("888Mi"), + corev1.ResourceCPU: resource.MustParse("888m"), + }, + }, }, TimeoutSeconds: &metav1.Duration{ Duration: 45 * time.Second, @@ -905,7 +915,16 @@ func TestMakePodSpec(t *testing.T) { Name: "K_SERVICE", Value: "", }}, - Resources: userResources, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("666Mi"), + corev1.ResourceCPU: resource.MustParse("666m"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("888Mi"), + corev1.ResourceCPU: resource.MustParse("888m"), + }, + }, Ports: userPorts, VolumeMounts: []corev1.VolumeMount{varLogVolumeMount}, Lifecycle: userLifecycle, @@ -957,8 +976,12 @@ func TestMakePodSpec(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + quantityComparer := cmp.Comparer(func(x, y resource.Quantity) bool { + return x.Cmp(y) == 0 + }) + got := makePodSpec(test.rev, test.lc, test.oc, test.ac, test.cc) - if diff := cmp.Diff(test.want, got, cmpopts.IgnoreUnexported(resource.Quantity{})); diff != "" { + if diff := cmp.Diff(test.want, got, quantityComparer); diff != "" { t.Errorf("makePodSpec (-want, +got) = %v", diff) } }) @@ -1275,3 +1298,86 @@ func TestMakeDeployment(t *testing.T) { }) } } + +func TestApplyDefaultResources(t *testing.T) { + tests := []struct { + name string + defaults corev1.ResourceRequirements + in *corev1.ResourceRequirements + want *corev1.ResourceRequirements + }{ + { + name: "resources are empty", + defaults: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + "resource": resource.MustParse("100m"), + }, + Limits: corev1.ResourceList{ + "resource": resource.MustParse("100m"), + }, + }, + in: &corev1.ResourceRequirements{}, + want: &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + "resource": resource.MustParse("100m"), + }, + Limits: corev1.ResourceList{ + "resource": resource.MustParse("100m"), + }, + }, + }, + { + name: "requests are not empty", + defaults: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + "same": resource.MustParse("100m"), + "other": resource.MustParse("200m"), + }, + }, + in: &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + "same": resource.MustParse("500m"), + "new": resource.MustParse("300m"), + }, + }, + want: &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + "same": resource.MustParse("500m"), + "new": resource.MustParse("200m"), + "other": resource.MustParse("300m"), + }, + }, + }, + { + name: "limits are not empty", + defaults: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + "same": resource.MustParse("100m"), + "other": resource.MustParse("200m"), + }, + }, + in: &corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + "same": resource.MustParse("500m"), + "new": resource.MustParse("300m"), + }, + }, + want: &corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + "same": resource.MustParse("500m"), + "new": resource.MustParse("200m"), + "other": resource.MustParse("300m"), + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + applyDefaultResources(test.defaults, test.in) + if diff := cmp.Diff(test.want, test.in, cmpopts.IgnoreUnexported(resource.Quantity{})); diff != "" { // Maybe this compare fails + t.Errorf("ApplyDefaultResources (-want, +got) = %v", diff) + } + }) + } +} diff --git a/test/configuration.go b/test/configuration.go index 058b0c8896d2..a9019613327b 100644 --- a/test/configuration.go +++ b/test/configuration.go @@ -30,6 +30,7 @@ type Options struct { EnvVars []corev1.EnvVar ContainerConcurrency int RevisionTimeout time.Duration + ContainerResources corev1.ResourceRequirements } // CreateConfiguration create a configuration resource in namespace with the name names.Config diff --git a/test/crd.go b/test/crd.go index 38f9963c96c2..7f11436c1c4b 100644 --- a/test/crd.go +++ b/test/crd.go @@ -100,7 +100,8 @@ func Configuration(namespace string, names ResourceNames, imagePath string, opti RevisionTemplate: v1alpha1.RevisionTemplateSpec{ Spec: v1alpha1.RevisionSpec{ Container: corev1.Container{ - Image: imagePath, + Image: imagePath, + Resources: options.ContainerResources, }, ContainerConcurrency: v1alpha1.RevisionContainerConcurrencyType(options.ContainerConcurrency), }, diff --git a/test/e2e/resources_test.go b/test/e2e/resources_test.go new file mode 100644 index 000000000000..cf91176eead9 --- /dev/null +++ b/test/e2e/resources_test.go @@ -0,0 +1,147 @@ +// +build e2e + +/* +Copyright 2018 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2e + +import ( + "fmt" + "net/http" + "strings" + "testing" + + pkgTest "github.com/knative/pkg/test" + "github.com/knative/pkg/test/logging" + "github.com/knative/pkg/test/spoof" + "github.com/knative/serving/test" + "k8s.io/api/core/v1" + 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) + + //add test case specific name to its own logger + logger = logging.GetContextLogger("TestCustomResourcesLimits") + + var imagePath = test.ImagePath("bloatingcow") + + logger.Infof("Creating a new Route and Configuration") + resources := corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("350Mi"), + }, + Requests: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("350Mi"), + }, + } + names, err := CreateRouteAndConfig(clients, logger, imagePath, &test.Options{ContainerResources: resources}) + if err != nil { + t.Fatalf("Failed to create Route and Configuration: %v", err) + } + test.CleanupOnInterrupt(func() { TearDown(clients, names, logger) }, logger) + defer TearDown(clients, names, logger) + + 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{}) + if err != nil { + t.Fatalf("Error fetching Route %s: %v", names.Route, err) + } + domain := route.Status.Domain + + logger.Info("Waiting for pod list to contain desired resource configuration") + err = test.WaitForPodListState( + clients.KubeClient, + func(p *v1.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) + } else { + return true, nil + } + } + } + } + } + return false, nil + }, + "WaitForAvailablePods") + 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.") + + pokeCowForMB := func(mb int) error { + response, err := sendPostRequest(test.ServingFlags.ResolvableDomain, domain, fmt.Sprintf("memory_in_mb=%d", mb)) + if err != nil { + return err + } + want := "Moo!" + if want != strings.TrimSpace(string(response.Body)) { + return fmt.Errorf("The response '%s' is not equal to expected response '%s'.", string(response.Body), want) + } + return nil + } + + logger.Info("Querying the application to see if the memory limits are enforced.") + if err := pokeCowForMB(100); err != nil { + t.Fatalf("Didn't get a response from bloating cow with %d MBs of Memory: %v", 100, err) + } + + if err := pokeCowForMB(200); err != nil { + t.Fatalf("Didn't get a response from bloating cow with %d MBs of Memory: %v", 200, err) + } + + if err := pokeCowForMB(500); err == nil { + 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) +} diff --git a/test/test_images/bloatingcow/README.md b/test/test_images/bloatingcow/README.md new file mode 100644 index 000000000000..cf9a008f4159 --- /dev/null +++ b/test/test_images/bloatingcow/README.md @@ -0,0 +1,13 @@ +# Bloating cow test image + +This directory contains the test image used in the resources e2e test. + +The image contains a simple Go webserver, `bloatingcow.go`, that will, by default, listen on port `8080` and expose a service at `/`. + +When called, the server emits a "Moo!" message, if query parameter `memory_in_mb` is specified, the app will allocated so many Mbs of memory. + +## Building + +For details about building and adding new images, see the [section about test +images](/test/README.md#test-images). + diff --git a/test/test_images/bloatingcow/bloatingcow.go b/test/test_images/bloatingcow/bloatingcow.go new file mode 100644 index 000000000000..da41a3148268 --- /dev/null +++ b/test/test_images/bloatingcow/bloatingcow.go @@ -0,0 +1,60 @@ +/* +Copyright 2018 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "flag" + "fmt" + "log" + "net/http" + "strconv" +) + +func bloat(mb int) { + if mb == 0 { + return + } + fmt.Printf("Bloat %v Mb of memory.\n", mb) + + b := make([]byte, mb*1024*1024) + b[0] = 1 + b[len(b)-1] = 1 +} + +func handler(w http.ResponseWriter, r *http.Request) { + memoryInMB := r.URL.Query().Get("memory_in_mb") + if memoryInMB != "" { + if mb, err := strconv.Atoi(memoryInMB); err != nil { + fmt.Fprintf(w, "cannot convert %s to int", memoryInMB) + w.WriteHeader(http.StatusBadRequest) + return + } else { + bloat(mb) + } + } + + log.Print("Moo!") + fmt.Fprintln(w, "Moo!") +} + +func main() { + flag.Parse() + log.Print("Bloating cow app started.") + + http.HandleFunc("/", handler) + http.ListenAndServe(":8080", nil) +} From d7a09370025932fcd1287659ea7a0978e95989e3 Mon Sep 17 00:00:00 2001 From: Matt Moore Date: Fri, 30 Nov 2018 02:51:48 +0000 Subject: [PATCH 2/3] Initialize all of the allocated memory. --- test/e2e/resources_test.go | 14 +++++++++++++- test/test_images/bloatingcow/bloatingcow.go | 5 +++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/test/e2e/resources_test.go b/test/e2e/resources_test.go index cf91176eead9..37ca4aa80b9f 100644 --- a/test/e2e/resources_test.go +++ b/test/e2e/resources_test.go @@ -106,12 +106,24 @@ func TestCustomResourcesLimits(t *testing.T) { logger.Info("pods are running with the desired configuration.") + want := "Moo!" + + _, err = pkgTest.WaitForEndpointState( + clients.KubeClient, + logger, + domain, + pkgTest.Retrying(pkgTest.MatchesBody(want), http.StatusNotFound), + "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) + } + pokeCowForMB := func(mb int) error { response, err := sendPostRequest(test.ServingFlags.ResolvableDomain, domain, fmt.Sprintf("memory_in_mb=%d", mb)) if err != nil { return err } - want := "Moo!" if want != strings.TrimSpace(string(response.Body)) { return fmt.Errorf("The response '%s' is not equal to expected response '%s'.", string(response.Body), want) } diff --git a/test/test_images/bloatingcow/bloatingcow.go b/test/test_images/bloatingcow/bloatingcow.go index da41a3148268..5e06ab02fdd0 100644 --- a/test/test_images/bloatingcow/bloatingcow.go +++ b/test/test_images/bloatingcow/bloatingcow.go @@ -31,8 +31,9 @@ func bloat(mb int) { fmt.Printf("Bloat %v Mb of memory.\n", mb) b := make([]byte, mb*1024*1024) - b[0] = 1 - b[len(b)-1] = 1 + for i := 0; i < len(b); i++ { + b[i] = 1 + } } func handler(w http.ResponseWriter, r *http.Request) { From a419b0077b0d38ca1a9bd4ef448d6302b4add6ac Mon Sep 17 00:00:00 2001 From: Matt Moore Date: Fri, 30 Nov 2018 04:50:03 +0000 Subject: [PATCH 3/3] Fix golint errors. --- test/e2e/resources_test.go | 7 +++---- test/test_images/bloatingcow/bloatingcow.go | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/test/e2e/resources_test.go b/test/e2e/resources_test.go index 37ca4aa80b9f..bf06539c51a4 100644 --- a/test/e2e/resources_test.go +++ b/test/e2e/resources_test.go @@ -89,10 +89,9 @@ func TestCustomResourcesLimits(t *testing.T) { } 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) - } else { - return true, nil + return false, fmt.Errorf("invalid resource configuration for pod %v. Want: %+v, got: %+v", pod.Name, want, c.Resources) } + return true, nil } } } @@ -125,7 +124,7 @@ func TestCustomResourcesLimits(t *testing.T) { return err } if want != strings.TrimSpace(string(response.Body)) { - return fmt.Errorf("The response '%s' is not equal to expected response '%s'.", string(response.Body), want) + return fmt.Errorf("the response %q is not equal to expected response %q", string(response.Body), want) } return nil } diff --git a/test/test_images/bloatingcow/bloatingcow.go b/test/test_images/bloatingcow/bloatingcow.go index 5e06ab02fdd0..7dab201f6317 100644 --- a/test/test_images/bloatingcow/bloatingcow.go +++ b/test/test_images/bloatingcow/bloatingcow.go @@ -39,13 +39,13 @@ func bloat(mb int) { func handler(w http.ResponseWriter, r *http.Request) { memoryInMB := r.URL.Query().Get("memory_in_mb") if memoryInMB != "" { - if mb, err := strconv.Atoi(memoryInMB); err != nil { + mb, err := strconv.Atoi(memoryInMB) + if err != nil { fmt.Fprintf(w, "cannot convert %s to int", memoryInMB) w.WriteHeader(http.StatusBadRequest) return - } else { - bloat(mb) } + bloat(mb) } log.Print("Moo!")