Skip to content

feat: update logcli so it tries to load the latest version of the schemaconfig #11852

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

Merged
merged 20 commits into from
Mar 15, 2024

Conversation

MichelHollands
Copy link
Contributor

@MichelHollands MichelHollands commented Feb 1, 2024

What this PR does / why we need it:

Cloud Logs exporter will write new versions of the schema config in separate files now. This means it will work with object stores that have a policy of write once (ie no updates). Logcli is changed in this PR to take that into account.

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. Example PR

Sorry, something went wrong.

Signed-off-by: Michel Hollands <michel.hollands@gmail.com>
MichelHollands and others added 6 commits February 1, 2024 17:16
Signed-off-by: Michel Hollands <michel.hollands@gmail.com>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands MichelHollands marked this pull request as ready for review February 6, 2024 13:58
@MichelHollands MichelHollands requested a review from a team as a code owner February 6, 2024 13:58
Copy link
Contributor

@DylanGuedes DylanGuedes left a comment

Choose a reason for hiding this comment

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

lgtm, just 2 nits

MichelHollands and others added 3 commits February 8, 2024 13:35
Co-authored-by: Dylan Guedes <djmgguedes@gmail.com>
Co-authored-by: Dylan Guedes <djmgguedes@gmail.com>
@MichelHollands MichelHollands changed the title Update logcli so it tries to load the latest version of the schemaconfig feat: update logcli so it tries to load the latest version of the schemaconfig Mar 13, 2024
Comment on lines 399 to 406
searchFor := fmt.Sprintf("%s.yaml", schemaConfigFilename) // schemaconfig.yaml for backwards compatibility
loadedSchema, err := LoadSchemaUsingObjectClient(client, searchFor)
if err == nil {
return loadedSchema, nil
}
if err != errNotExists && err != nil {
return nil, err
}
Copy link
Member

Choose a reason for hiding this comment

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

we need to move this fallback logic to the end of the function.

let's say if we do not have any files with the naming convention schemaconfig-tenant.yaml or schemaconfig-tenant-N.yaml, then we fallback and load schemaconfig.yaml.

with the current implementation, it will always load schemaconfig.yaml even if newer files exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. This has been moved to the back. Tests have been added as well to check the cases of 1) a single legacy schemaconfig and 2) both a new style one and a legacy one.

Tested it with running logcli against a GCP bucket with bucket retention policy set.

MichelHollands and others added 4 commits March 15, 2024 09:24
Signed-off-by: Michel Hollands <michel.hollands@gmail.com>
Signed-off-by: Michel Hollands <michel.hollands@gmail.com>
MichelHollands and others added 4 commits March 15, 2024 11:06
Signed-off-by: Michel Hollands <michel.hollands@gmail.com>
Signed-off-by: Michel Hollands <michel.hollands@gmail.com>
@MichelHollands MichelHollands merged commit 4ce5fa8 into main Mar 15, 2024
11 checks passed
@MichelHollands MichelHollands deleted the update_logcli_to_use_schemaconfig-x branch March 15, 2024 13:43
rhnasc pushed a commit to inloco/loki that referenced this pull request Apr 12, 2024
…emaconfig (grafana#11852)

Signed-off-by: Michel Hollands <michel.hollands@gmail.com>
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
Co-authored-by: Dylan Guedes <djmgguedes@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.

None yet

3 participants