-
Notifications
You must be signed in to change notification settings - Fork 590
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
fix(admission) update certificates when they change #2258
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rainest
force-pushed
the
fix/certificate-watcher
branch
from
February 11, 2022 23:23
b718e5a
to
61725d7
Compare
rainest
force-pushed
the
fix/certificate-watcher
branch
from
February 11, 2022 23:34
61725d7
to
3f96bd1
Compare
E2E in CI seems to not flip after 4m, but does locally. Not sure if there's any explanation beyond CI being slower--even locally it does take quite a while after pushing the update for it to be visible. Will poke around. |
rainest
force-pushed
the
fix/certificate-watcher
branch
from
February 15, 2022 21:26
618632a
to
eb22e83
Compare
rainest
force-pushed
the
fix/certificate-watcher
branch
from
February 15, 2022 21:36
eb22e83
to
c447f05
Compare
rainest
force-pushed
the
fix/certificate-watcher
branch
from
February 15, 2022 21:40
c447f05
to
a1dfdee
Compare
rainest
force-pushed
the
fix/certificate-watcher
branch
from
February 15, 2022 22:17
a1dfdee
to
8b0ecbb
Compare
rainest
force-pushed
the
fix/certificate-watcher
branch
from
February 15, 2022 22:19
8b0ecbb
to
bb9aef0
Compare
rainest
force-pushed
the
fix/certificate-watcher
branch
from
February 18, 2022 21:29
a97fec8
to
8edb14c
Compare
shaneutt
suggested changes
Feb 22, 2022
Co-authored-by: Shane Utt <shaneutt@linux.com>
shaneutt
approved these changes
Feb 23, 2022
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
The admission controller doesn't update certificates when the file updates. This makes the controller update that certificate when the file updates).
Which issue this PR fixes:
fixes #986 (only for files though, we can't reload/change the environment after start)
Fixes a CI issue where targeted runs weren't actually using their custom image.
Special notes for your reviewer:
https://github.com/Kong/kubernetes-ingress-controller/actions/runs/1866651736 is an example run with the change.
This is not yet supported in the chart: Kong/charts#543
This additionally reorganizes the E2E tests, which had almost everything in all_in_one_test.go.
Minor bugfixes and changes to the E2E port forward helper, because I originally tried to use that with weird results (forwards would inexplicably and randomly die after coming up), so I exposed the service instead.
This adds a new E2E helper to retrieve logs from the test cluster. This isn't actually used because I didn't have a good way to have it only log when it's of interest (when
deployKong()
fails), and it's spammy otherwise. It's still useful for debugging CI-specific issues, but you need to add it to a debug branch temporarily for now. We may be able to merge in use by refactoring around https://pkg.go.dev/github.com/stretchr/testify/suite?utm_source=godoc#AfterTest, unsure. For reference, actually using it looks like log.diff.txtPR Readiness Checklist:
Complete these before marking the PR as
ready to review
:CHANGELOG.md
release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR