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

[Helm] Don't try to install Traefik Middleware when Traefik is disabled #7184

Merged
merged 3 commits into from
Nov 29, 2023

Conversation

ozen
Copy link
Contributor

@ozen ozen commented Nov 26, 2023

This PR is complementary to #7132.

#7132 updated ingress templates to work with traefik.enabled=false and ingress.enabled=true values.

However the templates in templates/analytics/middlewares path lead to creation of resources of kind Middleware, which is a Traefik CRD. Helm install fails if Traefik is not installed on the cluster.

Changing the top-level conditional from ingress.enabled to traefik.enabled solves the issue.

@ozen ozen requested a review from azhavoro as a code owner November 26, 2023 11:04
Copy link

codecov bot commented Nov 26, 2023

Codecov Report

Merging #7184 (44cde5e) into develop (3bf88c5) will decrease coverage by 0.06%.
The diff coverage is n/a.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7184      +/-   ##
===========================================
- Coverage    81.48%   81.42%   -0.06%     
===========================================
  Files          364      364              
  Lines        39877    39877              
  Branches      3702     3702              
===========================================
- Hits         32494    32471      -23     
- Misses        7383     7406      +23     
Components Coverage Δ
cvat-ui 75.46% <ø> (-0.12%) ⬇️
cvat-server 87.02% <ø> (-0.01%) ⬇️

@SpecLad
Copy link
Contributor

SpecLad commented Nov 28, 2023

Looks good, thanks. Could you add a changelog entry?

@ozen ozen force-pushed the fix/helm-traefik-middleware branch from 44cde5e to 1ddc82b Compare November 29, 2023 08:25
@ozen ozen requested a review from nmanovic as a code owner November 29, 2023 08:25
@ozen
Copy link
Contributor Author

ozen commented Nov 29, 2023

Looks good, thanks. Could you add a changelog entry?

Sure, added.

@SpecLad SpecLad merged commit 946fb90 into cvat-ai:develop Nov 29, 2023
32 of 33 checks passed
@ozen ozen deleted the fix/helm-traefik-middleware branch November 30, 2023 11:55
@cvat-bot cvat-bot bot mentioned this pull request Dec 11, 2023
amjadsaadeh pushed a commit to amjadsaadeh/cvat that referenced this pull request Dec 14, 2023
…ed (cvat-ai#7184)

This PR is complementary to cvat-ai#7132. 

cvat-ai#7132 updated ingress templates to work with `traefik.enabled=false` and
`ingress.enabled=true` values.

However the templates in `templates/analytics/middlewares` path lead to
creation of resources of kind `Middleware`, which is a Traefik CRD. Helm
install fails if Traefik is not installed on the cluster.

Changing the top-level conditional from `ingress.enabled` to
`traefik.enabled` solves the issue.
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.

None yet

2 participants