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

Migrate to Thanos Objstore package at new location #2682

Merged
merged 7 commits into from
Oct 13, 2022
Merged

Conversation

bboreham
Copy link
Contributor

@bboreham bboreham commented Aug 9, 2022

grafana/thanos fork updated.

Requires update to grafana/e2e grafana/e2e#4 and grafana/dskit grafana/dskit#210.

Checklist

  • NA Tests updated
  • Documentation added
  • CHANGELOG.md updated

@bboreham bboreham force-pushed the thanos-objstore branch 2 times, most recently from 3ec7d33 to 356e6be Compare August 10, 2022 08:18
@pracucci
Copy link
Collaborator

I wanted to check if there's any logic difference, so I looked at:

git diff -w main... -- vendor/github.com/thanos-io/

There are changes to Azure client because of the Azure SDK upgrade, but logic looks unchanged, so LGTM so far.

@github-actions

This comment has been minimized.

@pracucci
Copy link
Collaborator

@bboreham What's the status of this? Are we ready to rebase and merge, or is there anything left?

@pracucci
Copy link
Collaborator

pracucci commented Oct 7, 2022

The CHANGELOG has just been cut to prepare for the next Mimir release. Please rebase main and eventually move the CHANGELOG entry added / updated in this PR to the top of the CHANGELOG document. Thanks!

CHANGELOG.md Outdated
@@ -11,6 +11,7 @@
* [CHANGE] Query-frontend: CLI flag `-query-frontend.align-querier-with-step` has been deprecated. Please use `-query-frontend.align-queries-with-step` instead. #2840
* [CHANGE] Distributor: change the default value of `-distributor.remote-timeout` to `2s` from `20s` and `-distributor.forwarding.request-timeout` to `2s` from `10s` to improve distributor resource usage when ingesters crash. #2728
* [CHANGE] Ingester: changed default setting for `-ingester.ring.readiness-check-ring-health` from `true` to `false`. #2953
* [CHANGE] Flag `-azure.msi-resource` no longer works. This setting is now made automatically by Azure. #???
Copy link
Collaborator

@pracucci pracucci Oct 13, 2022

Choose a reason for hiding this comment

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

Suggested change
* [CHANGE] Flag `-azure.msi-resource` no longer works. This setting is now made automatically by Azure. #???
* [CHANGE] Flag `-*.azure.msi-resource` has been deprecated and will be removed in Mimir 2.7. This setting is now made automatically by Azure. #2682

@@ -18,22 +19,22 @@ type Config struct {
ContainerName string `yaml:"container_name"`
Endpoint string `yaml:"endpoint_suffix"`
MaxRetries int `yaml:"max_retries" category:"advanced"`
MSIResource string `yaml:"msi_resource" category:"advanced"`
MSIResource string `yaml:"msi_resource" category:"advanced"` // deprecated
Copy link
Collaborator

@pracucci pracucci Oct 13, 2022

Choose a reason for hiding this comment

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

Suggested change
MSIResource string `yaml:"msi_resource" category:"advanced"` // deprecated
MSIResource string `yaml:"msi_resource" category:"advanced" doc:"hidden"` // TODO Remove in Mimir 2.7.

grafana/thanos fork updated.
Many changes to 3rd-party libraries.

Requires update to grafana/e2e and grafana/dskit.
Removed in upstream Azure library and marked as deprecated in
thanos-io/objstore, so lint fails if we use it.

Because we want to log if it is used, many places need to pass a logger
parameter through to `RegisterFlags`.

Also update the docs.
@bboreham bboreham marked this pull request as ready for review October 13, 2022 17:07
@bboreham bboreham requested review from osg-grafana and a team as code owners October 13, 2022 17:07
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Thanks Bryan!

Signed-off-by: Marco Pracucci <marco@pracucci.com>
@pracucci
Copy link
Collaborator

I've just pushed a go mod tidy.

@pracucci pracucci enabled auto-merge (squash) October 13, 2022 21:18
@pracucci pracucci merged commit 7777230 into main Oct 13, 2022
@pracucci pracucci deleted the thanos-objstore branch October 13, 2022 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants