Skip to content

Commit

Permalink
Merge pull request #3726 from Yelp/u/cuza/create-limits-for-paasta-na…
Browse files Browse the repository at this point in the history
…mespaces

Adding LimitRanges for paasta managed namespaces
  • Loading branch information
cuza authored Oct 25, 2023
2 parents 3e7fef2 + b37b1c3 commit 406bb5b
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 1 deletion.
46 changes: 45 additions & 1 deletion paasta_tools/kubernetes_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@
from kubernetes.client import V1KeyToPath
from kubernetes.client import V1LabelSelector
from kubernetes.client import V1Lifecycle
from kubernetes.client import V1LimitRange
from kubernetes.client import V1LimitRangeItem
from kubernetes.client import V1LimitRangeSpec
from kubernetes.client import V1Namespace
from kubernetes.client import V1Node
from kubernetes.client import V1NodeAffinity
Expand Down Expand Up @@ -2676,14 +2679,15 @@ def ensure_namespace(kube_client: KubeClient, namespace: str) -> None:
kube_client.core.create_namespace(body=paasta_namespace)

ensure_paasta_api_rolebinding(kube_client, namespace)
ensure_paasta_namespace_limits(kube_client, namespace)


def ensure_paasta_api_rolebinding(kube_client: KubeClient, namespace: str) -> None:
rolebindings = get_all_role_bindings(kube_client, namespace=namespace)
rolebinding_names = [item.metadata.name for item in rolebindings]
if "paasta-api-server-per-namespace" not in rolebinding_names:
log.warning(
f"Creating rolebinding paasta-api-server-per-namespace as it does not exist"
f"Creating rolebinding paasta-api-server-per-namespace on {namespace} namespace as it does not exist"
)
role_binding = V1RoleBinding(
metadata=V1ObjectMeta(
Expand All @@ -2707,6 +2711,39 @@ def ensure_paasta_api_rolebinding(kube_client: KubeClient, namespace: str) -> No
)


def ensure_paasta_namespace_limits(kube_client: KubeClient, namespace: str) -> None:
limits = get_all_limit_ranges(kube_client, namespace=namespace)
limits_names = {item.metadata.name for item in limits}
if "limit-mem-cpu-disk-per-container" not in limits_names:
log.warning(
f"Creating limit: limit-mem-cpu-disk-per-container on {namespace} namespace as it does not exist"
)
limit = V1LimitRange(
metadata=V1ObjectMeta(
name="limit-mem-cpu-disk-per-container",
namespace=namespace,
),
spec=V1LimitRangeSpec(
limits=[
V1LimitRangeItem(
type="Container",
default={
"cpu": "1",
"memory": "1024Mi",
"ephemeral-storage": "1Gi",
},
default_request={
"cpu": "1",
"memory": "1024Mi",
"ephemeral-storage": "1Gi",
},
)
]
),
)
kube_client.core.create_namespaced_limit_range(namespace=namespace, body=limit)


def list_deployments_in_all_namespaces(
kube_client: KubeClient, label_selector: str
) -> List[KubeDeployment]:
Expand Down Expand Up @@ -3893,6 +3930,13 @@ def get_all_role_bindings(
return kube_client.rbac.list_namespaced_role_binding(namespace=namespace).items


def get_all_limit_ranges(
kube_client: KubeClient,
namespace: str,
) -> Sequence[V1LimitRange]:
return kube_client.core.list_namespaced_limit_range(namespace).items


_RE_NORMALIZE_IAM_ROLE = re.compile(r"[^0-9a-zA-Z]+")


Expand Down
29 changes: 29 additions & 0 deletions tests/test_kubernetes_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
from paasta_tools.kubernetes_tools import create_stateful_set
from paasta_tools.kubernetes_tools import ensure_namespace
from paasta_tools.kubernetes_tools import ensure_paasta_api_rolebinding
from paasta_tools.kubernetes_tools import ensure_paasta_namespace_limits
from paasta_tools.kubernetes_tools import filter_nodes_by_blacklist
from paasta_tools.kubernetes_tools import filter_pods_by_service_instance
from paasta_tools.kubernetes_tools import force_delete_pods
Expand Down Expand Up @@ -3022,6 +3023,8 @@ def test_KubeClient():
def test_ensure_namespace_doesnt_create_if_namespace_exists():
with mock.patch(
"paasta_tools.kubernetes_tools.ensure_paasta_api_rolebinding", autospec=True
), mock.patch(
"paasta_tools.kubernetes_tools.ensure_paasta_namespace_limits", autospec=True
):
mock_metadata = mock.Mock()
type(mock_metadata).name = "paasta"
Expand All @@ -3036,6 +3039,8 @@ def test_ensure_namespace_doesnt_create_if_namespace_exists():
def test_ensure_namespace_kube_system():
with mock.patch(
"paasta_tools.kubernetes_tools.ensure_paasta_api_rolebinding", autospec=True
), mock.patch(
"paasta_tools.kubernetes_tools.ensure_paasta_namespace_limits", autospec=True
):
mock_metadata = mock.Mock()
type(mock_metadata).name = "kube-system"
Expand All @@ -3050,6 +3055,8 @@ def test_ensure_namespace_kube_system():
def test_ensure_namespace_creates_namespace_if_doesnt_exist():
with mock.patch(
"paasta_tools.kubernetes_tools.ensure_paasta_api_rolebinding", autospec=True
), mock.patch(
"paasta_tools.kubernetes_tools.ensure_paasta_namespace_limits", autospec=True
):
mock_namespaces = mock.Mock(items=[])
mock_client = mock.Mock(
Expand Down Expand Up @@ -3085,6 +3092,28 @@ def test_ensure_paasta_api_rolebinding_doesnt_create_if_exists():
assert not mock_client.rbac.create_namespaced_role_binding.called


def test_ensure_paasta_namespace_limits_creates_if_not_exist():
mock_limits = mock.Mock(items=[])
mock_client = mock.Mock(
core=mock.Mock(list_namespaced_limit_range=mock.Mock(return_value=mock_limits)),
)

ensure_paasta_namespace_limits(mock_client, namespace="paastasvc-cool-service-name")
assert mock_client.core.create_namespaced_limit_range.called


def test_ensure_paasta_namespace_limits_doesnt_create_if_exists():
mock_metadata = mock.Mock()
type(mock_metadata).name = "limit-mem-cpu-disk-per-container"
mock_limits = mock.Mock(items=[mock.Mock(metadata=mock_metadata)])
mock_client = mock.Mock(
core=mock.Mock(list_namespaced_limit_range=mock.Mock(return_value=mock_limits)),
)

ensure_paasta_namespace_limits(mock_client, namespace="paastasvc-cool-service-name")
assert not mock_client.core.create_namespaced_role_binding.called


@pytest.mark.parametrize(
"addl_labels,replicas",
(
Expand Down

0 comments on commit 406bb5b

Please sign in to comment.