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.

Sign up for GitHub

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

Jump to bottom

Fix failing e2e test #7620

Merged
merged 1 commit into from
Aug 12, 2024
Merged

Fix failing e2e test #7620

merged 1 commit into from
Aug 12, 2024

Conversation

harry671003
Copy link
Contributor

@harry671003 harry671003 commented Aug 9, 2024 •
edited
Loading

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

Changes

There seems to be a bug in avalanche. When --metric-interval alone is set, no timeseries are returned and no write requests will be made by avalanche. See: write.go#L143

Using --series-interval and --sample-interval seems to fix the test.

I haven't had a chance to look at avalanche bug in depth. But this PR should unblock Thanos e2e tests.

Verification

  • CI Workflows should pass

@harry671003 harry671003 force-pushed the fix_e2e branch 6 times, most recently from 08f7728 to 0e6cb7f Compare August 12, 2024 17:03
@harry671003 harry671003 marked this pull request as ready for review August 12, 2024 17:26
@harry671003
Fix failing e2e test
7b9ce91 
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
@harry671003 harry671003 mentioned this pull request Aug 12, 2024
Open
saswatamcode
saswatamcode approved these changes Aug 12, 2024
View reviewed changes
Copy link
Member

@saswatamcode saswatamcode left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM nice catch! But to prevent this, we should probably pin the image we use here too?

edit: Avalanche only has a main tag, so can't really replace. So merging!

MichaHoffmann
MichaHoffmann approved these changes Aug 12, 2024
View reviewed changes
Copy link
Contributor

@MichaHoffmann MichaHoffmann left a comment

Choose a reason for hiding this comment

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

Thank you!

