Merge pull request #3581 from jmolmo/issue-3564

Memory limit checked if it really exists

pkg/operator/ceph/spec/spec.go

@@ -256,19 +256,21 @@ func CheckPodMemory(resources v1.ResourceRequirements, cephPodMinimumMemory uint
 		return nil
 	}
 
-	// This means LIMIT and REQUEST are either identical or different but still we use LIMIT as a reference
-	if uint64(podMemoryLimit.Value()) < display.MbTob(cephPodMinimumMemory) {
-		return fmt.Errorf(errorMessage, display.BToMb(uint64(podMemoryLimit.Value())), cephPodMinimumMemory)
-	}
+	if !podMemoryLimit.IsZero() {
+		// This means LIMIT and REQUEST are either identical or different but still we use LIMIT as a reference
+		if uint64(podMemoryLimit.Value()) < display.MbTob(cephPodMinimumMemory) {
+			return fmt.Errorf(errorMessage, display.BToMb(uint64(podMemoryLimit.Value())), cephPodMinimumMemory)
+		}
 
-	// This means LIMIT < REQUEST
-	// Kubernetes will refuse to schedule that pod however it's still valuable to indicate that user's input was incorrect
-	if uint64(podMemoryLimit.Value()) < uint64(podMemoryRequest.Value()) {
-		extraErrorLine := `\n
-		User has specified a pod memory limit %dmb below the pod memory request %dmb in the cluster CR.\n
-		Rook will create pods that are expected to fail to serve as a more apparent error indicator to the user.`
+		// This means LIMIT < REQUEST
+		// Kubernetes will refuse to schedule that pod however it's still valuable to indicate that user's input was incorrect
+		if uint64(podMemoryLimit.Value()) < uint64(podMemoryRequest.Value()) {
+			extraErrorLine := `\n
+			User has specified a pod memory limit %dmb below the pod memory request %dmb in the cluster CR.\n
+			Rook will create pods that are expected to fail to serve as a more apparent error indicator to the user.`
 
-		return fmt.Errorf(extraErrorLine, display.BToMb(uint64(podMemoryLimit.Value())), display.BToMb(uint64(podMemoryRequest.Value())))
+			return fmt.Errorf(extraErrorLine, display.BToMb(uint64(podMemoryLimit.Value())), display.BToMb(uint64(podMemoryRequest.Value())))
+		}
 	}
 
 	return nil

pkg/operator/ceph/spec/spec_test.go

@@ -17,12 +17,15 @@ limitations under the License.
 package spec
 
 import (
+	"math"
 	"testing"
 
 	"github.com/rook/rook/pkg/operator/ceph/config"
 	"github.com/rook/rook/pkg/operator/k8sutil"
 	"github.com/rook/rook/pkg/operator/test"
 	"github.com/stretchr/testify/assert"
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 )
 
 func TestPodVolumes(t *testing.T) {
@@ -52,3 +55,43 @@ func TestGenerateLifeCycleCmd(t *testing.T) {
 	cmd = generateLifeCycleCmd("foo")
 	assert.Equal(t, append(config.ContainerPostStartCmd, "foo"), cmd)
 }
+
+func TestCheckPodMemory(t *testing.T) {
+	// This value is in MB
+	const PodMinimumMemory uint64 = 1024
+
+	// A value for the memory used in the tests
+	var memory_value = int64(PodMinimumMemory * 8 * uint64(math.Pow10(6)))
+
+	// Case 1: No memory limits, no memory requested
+	test_resource := v1.ResourceRequirements{}
+
+	if err := CheckPodMemory(test_resource, PodMinimumMemory); err != nil {
+		t.Errorf("Error case 1: %s", err.Error())
+	}
+
+	// Case 2: memory limit and memory requested
+	test_resource = v1.ResourceRequirements{
+		Limits: v1.ResourceList{
+			v1.ResourceMemory: *resource.NewQuantity(memory_value, resource.BinarySI),
+		},
+		Requests: v1.ResourceList{
+			v1.ResourceMemory: *resource.NewQuantity(memory_value, resource.BinarySI),
+		},
+	}
+
+	if err := CheckPodMemory(test_resource, PodMinimumMemory); err != nil {
+		t.Errorf("Error case 2: %s", err.Error())
+	}
+
+	// Only memory requested
+	test_resource = v1.ResourceRequirements{
+		Requests: v1.ResourceList{
+			v1.ResourceMemory: *resource.NewQuantity(memory_value, resource.BinarySI),
+		},
+	}
+
+	if err := CheckPodMemory(test_resource, PodMinimumMemory); err != nil {
+		t.Errorf("Error case 3: %s", err.Error())
+	}
+}