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

Adjusted Nginx ingress canary headers on init and promotion #907

Merged
merged 3 commits into from
May 14, 2021

Conversation

vorozhko
Copy link

Adjusted canary headers after canary deployment is promoted
It resolves validation webhook issue

Iaroslav Vorozhko added 2 commits May 13, 2021 14:45
Signed-off-by: Iaroslav Vorozhko <iaroslav.vorozhko@mcmakler.de>
Signed-off-by: Iaroslav Vorozhko <iaroslav.vorozhko@mcmakler.de>
@codecov-commenter
Copy link

codecov-commenter commented May 13, 2021

Codecov Report

Merging #907 (d7999e6) into main (3ad55c9) will increase coverage by 0.18%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #907      +/-   ##
==========================================
+ Coverage   56.76%   56.95%   +0.18%     
==========================================
  Files          69       69              
  Lines        5704     5710       +6     
==========================================
+ Hits         3238     3252      +14     
+ Misses       1983     1975       -8     
  Partials      483      483              
Impacted Files Coverage Δ
pkg/router/ingress.go 73.55% <100.00%> (+4.85%) ⬆️
pkg/canary/config_tracker.go 84.16% <0.00%> (+1.80%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ad55c9...d7999e6. Read the comment docs.

@stefanprodan
Copy link
Member

@vorozhko can you please enable the webhook in NGINX e2e tests to verify that this works? Thanks

@vorozhko
Copy link
Author

Default ingress nginx helm has webhook enabled by default.

@stefanprodan
Copy link
Member

stefanprodan commented May 14, 2021

So how come the test passes without this PR? Maybe the version we use in e2e doesn’t have the webhook?

…ook after canary promotion

Signed-off-by: Iaroslav Vorozhko <iaroslav.vorozhko@mcmakler.de>
@vorozhko
Copy link
Author

Added Nginx e2e tests to cover validation webhook case.

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @vorozhko 🏅

@stefanprodan stefanprodan merged commit 84ff6f7 into fluxcd:main May 14, 2021
@vorozhko
Copy link
Author

Thank you too!

@vorozhko vorozhko deleted the nginx-ingress-cleanup branch May 17, 2021 10:12
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.

Canary ingress nginx prevent update of primary ingress due to admission webhook
3 participants