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

mixin: adhere to RFC 1123 compatible component naming #4883

Merged
merged 4 commits into from Jan 5, 2022
Merged

mixin: adhere to RFC 1123 compatible component naming #4883

merged 4 commits into from Jan 5, 2022

Conversation

itspngu
Copy link
Contributor

@itspngu itspngu commented Nov 19, 2021

Fixes issue #4880.

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

  • Renamed bucket_replicate.libsonnet to bucket-replicate.libsonnet & changed references to it accordingly
  • Renamed query_frontend.libsonnet to query-frontend.libsonnet & changed references to it accordingly
  • Changed component name generation & sanitization in utils.libsonnet
  • Regenerated examples & docs

Verification

@itspngu
Copy link
Contributor Author

itspngu commented Nov 22, 2021

I'm afraid I'll need help with the failing e2e tests: https://github.com/thanos-io/thanos/runs/4280932433?check_suite_focus=true#step:5:2301

When running locally, the tests also fail (same final error), but with different runtime output. TestQueryFrontendMemcachedCache (failing in the linked workflow run) passes.

$ make test-e2e | tee test-e2e.log
all modules verified
>> building Thanos binary in /home/pngu/.asdf/installs/golang/1.17.2/packages/bin
 >   thanos
>> copying Thanos from /home/pngu/.asdf/installs/golang/1.17.2/packages/bin to ./thanos_tmp_for_docker
>> building docker image 'thanos'
[...]
>> cleaning e2e test garbage.
>> running /test/e2e tests.
# NOTE(bwplotka):
# * If you see errors on CI (timeouts), but not locally, try to add -parallel 1 to limit to single CPU to reproduce small 1CPU machine.
[...]
08:27:36 Killing minio
08:27:36 Killing minio
--- PASS: TestCompactWithStoreGatewayWithPenaltyDedup (56.89s)
--- PASS: TestCompactWithStoreGateway (57.00s)
FAIL
FAIL	github.com/thanos-io/thanos/test/e2e	57.250s
?   	github.com/thanos-io/thanos/test/e2e/e2ethanos	[no test files]
FAIL
make: *** [Makefile:244: test-e2e] Error 1

$ cat test-e2e.log | grep TestQueryFrontendMemcachedCache
=== RUN   TestQueryFrontendMemcachedCache
=== PAUSE TestQueryFrontendMemcachedCache
=== CONT  TestQueryFrontendMemcachedCache
--- PASS: TestQueryFrontendMemcachedCache (20.35s)

cat test-e2e.log | grep -B5 FAIL
    query_test.go:410: query_test.go:410:

         unexpected error: create docker network 'e2e_test_query_comp_query_0': exit status 1

=== CONT  TestReceive/hashring
--- FAIL: TestQueryCompatibilityWithPreInfoAPI (0.42s)
    --- FAIL: TestQueryCompatibilityWithPreInfoAPI/{queryImage:thanos_sidecarImage:quay.io/thanos/thanos:v0.22.0} (0.42s)
=== CONT  TestQueryFrontend
    query_frontend_test.go:30: query_frontend_test.go:30:

         unexpected error: create docker network 'e2e_test_query_frontend': exit status 1

--- FAIL: TestQueryFrontend (0.42s)
--
=== CONT  TestQuery
    query_test.go:110: query_test.go:110:

         unexpected error: create docker network 'e2e_test_query': exit status 1

--- FAIL: TestQuery (0.45s)
--
08:48:00 Killing prometheus-1
08:48:00 store-gw-1: level=info name=store-gw-1 ts=2021-11-22T08:48:00.195041309Z caller=fetcher.go:481 component=block.BaseFetcher msg="successfully synchronized block metadata" duration=7.70661ms duration_ms=7 cached=30 returned=30 partial=5
08:48:00 Killing receive-3
08:48:00 Killing receive-2
08:48:00 Killing receive-1
--- FAIL: TestReceive (0.00s)
    --- FAIL: TestReceive/multitenancy (0.42s)
    --- FAIL: TestReceive/hashring (0.16s)
