From d5ba24767f60bac7192739231cae0208f15b1c23 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Thu, 24 Mar 2022 11:43:28 +0100 Subject: [PATCH] reflect code review --- pkg/cluster/cluster_test.go | 4 +--- pkg/cluster/k8sres.go | 12 ++++-------- pkg/cluster/k8sres_test.go | 7 ++++++- pkg/cluster/util.go | 9 ++++++--- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 924ed7e26..401b1bc94 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -28,9 +28,7 @@ const ( ) var logger = logrus.New().WithField("test", "cluster") - -// bufferSize might have to be increased when unit test cover functions emitting events -var eventRecorder = record.NewFakeRecorder(5) +var eventRecorder = record.NewFakeRecorder(1) var cl = New( Config{ diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 873553f5d..e545be7ef 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -228,19 +228,15 @@ func (c *Cluster) generateResourceRequirements( defaultResources acidv1.Resources, containerName string) (*v1.ResourceRequirements, error) { var err error + specRequests := acidv1.ResourceDescription{} + specLimits := acidv1.ResourceDescription{} + result := v1.ResourceRequirements{} - var specRequests, specLimits acidv1.ResourceDescription - - if resources == nil { - specRequests = acidv1.ResourceDescription{} - specLimits = acidv1.ResourceDescription{} - } else { + if resources != nil { specRequests = resources.ResourceRequests specLimits = resources.ResourceLimits } - result := v1.ResourceRequirements{} - result.Requests, err = fillResourceList(specRequests, defaultResources.ResourceRequests) if err != nil { return nil, fmt.Errorf("could not fill resource requests: %v", err) diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index 510368158..f27f9b13b 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/fake" v1core "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/client-go/tools/record" ) func newFakeK8sTestClient() (k8sutil.KubernetesClient, *fake.Clientset) { @@ -1676,6 +1677,10 @@ func TestGenerateResourceRequirements(t *testing.T) { roleLabel := "spilo-role" sidecarName := "postgres-exporter" + // two test cases will call enforceMinResourceLimits which emits 2 events per call + // hence bufferSize of 4 is required + newEventRecorder := record.NewFakeRecorder(4) + configResources := config.Resources{ ClusterLabels: map[string]string{"application": "spilo"}, ClusterNameLabel: clusterNameLabel, @@ -1954,7 +1959,7 @@ func TestGenerateResourceRequirements(t *testing.T) { var cluster = New( Config{ OpConfig: tt.config, - }, client, tt.pgSpec, logger, eventRecorder) + }, client, tt.pgSpec, logger, newEventRecorder) cluster.Name = clusterName cluster.Namespace = namespace diff --git a/pkg/cluster/util.go b/pkg/cluster/util.go index f3318a369..5f9a0b379 100644 --- a/pkg/cluster/util.go +++ b/pkg/cluster/util.go @@ -597,9 +597,12 @@ func trimCronjobName(name string) string { func parseResourceRequirements(resourcesRequirement v1.ResourceRequirements) (acidv1.Resources, error) { var resources acidv1.Resources - resourcesJSON, _ := json.Marshal(resourcesRequirement) - if err := json.Unmarshal(resourcesJSON, &resources); err != nil { - return acidv1.Resources{}, fmt.Errorf("could not convert K8s resources requirements into acidv1.Resources struct") + resourcesJSON, err := json.Marshal(resourcesRequirement) + if err != nil { + return acidv1.Resources{}, fmt.Errorf("could not marshal K8s resources requirements") + } + if err = json.Unmarshal(resourcesJSON, &resources); err != nil { + return acidv1.Resources{}, fmt.Errorf("could not unmarshal K8s resources requirements into acidv1.Resources struct") } return resources, nil }