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

First steps towards upgrading Kubernetes, Minikube and Python. #42

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ntwalibas
Copy link
Contributor

@ntwalibas ntwalibas commented Nov 25, 2024

Description

This PR upgrades Kubernetes, Minikube and Python to newer versions that have at least one year of support.

Dependency Changes

The Pipfile now adds urllib3 as a dependency since it is required for a successful deployment.

Documentation (TODO)

Once this has been successfully done, the documentation must be updated to reflect the change in the Python version.
See the CKAN project build & deployment page for the need to do this -- it specifically asks users to have Python 3.8 installed.

Checklist

  • The Jira ticket for this issue has been updated to "Ready to Review" or equivalent.
  • I have developed these changes in discussion with the appropriate project manager.
  • My code follows the general Fjelltopp documentation (see Confluence).
  • I have made corresponding changes to the Fjelltopp documentation (see Confluence).
  • I have rebased this branch with master.
  • New dependency changes have been committed.
  • I have added automated tests that prove my fix is effective or that my feature works.
  • New and existing tests pass locally with my changes.
  • My changes generate no new warnings.
  • I have performed a self-review of my own code.
  • I have assigned at least one reviewer.
  • I have assigned at least one label to this PR: "patch", "minor", "major".

@ntwalibas
Copy link
Contributor Author

This PR should remain a draft until all projects can work with the changes made.
For instance, I was working with wac and zarr and I couldn't pull the image for clementmouchet/datapusher because of the following deprecation notice:

Using default tag: latest
latest: Pulling from clementmouchet/datapusher
[DEPRECATION NOTICE] Docker Image Format v1 and Docker Image manifest version 2, schema 1 support is disabled by default and will be removed in an upcoming release. Suggest the author of docker.io/clementmouchet/datapusher:latest to upgrade the image to the OCI Format or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/
ssh: Process exited with status 1

Moreover, I've been getting an error trying to have the fjelltopp-ansible/roles/ckan/templates/kubernetes/giftless_ingress.yaml file to run successfully with the message:

Ingress giftless-ingress: Failed to create object: b'{\"kind\":\"Status\",\"apiVersion\":\"v1\",\"metadata\":{},\"status\":\"Failure\",\"message\":\"admission webhook \\\\\"validate.nginx.ingress.kubernetes.io\\\\\" denied the request: nginx.ingress.kubernetes.io/configuration-snippet annotation cannot be used. Snippet directives are disabled by the Ingress administrator\",\"reason\":\"BadRequest\",\"code\":400}\\n'

So until error messages such as those are resolved, this PR cannot be merged.

@cooper667
Copy link
Collaborator

That datapusher image is very old (as we found out in AFRO) and likely shouldn't be used by any of our projects. AFRO uses one built from https://github.com/fjelltopp/datapusher

@cooper667
Copy link
Collaborator

k8s_version: "1.29.0" # ['1.29.0', '1.28.5', '1.28.3', '1.27.9', '1.27.7', '1.26.12', '1.26.10']. AWS is on 1.24

It's defined here rather than using the other setting as Azure has different valid versions, but we should likely take this opportunity to make this 1.30 too, as this version is LTS so will last us a while!

image

(1.31 is available, but isn't an LTS version, so likely not worth the bump, unless we plan on doing this process every 6 months)

@jonathansberry jonathansberry mentioned this pull request Dec 3, 2024
12 tasks
@@ -2,11 +2,11 @@
aws_region: "eu-west-1"
# Current EKS Kubernetes version
# https://docs.aws.amazon.com/eks/latest/userguide/kubernetes-versions.html
k8s_version: "1.24"
k8s_version: "1.30"
Copy link
Member

Choose a reason for hiding this comment

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

Why not 1.31?

Copy link
Member

Choose a reason for hiding this comment

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

1.30 is LTS I think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think that's a distinction AWS make tbh, if we go 1.30 now, will have to do this again before July 28, 2025 to avoid fees. 1.31 would give an extra 6 months (although we should get in the habit of doing it frequently)

Copy link
Member

Choose a reason for hiding this comment

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

Cool - 1.31 it is!
And, yes, we probably need to get into the habit of doing this at least annually.


[requires]
python_version = "3.8"
python_version = "3.10"
Copy link
Member

Choose a reason for hiding this comment

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

This change is already proposed here: #36

@@ -22,6 +22,7 @@ uamqp = "*"
requests = {extras = ["security"], version = "<2.32"}
psycopg2 = "*"
resolvelib = "==0.8.1"
urllib3 = "*"
Copy link
Member

Choose a reason for hiding this comment

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

I have urllib3 in my lock file without having specified it in the Pipfile. I guess there is no danger to this though.

Copy link
Member

Choose a reason for hiding this comment

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

If it's in our lockfile already then we don't need to put it here, especially as a wildcard. Either it should be a specific version (ideally everything should be but let's start here) or not added.

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

Successfully merging this pull request may close these issues.

4 participants