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

Upgrade Thanos #1257

Merged
merged 1 commit into from
Feb 25, 2022
Merged

Upgrade Thanos #1257

merged 1 commit into from
Feb 25, 2022

Conversation

aknuds1
Copy link
Contributor

@aknuds1 aknuds1 commented Feb 22, 2022

What this PR does

Upgrade Thanos to 2315a3a60a35.

This revision includes improvement to shipper error message when thanos.shipper.json is invalid.

Which issue(s) this PR fixes

Fixes #1241.

Checklist

  • [na] Tests updated
  • [na] Documentation added
  • [ na] CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@pracucci pracucci marked this pull request as draft February 22, 2022 08:33
@pracucci
Copy link
Collaborator

Moving to draft. We have to accurately check all Thanos changes we're going to vendor. Last time I checked I remember there was a change we may not want to get (but unfortunately I don't remember which one...).

@aknuds1
Copy link
Contributor Author

aknuds1 commented Feb 22, 2022

Moving to draft. We have to accurately check all Thanos changes we're going to vendor. Last time I checked I remember there was a change we may not want to get (but unfortunately I don't remember which one...).

@pracucci thanks, I was about to move to draft myself seeing it doesn't build.

@aknuds1 aknuds1 added the chore label Feb 22, 2022
@aknuds1 aknuds1 force-pushed the chore/upgrade-thanos branch 5 times, most recently from 0b7b3c1 to 37302ba Compare February 22, 2022 10:21
@@ -80,8 +80,7 @@ func TestGlobalMarkersBucket_DeleteShouldDeleteGlobalMarkIfBlockMarkerDoesntExis

// Delete block marker.
err := bkt.Delete(ctx, tc.blockMarker)
require.Error(t, err)
require.True(t, bkt.IsObjNotFoundErr(err))
require.NoError(t, err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The filesystem bucket implementation in Thanos has changed, to not treat missing files as an error.

Copy link
Member

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

We have to accurately check all Thanos changes we're going to vendor. Last time I checked I remember there was a change we may not want to get (but unfortunately I don't remember which one...).

I checked Thanos changes, and cannot identify the one we wouldn't want. PR looks good to me.

There is new support in Thanos S3 client to use AWS-SDK auth, which we may want to expose at some point, but it doesn't need to be in this PR.

pkg/storegateway/bucket_stores.go Outdated Show resolved Hide resolved
@aknuds1 aknuds1 marked this pull request as ready for review February 25, 2022 09:40
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
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.

Handle when shipper metadata file is non-JSON
3 participants