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

ksonnet: do not deploy table manager when shipper is in-use #11020

Merged
merged 6 commits into from
Oct 27, 2023

Conversation

ashwanthgoli
Copy link
Contributor

@ashwanthgoli ashwanthgoli commented Oct 25, 2023

What this PR does / why we need it:
Do not generate table manager manifests if shipper is being used.
table manager is a deprecated target and is not recommended when using tsdb or boltdb-shipper indexes.
Compactor is already bunlded along with shipper and handles retention, compaction for the tsdb and boltb-shipper indexes

Which issue(s) this PR fixes:
Fixes #10943

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
    • If the change is worth mentioning in the release notes, add add-to-release-notes label
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory.

@ashwanthgoli ashwanthgoli marked this pull request as ready for review October 25, 2023 08:17
@ashwanthgoli ashwanthgoli requested a review from a team as a code owner October 25, 2023 08:17
Copy link
Contributor

@MichelHollands MichelHollands left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@kavirajk kavirajk left a comment

Choose a reason for hiding this comment

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

One minor nit about changelog. everything else LGTM 👍

CHANGELOG.md Outdated
@@ -69,6 +69,8 @@

#### Jsonnet

* [11020](https://github.com/grafana/loki/pull/11020) **ashwanthgoli**: Do not generate table-manager manifests if shipper store is in-use.
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems very generic. May be let's make sure it's for Loki ksonnet deployment?

@ashwanthgoli ashwanthgoli merged commit 1781d7c into main Oct 27, 2023
@ashwanthgoli ashwanthgoli deleted the ashwanth/ksonnet-donot-use-table-manager branch October 27, 2023 06:08
rhnasc pushed a commit to inloco/loki that referenced this pull request Apr 12, 2024
…11020)

**What this PR does / why we need it**:
Do not generate table manager manifests if shipper is being used.
table manager is a deprecated target and is not recommended when using
`tsdb` or `boltdb-shipper` indexes.
Compactor is already bunlded along with shipper and handles retention,
compaction for the `tsdb` and `boltb-shipper` indexes


**Which issue(s) this PR fixes**:
Fixes [grafana#10943](grafana#10943)

**Special notes for your reviewer**:

**Checklist**
- [x] Reviewed the
[`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md)
guide (**required**)
- [ ] Documentation added
- [ ] Tests updated
- [x] `CHANGELOG.md` updated
- [ ] If the change is worth mentioning in the release notes, add
`add-to-release-notes` label
- [ ] Changes that require user attention or interaction to upgrade are
documented in `docs/sources/setup/upgrade/_index.md`
- [ ] For Helm chart changes bump the Helm chart version in
`production/helm/loki/Chart.yaml` and update
`production/helm/loki/CHANGELOG.md` and
`production/helm/loki/README.md`. [Example
PR](grafana@d10549e)
- [ ] If the change is deprecating or removing a configuration option,
update the `deprecated-config.yaml` and `deleted-config.yaml` files
respectively in the `tools/deprecated-config-checker` directory. <!--
TODO(salvacorts): Add example PR -->
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.

JSonnet always deploys a table_manager
3 participants