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(helm): upgrade traefik (#832) #832

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

Alputer
Copy link
Member

@Alputer Alputer commented Sep 10, 2024

I have realized that our traefik dependency is from 3 years ago. Updating the treaefik dependency resolved the linting error. I tested it with 5 different workflows and it seems to be working fine.

closes (#798)

@Alputer Alputer self-assigned this Sep 10, 2024
Copy link

codecov bot commented Sep 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 29.80%. Comparing base (fedc752) to head (0c0849d).
Report is 1 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #832   +/-   ##
=======================================
  Coverage   29.80%   29.80%           
=======================================
  Files          26       26           
  Lines        2486     2486           
=======================================
  Hits          741      741           
  Misses       1745     1745           

@mdonadoni
Copy link
Member

mdonadoni commented Sep 19, 2024

Works well!

By moving from the Traefik helm chart v10.6.2 to v31.0.0 we are also moving from Traefik v2.5.4 (source) to v3.1.2 (source).

I have checked the migration guide and the changes and it seems like we are not affected by any of it, even more so because we are using k8s Ingresses and not traefik's IngressRoutes. The only change that might have affected us is the change of behaviour of PathPrefix, which does not support regexes anymore (source); we are not using regexs in router rules, so we are good.

Just some small comments:

  • Could you rebase the PR against latest master?
  • Could you chage the commit message to highlight that we are moving to traefik v3? Something like build(helm): upgrade Traefik to v3 (#832)
  • Traefik helm chart v31.1.0 was released yesterday. Maybe we should already upgrade to this version?

Alputer added a commit to Alputer/reana that referenced this pull request Sep 19, 2024
@Alputer
Copy link
Member Author

Alputer commented Sep 19, 2024

I rebased and updated the commit message. I also tested the version 31.1.0 and it seems to be working fine too. So updated the version from 31.0.0 to 31.1.0.

@mdonadoni mdonadoni merged commit 0c0849d into reanahub:master Sep 19, 2024
12 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