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.

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 kubernetes client library to v24.2.0 #3632

Merged

Conversation

wilmer05
Copy link
Contributor

@wilmer05 wilmer05 commented Jun 27, 2023

As part of our kubernetes upgrade (to 1.22) we need to upgrade kubernetes libraries being used to support k8s 1.22.

The kubernetes client library got updated in this PR to v24.2.0 and the proper tests were updated

PAASTA-17925

@wilmer05 wilmer05 requested review from gmdfalk and ajayOO8 June 27, 2023 08:28
Copy link
Member

@nemacysts nemacysts left a comment

Choose a reason for hiding this comment

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

seems fine to me - but we should probably test this on kubestage/infrastage before merging

@@ -3905,7 +3905,7 @@ def test_warning_big_bounce():
job_config.format_kubernetes_app().spec.template.metadata.labels[
"paasta.yelp.com/config_sha"
]
== "config9e0d925d"
== "configc22655c7"
Copy link
Member

Choose a reason for hiding this comment

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

huh, interesting - I wonder what changed here in the underlying config (maybe the ordering in

json.dumps(config, sort_keys=True).encode("UTF-8")
?)

@wilmer05 wilmer05 force-pushed the u/wilmerrafael/PAASTA-17925_upgrading_kubernetes_library branch from b75e2d5 to 0fc36f8 Compare July 5, 2023 13:19
@@ -33,7 +33,7 @@


INTERNAL_CRDS = [
V1beta1CustomResourceDefinition(
V1CustomResourceDefinition(
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to update this CRD definition as well. there are few deprecated keys in V1
https://kubernetes.io/docs/reference/using-api/deprecation-guide/#customresourcedefinition-v122

i have an open PR which was sortof doing the same thing.
https://github.com/Yelp/paasta/pull/3611/files#diff-59d82c9819b7780a1257b5ef686a4f93d5b6626181b33637448ce656f3ce6650

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the catch, I will add the same in this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

there is another CRD definition L85:L116 that we might need to update.

sorry, I didn't see that earlier

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated it, thanks for the catch

@wilmer05 wilmer05 merged commit 2911df0 into master Jul 7, 2023
6 checks passed
gmdfalk added a commit that referenced this pull request Jul 10, 2023
Need to do a clean release of PaaSTA v0.183.0 that we can release to production.

This PR is currently blocking that release because PaaSTA cannot apply CRDs with version `v1beta1`.

We'll re-apply this PR once 183 is released.

Reverts #3632
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants