Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrading k8s lib #3965

Merged
merged 5 commits into from
Sep 20, 2024
Merged

Upgrading k8s lib #3965

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7d6cc87
Upgrading k8s lib
wilmer05 Sep 20, 2024
ac39d5c
Fixing tests
wilmer05 Sep 20, 2024
5ce7678
fixing setup crd
wilmer05 Sep 20, 2024
cfbea32
fixing setup CR
wilmer05 Sep 20, 2024
5265d56
Removing dead code
wilmer05 Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 9 additions & 24 deletions paasta_tools/kubernetes_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@
from kubernetes.client import models
from kubernetes.client import V1Affinity
from kubernetes.client import V1AWSElasticBlockStoreVolumeSource
from kubernetes.client import V1beta1CustomResourceDefinition
from kubernetes.client import V1beta1CustomResourceDefinitionList
from kubernetes.client import V1beta1PodDisruptionBudget
from kubernetes.client import V1beta1PodDisruptionBudgetSpec
from kubernetes.client import V1Capabilities
Expand All @@ -70,12 +68,12 @@
from kubernetes.client import V1EnvVar
from kubernetes.client import V1EnvVarSource
from kubernetes.client import V1ExecAction
from kubernetes.client import V1Handler
from kubernetes.client import V1HostPathVolumeSource
from kubernetes.client import V1HTTPGetAction
from kubernetes.client import V1KeyToPath
from kubernetes.client import V1LabelSelector
from kubernetes.client import V1Lifecycle
from kubernetes.client import V1LifecycleHandler
from kubernetes.client import V1LimitRange
from kubernetes.client import V1LimitRangeItem
from kubernetes.client import V1LimitRangeSpec
Expand Down Expand Up @@ -607,12 +605,6 @@ def __init__(
self.policy = kube_client.PolicyV1beta1Api(self.api_client)
self.apiextensions = kube_client.ApiextensionsV1Api(self.api_client)

# We need to support apiextensions /v1 and /v1beta1 in order
# to make our upgrade to k8s 1.22 smooth, otherwise
# updating the CRDs make this script fail
self.apiextensions_v1_beta1 = kube_client.ApiextensionsV1beta1Api(
self.api_client
)
self.custom = kube_client.CustomObjectsApi(self.api_client)
self.autoscaling = kube_client.AutoscalingV2beta2Api(self.api_client)
self.rbac = kube_client.RbacAuthorizationV1Api(self.api_client)
Expand Down Expand Up @@ -1112,7 +1104,7 @@ def get_hacheck_sidecar_container(
return V1Container(
image=system_paasta_config.get_hacheck_sidecar_image_url(),
lifecycle=V1Lifecycle(
pre_stop=V1Handler(
pre_stop=V1LifecycleHandler(
_exec=V1ExecAction(
command=[
"/bin/sh",
Expand Down Expand Up @@ -1155,7 +1147,7 @@ def get_gunicorn_exporter_sidecar_container(
env=self.get_kubernetes_environment(),
ports=[V1ContainerPort(container_port=9117)],
lifecycle=V1Lifecycle(
pre_stop=V1Handler(
pre_stop=V1LifecycleHandler(
_exec=V1ExecAction(
command=[
"/bin/sh",
Expand Down Expand Up @@ -1464,20 +1456,20 @@ def get_readiness_probe(
else:
return self.get_liveness_probe(service_namespace_config)

def get_kubernetes_container_termination_action(self) -> V1Handler:
def get_kubernetes_container_termination_action(self) -> V1LifecycleHandler:
command = self.config_dict.get("lifecycle", KubeLifecycleDict({})).get(
"pre_stop_command", []
)
# default pre stop hook for the container
if not command:
return V1Handler(
return V1LifecycleHandler(
_exec=V1ExecAction(
command=["/bin/sh", "-c", f"sleep {DEFAULT_PRESTOP_SLEEP_SECONDS}"]
)
)
if isinstance(command, str):
command = [command]
return V1Handler(_exec=V1ExecAction(command=command))
return V1LifecycleHandler(_exec=V1ExecAction(command=command))

def get_pod_volumes(
self,
Expand Down Expand Up @@ -4155,12 +4147,8 @@ def mode_to_int(mode: Optional[Union[str, int]]) -> Optional[int]:

def update_crds(
kube_client: KubeClient,
desired_crds: Collection[
Union[V1CustomResourceDefinition, V1beta1CustomResourceDefinition]
],
existing_crds: Union[
V1CustomResourceDefinitionList, V1beta1CustomResourceDefinitionList
],
desired_crds: Collection[Union[V1CustomResourceDefinition]],
existing_crds: Union[V1CustomResourceDefinitionList],
Comment on lines +4150 to +4151
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
desired_crds: Collection[Union[V1CustomResourceDefinition]],
existing_crds: Union[V1CustomResourceDefinitionList],
desired_crds: Collection[V1CustomResourceDefinition],
existing_crds: V1CustomResourceDefinitionList,

we don't need a union anymore since we're using a single type now :)

) -> bool:
for desired_crd in desired_crds:
existing_crd = None
Expand All @@ -4170,10 +4158,7 @@ def update_crds(
break
try:

if "apiextensions.k8s.io/v1beta1" == desired_crd.api_version:
apiextensions = kube_client.apiextensions_v1_beta1
else:
apiextensions = kube_client.apiextensions
apiextensions = kube_client.apiextensions

if existing_crd:
desired_crd.metadata[
Expand Down
3 changes: 1 addition & 2 deletions paasta_tools/setup_kubernetes_cr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ def setup_all_custom_resources(
# we need to try both possibilities
for apiextension in [
kube_client.apiextensions,
kube_client.apiextensions_v1_beta1,
]:

try:
Expand All @@ -175,7 +174,7 @@ def setup_all_custom_resources(
).items
except ApiException:
log.debug(
"Listing CRDs with apiextensions/v1 not supported on this cluster, falling back to v1beta1"
"Listing CRDs with apiextensions/v1 not supported on this cluster"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo: i think we'd want to revert the changes to this file where we added the for loop rather than make this a for loop over a single version

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(it would make the code more straightforward and while we may need to handle new apiextension versions in the future, imo it'd be better to only temporarily have this sort of multi-version code)

)
crds_list = []

Expand Down
44 changes: 7 additions & 37 deletions paasta_tools/setup_kubernetes_crd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@
from typing import Sequence

import service_configuration_lib
from kubernetes.client import V1beta1CustomResourceDefinition
from kubernetes.client import V1CustomResourceDefinition
from kubernetes.client.exceptions import ApiException

from paasta_tools.kubernetes_tools import KubeClient
from paasta_tools.kubernetes_tools import paasta_prefixed
Expand Down Expand Up @@ -106,22 +104,7 @@ def setup_kube_crd(
label_selector=paasta_prefixed("service")
)

# This step can fail in k8s 1.22 since this version is not existing anymore
# we need to support this for the transition
try:
existing_crds_v1_beta1 = (
kube_client.apiextensions_v1_beta1.list_custom_resource_definition(
label_selector=paasta_prefixed("service")
)
)
except ApiException:
existing_crds_v1_beta1 = []
log.debug(
"Listing CRDs with apiextensions/v1beta1 not supported on this cluster, falling back to v1"
)

desired_crds = []
desired_crds_v1_beta1 = []
for service in services:
crd_config = service_configuration_lib.read_extra_service_information(
service, f"crd-{cluster}", soa_dir=soa_dir
Expand All @@ -136,31 +119,18 @@ def setup_kube_crd(
metadata["labels"]["yelp.com/paasta_service"] = service
metadata["labels"][paasta_prefixed("service")] = service

if "apiextensions.k8s.io/v1beta1" == crd_config["apiVersion"]:
desired_crd = V1beta1CustomResourceDefinition(
api_version=crd_config.get("apiVersion"),
kind=crd_config.get("kind"),
metadata=metadata,
spec=crd_config.get("spec"),
)
desired_crds_v1_beta1.append(desired_crd)
else:
desired_crd = V1CustomResourceDefinition(
api_version=crd_config.get("apiVersion"),
kind=crd_config.get("kind"),
metadata=metadata,
spec=crd_config.get("spec"),
)
desired_crds.append(desired_crd)
desired_crd = V1CustomResourceDefinition(
api_version=crd_config.get("apiVersion"),
kind=crd_config.get("kind"),
metadata=metadata,
spec=crd_config.get("spec"),
)
desired_crds.append(desired_crd)

return update_crds(
kube_client=kube_client,
desired_crds=desired_crds,
existing_crds=existing_crds,
) and update_crds(
kube_client=kube_client,
desired_crds=desired_crds_v1_beta1,
existing_crds=existing_crds_v1_beta1,
)


Expand Down
2 changes: 1 addition & 1 deletion requirements-minimal.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ kazoo >= 2.0.0
# that we can use across our different clusters (e.g, if X.0.0 removes an API version that we use
# in any cluster, this upper-bound should be < X.0.0)
# we should probably also be better at setting a correct lower-bound, but that's less likely to cause issues.
kubernetes >= 18.20.0, < 22.0.0
kubernetes >= 18.20.0, < 26.0.0
ldap3
manhole
mypy-extensions >= 0.3.0
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ jmespath==0.9.3
jsonref==0.1
jsonschema==2.5.1
kazoo==2.8.0
kubernetes==21.7.0
kubernetes==24.2.0
ldap3==2.6
manhole==1.5.0
MarkupSafe==1.1.1
Expand Down
14 changes: 7 additions & 7 deletions tests/test_kubernetes_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@
from kubernetes.client import V1EnvVar
from kubernetes.client import V1EnvVarSource
from kubernetes.client import V1ExecAction
from kubernetes.client import V1Handler
from kubernetes.client import V1HostPathVolumeSource
from kubernetes.client import V1HTTPGetAction
from kubernetes.client import V1KeyToPath
from kubernetes.client import V1LabelSelector
from kubernetes.client import V1Lifecycle
from kubernetes.client import V1LifecycleHandler
from kubernetes.client import V1NodeAffinity
from kubernetes.client import V1NodeSelector
from kubernetes.client import V1NodeSelectorRequirement
Expand Down Expand Up @@ -603,7 +603,7 @@ def test_get_sidecar_containers(self):
],
image="some-docker-image",
lifecycle=V1Lifecycle(
pre_stop=V1Handler(
pre_stop=V1LifecycleHandler(
_exec=V1ExecAction(
command=[
"/bin/sh",
Expand Down Expand Up @@ -649,7 +649,7 @@ def test_get_sidecar_containers(self):
],
image="some-docker-image",
lifecycle=V1Lifecycle(
pre_stop=V1Handler(
pre_stop=V1LifecycleHandler(
_exec=V1ExecAction(
command=[
"/bin/sh",
Expand Down Expand Up @@ -929,7 +929,7 @@ def test_get_kubernetes_containers(self, prometheus_port, expected_ports):
resources=mock_get_resource_requirements.return_value,
image=mock_get_docker_url.return_value,
lifecycle=V1Lifecycle(
pre_stop=V1Handler(
pre_stop=V1LifecycleHandler(
_exec=V1ExecAction(command=["/bin/sh", "-c", "sleep 30"])
)
),
Expand Down Expand Up @@ -2201,7 +2201,7 @@ def test_kubernetes_container_termination_action(
self.deployment.config_dict["lifecycle"] = {
"pre_stop_command": termination_action
}
handler = V1Handler(_exec=V1ExecAction(command=expected))
handler = V1LifecycleHandler(_exec=V1ExecAction(command=expected))
assert self.deployment.get_kubernetes_container_termination_action() == handler

@pytest.mark.parametrize(
Expand Down Expand Up @@ -4281,7 +4281,7 @@ def test_warning_big_bounce():
job_config.format_kubernetes_app().spec.template.metadata.labels[
"paasta.yelp.com/config_sha"
]
== "configd2fd7b15"
== "config3bd814d2"
), "If this fails, just change the constant in this test, but be aware that deploying this change will cause every service to bounce!"


Expand Down Expand Up @@ -4327,7 +4327,7 @@ def test_warning_big_bounce_routable_pod():
job_config.format_kubernetes_app().spec.template.metadata.labels[
"paasta.yelp.com/config_sha"
]
== "configa2ea39be"
== "configf23a3edb"
), "If this fails, just change the constant in this test, but be aware that deploying this change will cause every smartstack-registered service to bounce!"


Expand Down
Loading