@saswatamcode saswatamcode merged commit 49617f4 into thanos-io:main Aug 12, 2024
20 checks passed
@harry671003 harry671003 deleted the fix_e2e branch August 12, 2024 18:15
saswatamcode pushed a commit to saswatamcode/thanos that referenced this pull request Aug 13, 2024
@harry671003 @saswatamcode
Fix failing e2e test (thanos-io#7620)
db4407d 
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
saswatamcode pushed a commit to saswatamcode/thanos that referenced this pull request Aug 13, 2024
@harry671003 @saswatamcode
Fix failing e2e test (thanos-io#7620)
d2f985a 
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>
saswatamcode added a commit that referenced this pull request Aug 13, 2024
@saswatamcode @cincinnat
@simonpasquier @harry671003
Cut patch release v0.36.1 (#7636)
99a5742 
* Proxy: Query goroutine leak when `store.response-timeout` is set (#7618)

time.AfterFunc() returns a time.Timer object whose C field is nil,
accroding to the documentation. A goroutine blocks forever on reading
from a `nil` channel, leading to a goroutine leak on random slow
queries.

Signed-off-by: Mikhail Nozdrachev <[email protected]>

* pkg/clientconfig: fix TLS configs with only CA (#7634)

065e3dd introduced a regression: TLS configurations for Thanos Ruler
query and alerting with only a CA file failed to load.

For instance, the following snippet is a valid query configuration:

```
- static_configs:
  - prometheus.example.com:9090
  scheme: https
  http_config:
    tls_config:
      ca_file: /etc/ssl/cert.pem
```

The test fixtures (CA, certificate and key files) are copied from
prometheus/common and are valid until 2072.

Signed-off-by: Simon Pasquier <[email protected]>

* Cut patch release v0.36.1

Signed-off-by: Saswata Mukherjee <[email protected]>

* Fix failing e2e test (#7620)

Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>

---------

Signed-off-by: Mikhail Nozdrachev <[email protected]>
Signed-off-by: Simon Pasquier <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Co-authored-by: Mikhail Nozdrachev <[email protected]>
Co-authored-by: Simon Pasquier <[email protected]>
Co-authored-by: Harry John <[email protected]>
saswatamcode added a commit that referenced this pull request Aug 14, 2024
@saswatamcode @MichaHoffmann
@fpetkovski @cincinnat @simonpasquier @harry671003
Merge release 0.36.1 to main (#7639)
4e08c20 
* CHANGELOG: Mark 0.36 as in progress

Signed-off-by: Michael Hoffmann <[email protected]>

* Cut release candidate v0.36.0-rc.0 (#7490)

Signed-off-by: Michael Hoffmann <[email protected]>

* Cut release candidate 0.36.0 rc.1 (#7510)

* *: fix server grpc histograms (#7493)

Signed-off-by: Michael Hoffmann <[email protected]>

* Close endpoints after the gRPC server has terminated (#7509)

Endpoints are currently closed as soon as we receive a SIGTERM or SIGINT.
This causes in-flight queries to get cancelled since outgoing connections
get closed instantly.

This commit moves the endpoints.Close call after the grpc server shutdown
to make sure connections are available as long as the server is running.

Signed-off-by: Filip Petkovski <[email protected]>

* Cut release candidate v0.36.0-rc.1

Signed-off-by: Michael Hoffmann <[email protected]>

---------

Signed-off-by: Michael Hoffmann <[email protected]>
Signed-off-by: Filip Petkovski <[email protected]>
Co-authored-by: Filip Petkovski <[email protected]>

* Cut release v0.36.0 (#7578)

Signed-off-by: Michael Hoffmann <[email protected]>

* Cut patch release `v0.36.1` (#7636)

* Proxy: Query goroutine leak when `store.response-timeout` is set (#7618)

time.AfterFunc() returns a time.Timer object whose C field is nil,
accroding to the documentation. A goroutine blocks forever on reading
from a `nil` channel, leading to a goroutine leak on random slow
queries.

Signed-off-by: Mikhail Nozdrachev <[email protected]>

* pkg/clientconfig: fix TLS configs with only CA (#7634)

065e3dd introduced a regression: TLS configurations for Thanos Ruler
query and alerting with only a CA file failed to load.

For instance, the following snippet is a valid query configuration:

```
- static_configs:
  - prometheus.example.com:9090
  scheme: https
  http_config:
    tls_config:
      ca_file: /etc/ssl/cert.pem
```

The test fixtures (CA, certificate and key files) are copied from
prometheus/common and are valid until 2072.

Signed-off-by: Simon Pasquier <[email protected]>

* Cut patch release v0.36.1

Signed-off-by: Saswata Mukherjee <[email protected]>

* Fix failing e2e test (#7620)

Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>

---------

Signed-off-by: Mikhail Nozdrachev <[email protected]>
Signed-off-by: Simon Pasquier <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Co-authored-by: Mikhail Nozdrachev <[email protected]>
Co-authored-by: Simon Pasquier <[email protected]>
Co-authored-by: Harry John <[email protected]>

---------

Signed-off-by: Michael Hoffmann <[email protected]>
Signed-off-by: Filip Petkovski <[email protected]>
Signed-off-by: Mikhail Nozdrachev <[email protected]>
Signed-off-by: Simon Pasquier <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Co-authored-by: Michael Hoffmann <[email protected]>
Co-authored-by: Filip Petkovski <[email protected]>
Co-authored-by: Mikhail Nozdrachev <[email protected]>
Co-authored-by: Simon Pasquier <[email protected]>
Co-authored-by: Harry John <[email protected]>
hczhu-db pushed a commit to databricks/thanos that referenced this pull request Aug 22, 2024
@saswatamcode @cincinnat
@simonpasquier @harry671003 @hczhu-db
Cut patch release v0.36.1 (thanos-io#7636)
3fc9e64 
* Proxy: Query goroutine leak when `store.response-timeout` is set (thanos-io#7618)

time.AfterFunc() returns a time.Timer object whose C field is nil,
accroding to the documentation. A goroutine blocks forever on reading
from a `nil` channel, leading to a goroutine leak on random slow
queries.

Signed-off-by: Mikhail Nozdrachev <[email protected]>

* pkg/clientconfig: fix TLS configs with only CA (thanos-io#7634)

065e3dd introduced a regression: TLS configurations for Thanos Ruler
query and alerting with only a CA file failed to load.

For instance, the following snippet is a valid query configuration:

```
- static_configs:
  - prometheus.example.com:9090
  scheme: https
  http_config:
    tls_config:
      ca_file: /etc/ssl/cert.pem
```

The test fixtures (CA, certificate and key files) are copied from
prometheus/common and are valid until 2072.

Signed-off-by: Simon Pasquier <[email protected]>

* Cut patch release v0.36.1

Signed-off-by: Saswata Mukherjee <[email protected]>

* Fix failing e2e test (thanos-io#7620)

Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>

---------

Signed-off-by: Mikhail Nozdrachev <[email protected]>
Signed-off-by: Simon Pasquier <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Co-authored-by: Mikhail Nozdrachev <[email protected]>
Co-authored-by: Simon Pasquier <[email protected]>
Co-authored-by: Harry John <[email protected]>
GiedriusS pushed a commit to vinted/thanos that referenced this pull request Aug 28, 2024
@harry671003 @GiedriusS
Fix failing e2e test (thanos-io#7620)
f1608c0 
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
@harry671003 harry671003 mentioned this pull request Jan 15, 2025
Draft
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaHoffmann MichaHoffmann MichaHoffmann approved these changes

@yeya24 yeya24 yeya24 approved these changes

@saswatamcode saswatamcode saswatamcode approved these changes

Assignees
No one assigned
Labels
size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@harry671003 @MichaHoffmann @yeya24 @saswatamcode