Skip to content

Commit a65db70

Browse files
committed
refactor(swap_config): add BaseSwapConfig abstract base class
GkeSwapConfig and EksSwapConfig now both inherit from BaseSwapConfig(BaseResource). Common sysctl attrs (swappiness, min_free_kbytes, watermark_scale_factor) live in the base class. Cloud-specific attrs remain in each subclass. Addresses Zac review: GkeSwapConfig & EksSwapConfig should inherit from BaseSwapConfig.
1 parent d997254 commit a65db70

1 file changed

Lines changed: 80 additions & 19 deletions

File tree

perfkitbenchmarker/resources/container_service/swap_config.py

Lines changed: 80 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,18 @@
1111
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
14-
"""GkeSwapConfig and EksSwapConfig: swap configuration as PKB BaseResource.
14+
"""Swap configuration as PKB BaseResource: BaseSwapConfig, GkeSwapConfig, EksSwapConfig.
1515
1616
These resources encapsulate cloud-specific swap configuration for GKE and EKS
1717
nodepools. They are referenced via NodepoolSpec.swap_config (declared in the
1818
benchmark BENCHMARK_CONFIG YAML) and consumed by the cloud provider's
1919
_AddNodeParamsToCmd() during cluster/nodepool creation.
2020
21+
Class hierarchy:
22+
BaseSwapConfig(BaseResource) — common sysctl attrs + abstract from_spec()
23+
GkeSwapConfig(BaseSwapConfig) — linuxConfig YAML for --system-config-from-file
24+
EksSwapConfig(BaseSwapConfig) — nodeadm kubelet config (deferred to PR #6780)
25+
2126
Usage in BENCHMARK_CONFIG:
2227
container_cluster:
2328
nodepools:
@@ -51,16 +56,58 @@
5156
_HYPERDISK_MAX_IOPS_PER_MBPS = 256
5257

5358

54-
class GkeSwapConfig(resource.BaseResource):
59+
class BaseSwapConfig(resource.BaseResource):
60+
"""Abstract base class for cloud-specific nodepool swap configuration.
61+
62+
Subclasses (GkeSwapConfig, EksSwapConfig) implement the cloud-specific
63+
method for applying swap configuration during nodepool creation.
64+
65+
Common sysctl attributes (vm.swappiness, vm.min_free_kbytes,
66+
vm.watermark_scale_factor) are shared across all cloud providers.
67+
68+
_Create() and _Delete() are no-ops: the swap config is applied as a
69+
parameter to nodepool creation, not as a standalone cloud resource.
70+
"""
71+
72+
RESOURCE_TYPE = 'BaseSwapConfig'
73+
REQUIRED_ATTRS = []
74+
75+
def __init__(
76+
self,
77+
swappiness: int = 100,
78+
min_free_kbytes: int = 200,
79+
watermark_scale_factor: int = 500,
80+
) -> None:
81+
super().__init__()
82+
self.swappiness = swappiness
83+
self.min_free_kbytes = min_free_kbytes
84+
self.watermark_scale_factor = watermark_scale_factor
85+
86+
@classmethod
87+
def from_spec(cls, swap_spec) -> 'BaseSwapConfig':
88+
"""Create a BaseSwapConfig subclass from a SwapConfigSpec.
89+
90+
Subclasses must override this to instantiate with cloud-specific attrs.
91+
"""
92+
raise NotImplementedError(
93+
f'{cls.__name__}.from_spec() must be implemented by subclasses.'
94+
)
95+
96+
def _Create(self) -> None:
97+
"""No-op: swap config is applied during nodepool creation."""
98+
99+
def _Delete(self) -> None:
100+
"""No-op: cleaned up when the nodepool is deleted."""
101+
102+
103+
class GkeSwapConfig(BaseSwapConfig):
55104
"""GKE swap configuration for a nodepool.
56105
57106
Encapsulates the linuxConfig (swapConfig + sysctl) YAML for
58107
--system-config-from-file and optional Hyperdisk IOPS/throughput overrides.
59108
60109
Consumed by GkeCluster._AddNodeParamsToCmd() when nodepool_config.swap_config
61-
is set. _Create() and _Delete() are no-ops because the swap config is applied
62-
as part of the gcloud node-pools create command; the nodepool itself manages
63-
the lifecycle.
110+
is set.
64111
65112
Attributes:
66113
swappiness: vm.swappiness sysctl value (0-200, default 100).
@@ -85,10 +132,11 @@ def __init__(
85132
boot_disk_iops: int = 0,
86133
boot_disk_throughput: int = 0,
87134
) -> None:
88-
super().__init__()
89-
self.swappiness = swappiness
90-
self.min_free_kbytes = min_free_kbytes
91-
self.watermark_scale_factor = watermark_scale_factor
135+
super().__init__(
136+
swappiness=swappiness,
137+
min_free_kbytes=min_free_kbytes,
138+
watermark_scale_factor=watermark_scale_factor,
139+
)
92140
self.lssd = lssd
93141
self.lssd_count = lssd_count
94142
self.boot_disk_iops = boot_disk_iops
@@ -108,11 +156,8 @@ def from_spec(cls, swap_spec) -> 'GkeSwapConfig':
108156
boot_disk_throughput=swap_spec.boot_disk_throughput,
109157
)
110158

111-
def _Create(self) -> None:
112-
"""No-op: swap config is applied during nodepool creation."""
113-
114159
def _Delete(self) -> None:
115-
"""No-op: cleaned up when the nodepool is deleted."""
160+
"""Cleans up any written YAML tempfile."""
116161
self._CleanupYaml()
117162

118163
def WriteLinuxConfigYaml(self) -> str:
@@ -207,13 +252,16 @@ def _CleanupYaml(self) -> None:
207252
self.CleanupYaml()
208253

209254

210-
class EksSwapConfig(resource.BaseResource):
255+
class EksSwapConfig(BaseSwapConfig):
211256
"""EKS swap configuration for a nodepool (stub).
212257
213258
Configures kubelet LimitedSwap via nodeadm bootstrap configuration.
214259
Full implementation deferred to PR #6780.
215260
216261
Attributes:
262+
swappiness: vm.swappiness sysctl value (inherited from BaseSwapConfig).
263+
min_free_kbytes: vm.min_free_kbytes sysctl (inherited from BaseSwapConfig).
264+
watermark_scale_factor: vm.watermark_scale_factor (inherited from BaseSwapConfig).
217265
memory_swap_behavior: kubelet memorySwapBehavior value ('LimitedSwap').
218266
fail_swap_on: kubelet failSwapOn setting (False to allow swap on EKS).
219267
"""
@@ -223,17 +271,28 @@ class EksSwapConfig(resource.BaseResource):
223271

224272
def __init__(
225273
self,
274+
swappiness: int = 100,
275+
min_free_kbytes: int = 200,
276+
watermark_scale_factor: int = 500,
226277
memory_swap_behavior: str = 'LimitedSwap',
227278
fail_swap_on: bool = False,
228279
) -> None:
229-
super().__init__()
280+
super().__init__(
281+
swappiness=swappiness,
282+
min_free_kbytes=min_free_kbytes,
283+
watermark_scale_factor=watermark_scale_factor,
284+
)
230285
self.memory_swap_behavior = memory_swap_behavior
231286
self.fail_swap_on = fail_swap_on
232287

233288
@classmethod
234289
def from_spec(cls, swap_spec) -> 'EksSwapConfig':
235290
"""Create an EksSwapConfig from a SwapConfigSpec."""
236-
return cls()
291+
return cls(
292+
swappiness=swap_spec.swappiness,
293+
min_free_kbytes=swap_spec.min_free_kbytes,
294+
watermark_scale_factor=swap_spec.watermark_scale_factor,
295+
)
237296

238297
def _Create(self) -> None:
239298
"""Stub: EKS kubelet LimitedSwap config via nodeadm (deferred to PR #6780)."""
@@ -243,9 +302,6 @@ def _Create(self) -> None:
243302
'(deferred to PR #6780). Swap will not be enabled on EKS nodes.'
244303
)
245304

246-
def _Delete(self) -> None:
247-
"""No-op."""
248-
249305
def GetNodeadmConfig(self) -> str:
250306
"""Return nodeadm bootstrap YAML for kubelet swap settings."""
251307
return (
@@ -256,4 +312,9 @@ def GetNodeadmConfig(self) -> str:
256312
' config:\n'
257313
f' memorySwapBehavior: {self.memory_swap_behavior}\n'
258314
f' failSwapOn: {str(self.fail_swap_on).lower()}\n'
315+
' containerd:\n'
316+
' config:\n'
317+
f' vm.swappiness: {self.swappiness}\n'
318+
f' vm.min_free_kbytes: {self.min_free_kbytes}\n'
319+
f' vm.watermark_scale_factor: {self.watermark_scale_factor}\n'
259320
)

0 commit comments

Comments
 (0)