refactor(cmd/ingress): invert signal ctx logic #2139
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
this commit changes the signal handling in cmd/ingress to be wrapped in a context, and inverts which goroutine runs the controller and which watches for the context to be cancelled, which allows some scaffolding (`sync.WaitGroup`) to be removed and now properly handles the controller exiting with `nil` (as it does when leader election fails)
|
@acuteaura Can you fix the failing unit test? Rest, the change LGTM |
|
The test failing is code I didn't touch, which is just an unstable test, it seems to be iterating over a map with two elements and not accounting for map iteration order not being stable.
https://go.dev/ref/spec#RangeClause The range over map is here: Should go into another PR, and I'd be happy to rebase on that. |
|
@acuteaura I have rerun the test. I will fix it in another PR. |
|
@acuteaura Actually other than the flaky test, this one is also failing |
|
Ah, right, Notify can fire more than once, should be fixed now. |
Revolyssup
approved these changes
Jan 31, 2024
AlinsRan
approved these changes
Jan 31, 2024
|
@acuteaura Thanks for your contribution. You can now create the other PR as well |
Revolyssup
pushed a commit
to Revolyssup/apisix-ingress-controller
that referenced
this pull request
Apr 12, 2024
* refactor(cmd/ingress): invert signal ctx logic this commit changes the signal handling in cmd/ingress to be wrapped in a context, and inverts which goroutine runs the controller and which watches for the context to be cancelled, which allows some scaffolding (`sync.WaitGroup`) to be removed and now properly handles the controller exiting with `nil` (as it does when leader election fails)
Revolyssup
added a commit
that referenced
this pull request
Apr 12, 2024
* docs: clarify usage of external service discovery (#2124) * feat: add plugin_config_namespace parameter to ApisixRoute (#2137) * feat: add plugin_config_namespace parameter to ApisixRoute Add plugin_config_namespace parameter to ApisixRoute resource to allow cross namespace discovery. * fix indentation Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com> * remove route.yaml Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com> * fix e2e test Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com> * update gomod gosum * fix e2e test Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com> * fix e2e test Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com> * Update pkg/providers/apisix/apisix_route.go Co-authored-by: Gallardot <gallardot@apache.org> * create namespace * refactor test * refactor test * fix e2e * fix e2e * update crd * Add EOL Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com> --------- Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com> Co-authored-by: Gallardot <gallardot@apache.org> * fix: remove path validation (#2140) * docs: update NOTICE (#2149) * refactor(cmd/ingress): invert signal ctx logic (#2139) * refactor(cmd/ingress): invert signal ctx logic this commit changes the signal handling in cmd/ingress to be wrapped in a context, and inverts which goroutine runs the controller and which watches for the context to be cancelled, which allows some scaffolding (`sync.WaitGroup`) to be removed and now properly handles the controller exiting with `nil` (as it does when leader election fails) * test: failing flaky unit test (#2151) * fix: failing flaky unit test * chore(ci): remove tao12345666333 and lingsamuel in reviewers (#2150) * chore(deps): bump github.com/onsi/ginkgo/v2 in /test/e2e (#2177) Bumps [github.com/onsi/ginkgo/v2](https://github.com/onsi/ginkgo) from 2.13.2 to 2.16.0. - [Release notes](https://github.com/onsi/ginkgo/releases) - [Changelog](https://github.com/onsi/ginkgo/blob/master/CHANGELOG.md) - [Commits](onsi/ginkgo@v2.13.2...v2.16.0) --- updated-dependencies: - dependency-name: github.com/onsi/ginkgo/v2 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump k8s.io/apimachinery from 0.29.0 to 0.29.2 in /test/e2e (#2161) Bumps [k8s.io/apimachinery](https://github.com/kubernetes/apimachinery) from 0.29.0 to 0.29.2. - [Commits](kubernetes/apimachinery@v0.29.0...v0.29.2) --- updated-dependencies: - dependency-name: k8s.io/apimachinery dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump k8s.io/api from 0.29.0 to 0.29.2 in /test/e2e (#2163) Bumps [k8s.io/api](https://github.com/kubernetes/api) from 0.29.0 to 0.29.2. - [Commits](kubernetes/api@v0.29.0...v0.29.2) --- updated-dependencies: - dependency-name: k8s.io/api dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump go.uber.org/zap from 1.26.0 to 1.27.0 in /test/e2e (#2172) Bumps [go.uber.org/zap](https://github.com/uber-go/zap) from 1.26.0 to 1.27.0. - [Release notes](https://github.com/uber-go/zap/releases) - [Changelog](https://github.com/uber-go/zap/blob/master/CHANGELOG.md) - [Commits](uber-go/zap@v1.26.0...v1.27.0) --- updated-dependencies: - dependency-name: go.uber.org/zap dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump k8s.io/client-go from 0.29.0 to 0.29.2 in /test/e2e (#2162) Bumps [k8s.io/client-go](https://github.com/kubernetes/client-go) from 0.29.0 to 0.29.2. - [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md) - [Commits](kubernetes/client-go@v0.29.0...v0.29.2) --- updated-dependencies: - dependency-name: k8s.io/client-go dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump k8s.io/apimachinery from 0.29.2 to 0.29.3 in /test/e2e (#2185) Bumps [k8s.io/apimachinery](https://github.com/kubernetes/apimachinery) from 0.29.2 to 0.29.3. - [Commits](kubernetes/apimachinery@v0.29.2...v0.29.3) --- updated-dependencies: - dependency-name: k8s.io/apimachinery dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: upgrade etcd-adapter to latest version (#2205) * chore(deps): bump github.com/spf13/cobra from 1.7.0 to 1.8.0 (#2196) Bumps [github.com/spf13/cobra](https://github.com/spf13/cobra) from 1.7.0 to 1.8.0. - [Release notes](https://github.com/spf13/cobra/releases) - [Commits](spf13/cobra@v1.7.0...v1.8.0) --- updated-dependencies: - dependency-name: github.com/spf13/cobra dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump github.com/onsi/ginkgo/v2 in /test/e2e (#2195) Bumps [github.com/onsi/ginkgo/v2](https://github.com/onsi/ginkgo) from 2.16.0 to 2.17.1. - [Release notes](https://github.com/onsi/ginkgo/releases) - [Changelog](https://github.com/onsi/ginkgo/blob/master/CHANGELOG.md) - [Commits](onsi/ginkgo@v2.16.0...v2.17.1) --- updated-dependencies: - dependency-name: github.com/onsi/ginkgo/v2 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix: use force=true to hard delete the apisix resource (#2210) Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com> * chore: remove redundant logs and improve logs for users (#2206) * chore: remove redundant logs and improve error when upstream is created Co-authored-by: AlinsRan <alinsran333@gmail.com> --------- Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Gallardot <gallardot@apache.org> Co-authored-by: Leigang Zhang <71714656+zll600@users.noreply.github.com> Co-authored-by: Aurelia <aurelia@acuteaura.net> Co-authored-by: AlinsRan <alinsran333@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.
Type of change:
What this PR does / why we need it:
(verbatin commit message)
this commit changes the signal handling in cmd/ingress to be wrapped in a context, and inverts which goroutine runs the controller and which watches for the context to be cancelled, which allows some scaffolding (
sync.WaitGroup) to be removed and now properly handles the controller exiting withnil(as it does when leader election fails)