From b711bb6a4db8281a33f9809cff9ba84c9c5fe1e4 Mon Sep 17 00:00:00 2001
From: Steven Stallion <sstallion@gmail.com>
Date: Tue, 12 Jul 2022 09:16:57 -0500
Subject: [PATCH] mounts: fix suggested_swapsize for > 64GB hosts (#1569)

Reference:https://github.com/canonical/cloud-init/commit/b711bb6a4db8281a33f9809cff9ba84c9c5fe1e4
Conflict:(1)tools/.github-cla-signers not change
(2)test format

When provisioning hosts with more than 64GB of memory, swap was not
created and an error was logged by cloud-init. This was due to a
bug in the "suggested_swapsize" implementation, such that the swap
formula was not being taken into account.

This commit fixes the bug while also aligning the recommended swap
size to more closely align with the (no hibernation) swap
recommendations available at
https://help.ubuntu.com/community/SwapFaq
---
 cloudinit/config/cc_mounts.py         | 42 +++++++++------------------
 cloudinit/config/tests/test_mounts.py | 40 ++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 30 deletions(-)

diff --git a/cloudinit/config/cc_mounts.py b/cloudinit/config/cc_mounts.py
index eeb008d..1c6b883 100644
--- a/cloudinit/config/cc_mounts.py
+++ b/cloudinit/config/cc_mounts.py
@@ -65,6 +65,7 @@ swap file is created.
 from string import whitespace
 
 import logging
+import math
 import os
 import re
 
@@ -81,6 +82,8 @@ NETWORK_NAME_RE = re.compile(NETWORK_NAME_FILTER)
 WS = re.compile("[%s]+" % (whitespace))
 FSTAB_PATH = "/etc/fstab"
 MNT_COMMENT = "comment=cloudconfig"
+MB = 2**20
+GB = 2**30
 
 LOG = logging.getLogger(__name__)
 
@@ -176,13 +179,12 @@ def suggested_swapsize(memsize=None, maxsize=None, fsys=None):
     if memsize is None:
         memsize = util.read_meminfo()['total']
 
-    GB = 2 ** 30
-    sugg_max = 8 * GB
+    sugg_max = memsize * 2
 
     info = {'avail': 'na', 'max_in': maxsize, 'mem': memsize}
 
     if fsys is None and maxsize is None:
-        # set max to 8GB default if no filesystem given
+        # set max to default if no filesystem given
         maxsize = sugg_max
     elif fsys:
         statvfs = os.statvfs(fsys)
@@ -200,35 +202,17 @@ def suggested_swapsize(memsize=None, maxsize=None, fsys=None):
 
     info['max'] = maxsize
 
-    formulas = [
-        # < 1G: swap = double memory
-        (1 * GB, lambda x: x * 2),
-        # < 2G: swap = 2G
-        (2 * GB, lambda x: 2 * GB),
-        # < 4G: swap = memory
-        (4 * GB, lambda x: x),
-        # < 16G: 4G
-        (16 * GB, lambda x: 4 * GB),
-        # < 64G: 1/2 M up to max
-        (64 * GB, lambda x: x / 2),
-    ]
-
-    size = None
-    for top, func in formulas:
-        if memsize <= top:
-            size = min(func(memsize), maxsize)
-            # if less than 1/2 memory and not much, return 0
-            if size < (memsize / 2) and size < 4 * GB:
-                size = 0
-                break
-            break
-
-    if size is not None:
-        size = maxsize
+    if memsize < 4 * GB:
+        minsize = memsize
+    elif memsize < 16 * GB:
+        minsize = 4 * GB
+    else:
+        minsize = round(math.sqrt(memsize / GB)) * GB
+        
+    size = min(minsize, maxsize)
 
     info['size'] = size
 
-    MB = 2 ** 20
     pinfo = {}
     for k, v in info.items():
         if isinstance(v, int):
diff --git a/cloudinit/config/tests/test_mounts.py b/cloudinit/config/tests/test_mounts.py
index 56510fd..e922122 100644
--- a/cloudinit/config/tests/test_mounts.py
+++ b/cloudinit/config/tests/test_mounts.py
@@ -1,9 +1,17 @@
 # This file is part of cloud-init. See LICENSE file for license information.
+import math
 from unittest import mock
+from collections import namedtuple
+from pytest import approx
 
 import pytest
 
-from cloudinit.config.cc_mounts import create_swapfile
+from cloudinit.config.cc_mounts import (
+    GB,
+    MB,
+    create_swapfile,
+    suggested_swapsize,
+)
 from cloudinit.subp import ProcessExecutionError
 
 
@@ -59,3 +67,33 @@ class TestCreateSwapfile:
 
         msg = "fallocate swap creation failed, will attempt with dd"
         assert msg in caplog.text
+
+    # See https://help.ubuntu.com/community/SwapFaq
+    @pytest.mark.parametrize(
+        "memsize,expected",
+        [
+            (256 * MB, 256 * MB),
+            (512 * MB, 512 * MB),
+            (1 * GB, 1 * GB),
+            (2 * GB, 2 * GB),
+            (4 * GB, 4 * GB),
+            (8 * GB, 4 * GB),
+            (16 * GB, 4 * GB),
+            (32 * GB, 6 * GB),
+            (64 * GB, 8 * GB),
+            (128 * GB, 11 * GB),
+            (256 * GB, 16 * GB),
+            (512 * GB, 23 * GB),
+        ],
+    )
+    def test_suggested_swapsize(self, memsize, expected, mocker):
+        mock_stat = namedtuple("mock_stat", "f_frsize f_bfree")
+        mocker.patch(
+            "os.statvfs",
+            # Don't care about available disk space for the purposes of this
+            # test
+            return_value=mock_stat(math.inf, math.inf),
+        )
+        size = suggested_swapsize(memsize, math.inf, "dontcare")
+        assert expected == approx(size)
+
-- 
2.33.0


