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

fix(ksonnet): don't depend on specific k8s version #2525

Merged
merged 3 commits into from
Aug 20, 2020

Conversation

Duologic
Copy link
Member

What this PR does / why we need it:
Loki should not depend on a specific k8s version (1.14), this would make upgrading clusters much harder.

As no imports are happening directly in this library, this change should be a no-op.

@Duologic Duologic marked this pull request as ready for review August 20, 2020 10:15
@codecov-commenter
Copy link

codecov-commenter commented Aug 20, 2020

Codecov Report

Merging #2525 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2525      +/-   ##
==========================================
+ Coverage   63.56%   63.59%   +0.02%     
==========================================
  Files         163      163              
  Lines       14305    14305              
==========================================
+ Hits         9093     9097       +4     
+ Misses       4499     4497       -2     
+ Partials      713      711       -2     
Impacted Files Coverage Δ
pkg/logql/evaluator.go 92.88% <0.00%> (+0.40%) ⬆️
pkg/ingester/transfer.go 64.82% <0.00%> (+1.37%) ⬆️

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.

Thanks!

Copy link
Member

@owen-d owen-d left a comment

Choose a reason for hiding this comment

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

I unfortunately think removing this breaks functionality for end users. Can you try installing from a fresh tanka according to the docs? I think this stems from the changes in https://github.com/grafana/loki/pull/2460/files which don't play nicely with the default versions used by tanka, but which we override internally. We'll also need to update the blurb in our docs (the prior deploying link), but I think this is important to sort out. Thanks for looking into it @Duologic

@Duologic
Copy link
Member Author

Completely right, @owen-d. I've updated the docs accordingly.

We are actively working to get k8s-alpha to GA.

@Duologic Duologic force-pushed the duologic/no_k8s_version_lock branch from b6a296a to 647a1a5 Compare August 20, 2020 12:54
@Duologic
Copy link
Member Author

Removed docs and added compatibility for envVar.

Copy link
Member

@owen-d owen-d left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@owen-d owen-d merged commit b29bc19 into master Aug 20, 2020
@owen-d owen-d deleted the duologic/no_k8s_version_lock branch August 20, 2020 13:11
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.

4 participants