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 k8s lib #3965

Merged
merged 5 commits into from
Sep 20, 2024
Merged

Upgrading k8s lib #3965

merged 5 commits into from
Sep 20, 2024

Conversation

wilmer05
Copy link
Contributor

@wilmer05 wilmer05 commented Sep 20, 2024

We need to update the kubernetes library to newer versions.

Containerd gives problems when looking for container image names with the kubernetes client in the current version.

PAASTA-18345

ticket describing the issue COMPINFRA-4411 and doc

self.apiextensions_v1_beta1 = kube_client.ApiextensionsV1beta1Api(
self.api_client
)
# self.apiextensions_v1_beta1 = kube_client.ApiextensionsV1beta1Api(
Copy link
Member

Choose a reason for hiding this comment

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

was it dead code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was needed to go to k8s 1.22, should we remove it completely? I just wanted to keep it in case we face the same change of API in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it now

Copy link
Member

Choose a reason for hiding this comment

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

if we need this code again we can grab it from git history :)

@wilmer05 wilmer05 requested a review from a team as a code owner September 20, 2024 15:20
self.apiextensions_v1_beta1 = kube_client.ApiextensionsV1beta1Api(
self.api_client
)
# self.apiextensions_v1_beta1 = kube_client.ApiextensionsV1beta1Api(
Copy link
Member

Choose a reason for hiding this comment

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

if we need this code again we can grab it from git history :)

Comment on lines +4156 to +4157
desired_crds: Collection[Union[V1CustomResourceDefinition]],
existing_crds: Union[V1CustomResourceDefinitionList],
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 :)

@@ -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)

@wilmer05 wilmer05 merged commit 14ad34d into master Sep 20, 2024
10 checks passed
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.

3 participants