-
Notifications
You must be signed in to change notification settings - Fork 2.8k
test(log): fix random race detection #5273
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
test(log): fix random race detection #5273
Conversation
Signed-off-by: ivan katliarchuk <[email protected]>
@mloiseleur wdyt? If is going to be flakey, I'll rewrite this test helper |
var b *bytes.Buffer | ||
if len(tt.logExpectations) > 0 { | ||
b = testutils.LogsToBuffer(log.DebugLevel, t) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 What led you to remove the condition here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As initial issue seems like is resolved. I'm saying seems
as data races the hardest to resovle. The -race
flag is a race detector, this condition was to trick -race
detector
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could keep it, no harm
func LogsToBuffer(level log.Level, t *testing.T) *bytes.Buffer { | ||
t.Helper() | ||
buf := new(bytes.Buffer) | ||
log.SetOutput(buf) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the output reset to original value after ? (like a file or os.Stderr)
Anyway, it's better, but I'm not sure it will fix the issue.
Wdyt of this stack overflow example ?
An other way to do it would be to redirect log output to a file, and read it in one row, see for instance traefik integration test on acccess log.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Traefik integration test example will work, but it cover a case when there is a single logger. Exactly why I deleted klog
from the intial function.
At the moment we not testing klog
logs, for that we could have distinct function.
I agree with string, but not sure about paralel -race
writes to file as IO is async, so not sure but we could try if problem still exist. The problelm with bytes.Buffer
- is a slice where mutliple go routines could concurrently write to buffer
Shouldn't the output reset to original value after ?
Yes this is another option, aka buf.Reset()
.
Probably worth to re-run few times to validate that -race
no longer catchin data races
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mloiseleur The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retitle test(log): fix random race detection |
…o v0.17.0 (#712) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [registry.k8s.io/external-dns/external-dns](https://github.com/kubernetes-sigs/external-dns) | minor | `v0.16.1` -> `v0.17.0` | --- ### Release Notes <details> <summary>kubernetes-sigs/external-dns (registry.k8s.io/external-dns/external-dns)</summary> ### [`v0.17.0`](https://github.com/kubernetes-sigs/external-dns/releases/tag/v0.17.0) [Compare Source](kubernetes-sigs/external-dns@v0.16.1...v0.17.0) #### Global information :information_source: On Pi Hole, support for v6 has been added. Pi Hole v5 support is deprecated and will be removed in a future version :information_source: On OVH, the provider has been heavily rewritten. New ACLs are need. See documentation and PR [#​5143](kubernetes-sigs/external-dns#5143) for details. :information_source: On IPv6, the [proposal 002](https://github.com/kubernetes-sigs/external-dns/blob/master/docs/proposal/002-internal-ipv6-handling-rollback.md) has been implemented, thanks to [@​hjoshi123](https://github.com/hjoshi123) -⚠️ In the next release, the default of this new `expose-internal-ipv6` flag will be set to false⚠️ There is a severe known issue with Active Directory ([#​5240](kubernetes-sigs/external-dns#5240)) since v0.16.0. PR to fix it ([#​5385](kubernetes-sigs/external-dns#5385)) is in review :information_source: The legacy txt-format will be removed in the next minor version ([#​5172](kubernetes-sigs/external-dns#5172)) -⚠️ There is currently no migration script to clean old style txt records. If you need it, if you think you can do it, PRs are welcome. ℹ️ A new Nomad source may be added to external-dns ([#​5284](kubernetes-sigs/external-dns#5284)) - This PR can be merged only after a review and test from real Nomad users #### 🚀 Features - feat: additional template functions by [@​matkam](https://github.com/matkam) in [#​3949](kubernetes-sigs/external-dns#3949) - feat(banner): standardize user agent and output by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5154](kubernetes-sigs/external-dns#5154) - feat(chart): Update image to v0.16.1 by [@​stevehipwell](https://github.com/stevehipwell) in [#​5201](kubernetes-sigs/external-dns#5201) - feat(cloudflare): multiple custom hostnames support by [@​mrozentsvayg](https://github.com/mrozentsvayg) in [#​5239](kubernetes-sigs/external-dns#5239) - feat(helm): allow extraArgs to also be a map enabling overrides of individual values by [@​frittentheke](https://github.com/frittentheke) in [#​5293](kubernetes-sigs/external-dns#5293) - feat: IDNA awareness in the zone finder by [@​lexisother](https://github.com/lexisother) in [#​5147](kubernetes-sigs/external-dns#5147) - feat(ovh): major rewriting of the provider by [@​rbeuque74](https://github.com/rbeuque74) in [#​5143](kubernetes-sigs/external-dns#5143) - feat(pihole): add optional support for v6 by [@​tJouve](https://github.com/tJouve) in [#​5226](kubernetes-sigs/external-dns#5226) - feat(pihole): add support for IPv6 Dual format by [@​tJouve](https://github.com/tJouve) in [#​5253](kubernetes-sigs/external-dns#5253) - feat(source): optional exclusion of unschedulable nodes by [@​Hayajiro](https://github.com/Hayajiro) in [#​5045](kubernetes-sigs/external-dns#5045) - feat(source): optional expose of nodes internal ipv6 by [@​hjoshi123](https://github.com/hjoshi123) in [#​5192](kubernetes-sigs/external-dns#5192) #### 🐛 Bug fixes - fix(aws): typo on route53 IAM by [@​tico24](https://github.com/tico24) in [#​5197](kubernetes-sigs/external-dns#5197) - fix(chart): add missing types for empty values by [@​t3mi](https://github.com/t3mi) in [#​5207](kubernetes-sigs/external-dns#5207) - fix(cloudflare): custom hostnames edge-cases causing duplicates by [@​mrozentsvayg](https://github.com/mrozentsvayg) in [#​5183](kubernetes-sigs/external-dns#5183) - fix(cloudflare): regional hostnames by [@​vflaux](https://github.com/vflaux) in [#​5175](kubernetes-sigs/external-dns#5175) - fix(Gateway API): ensure generation match by [@​davidwin93](https://github.com/davidwin93) in [#​5241](kubernetes-sigs/external-dns#5241) - fix(gateway-api): ensure to use only latest generation with HTTPRoutes by [@​kashalls](https://github.com/kashalls) in [#​5349](kubernetes-sigs/external-dns#5349) - fix(helm): added missing schema values by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5228](kubernetes-sigs/external-dns#5228) - fix(helm): update helm schema by [@​semnell](https://github.com/semnell) in [#​5297](kubernetes-sigs/external-dns#5297) - fix(log testing): re-use logger library testing functionality by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5368](kubernetes-sigs/external-dns#5368) - fix(node): logger test fixed by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5232](kubernetes-sigs/external-dns#5232) - fix(ovh): handling capitalized DNS records + prevent panic by [@​rbeuque74](https://github.com/rbeuque74) in [#​5390](kubernetes-sigs/external-dns#5390) - fix(webhook): api json object plan.Changes case by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5355](kubernetes-sigs/external-dns#5355) - fix(zonefinder): handle underscores in dns records by [@​arthlr](https://github.com/arthlr) in [#​5281](kubernetes-sigs/external-dns#5281) #### 📝 Documentation - docs(contributing): add conventional commits by [@​mloiseleur](https://github.com/mloiseleur) in [#​5333](kubernetes-sigs/external-dns#5333) - docs(proposal): externaldns api graduation to beta by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5079](kubernetes-sigs/external-dns#5079) - docs(rfc2136): fix env variable in the guideline by [@​riupie](https://github.com/riupie) in [#​5352](kubernetes-sigs/external-dns#5352) - docs(tutorials): add IONOS Cloud setup tutorial for ExternalDNS by [@​smilutinovic-ionos](https://github.com/smilutinovic-ionos) in [#​5364](kubernetes-sigs/external-dns#5364) - docs(typo): grcp → grpc by [@​octo](https://github.com/octo) in [#​5231](kubernetes-sigs/external-dns#5231) - docs: update link to Anexia webhook provider by [@​mloiseleur](https://github.com/mloiseleur) in [#​5244](kubernetes-sigs/external-dns#5244) - docs: update rfc2136 by [@​BasJ93](https://github.com/BasJ93) in [#​5325](kubernetes-sigs/external-dns#5325) #### 📦 Others - Add Yandex Cloud Webhook by [@​ismailbaskin](https://github.com/ismailbaskin) in [#​5190](kubernetes-sigs/external-dns#5190) - chore: add se for nlb, alb in thailand region by [@​xshot9011](https://github.com/xshot9011) in [#​5200](kubernetes-sigs/external-dns#5200) - chore: fix typo on txtOwnerId comment/description by [@​lanandra](https://github.com/lanandra) in [#​5351](kubernetes-sigs/external-dns#5351) - chore(ci): fix random data race failure on source/node test by [@​mloiseleur](https://github.com/mloiseleur) in [#​5268](kubernetes-sigs/external-dns#5268) - chore(ci): improve release script by [@​mloiseleur](https://github.com/mloiseleur) in [#​5394](kubernetes-sigs/external-dns#5394) - chore(ci): update linter to v2.0.2 by [@​mloiseleur](https://github.com/mloiseleur) in [#​5246](kubernetes-sigs/external-dns#5246) - chore(code-cleanup): move logic away from main.go add tests by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5222](kubernetes-sigs/external-dns#5222) - chore(code): improve some tests + re-order sources flags CLI by [@​mloiseleur](https://github.com/mloiseleur) in [#​5288](kubernetes-sigs/external-dns#5288) - chore(code-quality): added lint checks by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5318](kubernetes-sigs/external-dns#5318) - chore(code-quality): providers linter warnings fixes by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5320](kubernetes-sigs/external-dns#5320) - chore(code-quality): refactoring and linter fixes by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5374](kubernetes-sigs/external-dns#5374) - chore(code-quality): webhook increase code coverage by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5337](kubernetes-sigs/external-dns#5337) - chore(crd): use conventional paths and update controller-gen to v0.17.2 by [@​mloiseleur](https://github.com/mloiseleur) in [#​5287](kubernetes-sigs/external-dns#5287) - chore(dependencies): update toools versions by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5252](kubernetes-sigs/external-dns#5252) - chore(deps): build with go 1.24.2 by [@​mloiseleur](https://github.com/mloiseleur) in [#​5306](kubernetes-sigs/external-dns#5306) - chore(deps): bump renovatebot/github-action from 41.0.14 to 41.0.16 in the dev-dependencies group by [@​app/dependabot](https://github.com/app/dependabot) in [#​5188](kubernetes-sigs/external-dns#5188) - chore(deps): bump renovatebot/github-action from 41.0.16 to 41.0.17 in the dev-dependencies group by [@​app/dependabot](https://github.com/app/dependabot) in [#​5210](kubernetes-sigs/external-dns#5210) - chore(deps): bump the dev-dependencies group across 1 directory with 14 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5346](kubernetes-sigs/external-dns#5346) - chore(deps): bump the dev-dependencies group across 1 directory with 14 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5382](kubernetes-sigs/external-dns#5382) - chore(deps): bump the dev-dependencies group across 1 directory with 17 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5238](kubernetes-sigs/external-dns#5238) - chore(deps): bump the dev-dependencies group across 1 directory with 19 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5262](kubernetes-sigs/external-dns#5262) - chore(deps): bump the dev-dependencies group across 1 directory with 20 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5211](kubernetes-sigs/external-dns#5211) - chore(deps): bump the dev-dependencies group across 1 directory with 21 updates by [@​mloiseleur](https://github.com/mloiseleur) in [#​5313](kubernetes-sigs/external-dns#5313) - chore(deps): bump the dev-dependencies group across 1 directory with 2 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5247](kubernetes-sigs/external-dns#5247) - chore(deps): bump the dev-dependencies group across 1 directory with 2 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5301](kubernetes-sigs/external-dns#5301) - chore(deps): bump the dev-dependencies group across 1 directory with 2 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5323](kubernetes-sigs/external-dns#5323) - chore(deps): code cleanup, pkg/errors is archived by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5335](kubernetes-sigs/external-dns#5335) - chore(deps): switch to goccy yaml by [@​Raffo](https://github.com/Raffo) in [#​5292](kubernetes-sigs/external-dns#5292) - chore(deps): update linter to v2.1.x by [@​mloiseleur](https://github.com/mloiseleur) in [#​5366](kubernetes-sigs/external-dns#5366) - chore(fqdn-template): fqdn templating move to specific folder and update documentation by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5354](kubernetes-sigs/external-dns#5354) - chore(github): add a release template by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5324](kubernetes-sigs/external-dns#5324) - chore(helm): add validation for prefix and suffix and capture regression by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5250](kubernetes-sigs/external-dns#5250) - chore(openstack designate)!: remove in-tree provider by [@​mloiseleur](https://github.com/mloiseleur) in [#​5126](kubernetes-sigs/external-dns#5126) - chore(providers): rename custom TTL constants to defaultTTL by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5312](kubernetes-sigs/external-dns#5312) - chore: Release Helm chart v1.16.1 by [@​stevehipwell](https://github.com/stevehipwell) in [#​5270](kubernetes-sigs/external-dns#5270) - chore(release): updates kustomize & docs with v0.16.1 by [@​mloiseleur](https://github.com/mloiseleur) in [#​5184](kubernetes-sigs/external-dns#5184) - chore(source): code cleanup by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5304](kubernetes-sigs/external-dns#5304) - chore(webhook): bump cenkalti/backoff version by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5342](kubernetes-sigs/external-dns#5342) - test: add tests for cloudflare provider by [@​natitomattis](https://github.com/natitomattis) in [#​5248](kubernetes-sigs/external-dns#5248) - test(log): execute on multiple platforms by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5370](kubernetes-sigs/external-dns#5370) - test(log): fix random race detection by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5273](kubernetes-sigs/external-dns#5273) - test(source): cover unhappy paths by [@​linoleparquet](https://github.com/linoleparquet) in [#​5369](kubernetes-sigs/external-dns#5369) - test(source): fix data race on node_test by [@​mloiseleur](https://github.com/mloiseleur) in [#​5334](kubernetes-sigs/external-dns#5334) - test(source/pod): improve code coverage by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5378](kubernetes-sigs/external-dns#5378) - test(tlsconfig): add unit tests by [@​linoleparquet](https://github.com/linoleparquet) in [#​5381](kubernetes-sigs/external-dns#5381) - test(zone_filter): improve coverage from 66.7% to 100% by [@​upsaurav12](https://github.com/upsaurav12) in [#​5388](kubernetes-sigs/external-dns#5388) #### 📦 Docker Image docker pull registry.k8s.io/external-dns/external-dns:v0.17.0 #### New Contributors - [@​ismailbaskin](https://github.com/ismailbaskin) made their first contribution in kubernetes-sigs/external-dns#5190 - [@​lexisother](https://github.com/lexisother) made their first contribution in kubernetes-sigs/external-dns#5147 - [@​tico24](https://github.com/tico24) made their first contribution in kubernetes-sigs/external-dns#5197 - [@​t3mi](https://github.com/t3mi) made their first contribution in kubernetes-sigs/external-dns#5207 - [@​octo](https://github.com/octo) made their first contribution in kubernetes-sigs/external-dns#5231 - [@​xshot9011](https://github.com/xshot9011) made their first contribution in kubernetes-sigs/external-dns#5200 - [@​tJouve](https://github.com/tJouve) made their first contribution in kubernetes-sigs/external-dns#5226 - [@​Hayajiro](https://github.com/Hayajiro) made their first contribution in kubernetes-sigs/external-dns#5045 - [@​davidwin93](https://github.com/davidwin93) made their first contribution in kubernetes-sigs/external-dns#5241 - [@​vflaux](https://github.com/vflaux) made their first contribution in kubernetes-sigs/external-dns#5175 - [@​arthlr](https://github.com/arthlr) made their first contribution in kubernetes-sigs/external-dns#5281 - [@​semnell](https://github.com/semnell) made their first contribution in kubernetes-sigs/external-dns#5297 - [@​BasJ93](https://github.com/BasJ93) made their first contribution in kubernetes-sigs/external-dns#5325 - [@​natitomattis](https://github.com/natitomattis) made their first contribution in kubernetes-sigs/external-dns#5248 - [@​riupie](https://github.com/riupie) made their first contribution in kubernetes-sigs/external-dns#5352 - [@​lanandra](https://github.com/lanandra) made their first contribution in kubernetes-sigs/external-dns#5351 - [@​smilutinovic-ionos](https://github.com/smilutinovic-ionos) made their first contribution in kubernetes-sigs/external-dns#5364 - [@​linoleparquet](https://github.com/linoleparquet) made their first contribution in kubernetes-sigs/external-dns#5369 - [@​kashalls](https://github.com/kashalls) made their first contribution in kubernetes-sigs/external-dns#5349 - [@​matkam](https://github.com/matkam) made their first contribution in kubernetes-sigs/external-dns#3949 - [@​upsaurav12](https://github.com/upsaurav12) made their first contribution in kubernetes-sigs/external-dns#5388 **Full Changelog**: kubernetes-sigs/external-dns@v0.16.1...v0.17.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC4xMS4xNSIsInVwZGF0ZWRJblZlciI6IjQwLjExLjE1IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJkZXAvbWlub3IiXX0=--> Co-authored-by: JesusMtnez <[email protected]> Reviewed-on: https://codeberg.org/JesusMtnez/homelab/pulls/712 Co-authored-by: JesusMtnez-bot <[email protected]> Co-committed-by: JesusMtnez-bot <[email protected]>
Description
Resolves issues like this one #5268
Capturing both logs that behave differently not a great idea as it looks. At the moment we not testing klogs logs, so not loosing much.
If the problem persist, I debug more. Example next step to try similar stuff https://github.com/sirupsen/logrus/blob/master/internal/testutils/testutils.go, why we not using it, it's in internal package
Checklist