--
queryAndAssert: Waiting for 34 results for query count_over_time({a="1"}[13h] offset 52490898s)
08:48:18 Killing querier-1
08:48:18 Killing store-gw-1
08:48:19 Killing minio
--- PASS: TestCompactWithStoreGateway (54.02s)
FAIL
FAIL	github.com/thanos-io/thanos/test/e2e	54.282s
?   	github.com/thanos-io/thanos/test/e2e/e2ethanos	[no test files]
FAIL

As a quick-and-dirty sanity check I ran the same tests on the current HEAD of main, and they fail too:

$ make test-e2e | tee test-e2e-main.log
>> building Thanos binary in /home/pngu/.asdf/installs/golang/1.17.2/packages/bin
 >   thanos
>> copying Thanos from /home/pngu/.asdf/installs/golang/1.17.2/packages/bin to ./thanos_tmp_for_docker
>> building docker image 'thanos'
[...]
>> cleaning e2e test garbage.
>> running /test/e2e tests.
# NOTE(bwplotka):
# * If you see errors on CI (timeouts), but not locally, try to add -parallel 1 to limit to single CPU to reproduce small 1CPU machine.
[...]
queryAndAssert: Waiting for 34 results for query count_over_time({a="1"}[13h] offset 52491480s)
08:58:00 Killing querier-1
08:58:00 Killing store-gw-1
08:58:00 Killing minio
--- PASS: TestCompactWithStoreGatewayWithPenaltyDedup (61.56s)
FAIL
FAIL	github.com/thanos-io/thanos/test/e2e	61.769s
?   	github.com/thanos-io/thanos/test/e2e/e2ethanos	[no test files]
FAIL

Is there something I'm missing? As I'm not familiar with the codebase and don't have too much time to sink into this I'd very much appreciate somebody pointing me in the right direction. Attaching the logs of both of my local runs to this post for good measure.

test-e2e.log
test-e2e-main.log

@yeya24 yeya24 requested a review from kakkoyun November 22, 2021 17:21
@yeya24
Copy link
Contributor

yeya24 commented Nov 22, 2021

Let me restart the CI. That is a known flaky test.

Copy link
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this. Amazing work!

I didn't know about this. Could you please fix all the files in the mixin? And add a section to the readme for this. It would be even more amazing if we could have a one-liner as part of jsonnet-lint action

thanos/Makefile

Lines 366 to 368 in de0e384

jsonnet-lint: $(JSONNET_LINT) ${JSONNET_VENDOR_DIR}
find . -name 'vendor' -prune -o -name '*.libsonnet' -print -o -name '*.jsonnet' -print | \
xargs -n 1 -- $(JSONNET_LINT) -J ${JSONNET_VENDOR_DIR}

mixin/alerts/alerts.libsonnet Outdated Show resolved Hide resolved
@itspngu
Copy link
Contributor Author

itspngu commented Dec 6, 2021

Hey @kakkoyun I've implemented (what I think) were the desired changes. Running make format and make docs as part of the PR preparation changed some files and now the commit history is a little messy. Let me know if there's anything else to do.

Update: I've removed the respective commits for make format and make docs in my latest rebase.

@itspngu itspngu requested a review from kakkoyun December 6, 2021 15:47
kakkoyun
kakkoyun previously approved these changes Dec 13, 2021
@kakkoyun kakkoyun enabled auto-merge (squash) December 13, 2021 08:31
auto-merge was automatically disabled December 18, 2021 10:11

Head branch was pushed to by a user without write access

@yeya24
Copy link
Contributor

yeya24 commented Dec 29, 2021

@itspngu Would you mind doing a rebase on main? I think there is some conflicts but GH doesn't identify them

….libsonnet for consistency

Signed-off-by: itspngu <hi@pngu.io>
…nnet for consistency

Signed-off-by: itspngu <hi@pngu.io>
@itspngu itspngu requested a review from kakkoyun January 5, 2022 15:18
@itspngu
Copy link
Contributor Author

itspngu commented Jan 5, 2022

@itspngu Would you mind doing a rebase on main? I think there is some conflicts but GH doesn't identify them

Done. Sorry for the delay. The conflict was in CHANGELOG.md.

@kakkoyun kakkoyun merged commit 4919eb8 into thanos-io:main Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants