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

feat(geoipupdater): remove the mount for the pvc as we use azcopy #1454

Merged
merged 16 commits into from
Dec 2, 2024

Conversation

smerle33
Copy link
Contributor

@smerle33 smerle33 commented Nov 28, 2024

as per jenkins-infra/helpdesk#4278
providing a new chart version to avoid the mount for the pvc but provide the informations for the fileshare target for azcopy.
⚠️ BREAKING CHANGE

  • it remove the pvc mounting and rely only on the azcopy.

  • it change the definition of the image used to match the ones we are using

  • add the dry-run option to avoid geoip ratelimit

depends on jenkins-infra/docker-geoipupdate#11 and jenkins-infra/docker-geoipupdate#13

split the kubernetes objects within 2 files (one for rbac, and one for the cronjob) with corresponding unit test.
Change the docker image to use to our own image see https://github.com/jenkins-infra/docker-geoipupdate and update the corresponding updatecli manifest to track it.

Change the DEFAULT dry-run value to true.

The resources are not set but correct minimum values are provided within a comment.

@dduportal dduportal added breaking and removed enhancement New feature or request labels Nov 28, 2024
@smerle33 smerle33 marked this pull request as draft November 28, 2024 16:51
@smerle33 smerle33 marked this pull request as ready for review December 2, 2024 13:21
Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

Pair review

charts/geoipupdates/values.yaml Outdated Show resolved Hide resolved
charts/geoipupdates/values.yaml Outdated Show resolved Hide resolved
charts/geoipupdates/values.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

pair review

@smerle33 smerle33 requested a review from dduportal December 2, 2024 15:55
Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

The updatecli manifest is incorrect, but I'll let you fix it on a subsequent PR (not blocking the main chart change)

@dduportal dduportal merged commit d0a5eb6 into jenkins-infra:main Dec 2, 2024
3 checks passed
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.

2 participants