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

operator: Move Loki operand from v2.6.1 to main-ec0bf70 #7298

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

periklis
Copy link
Collaborator

What this PR does / why we need it:
Upgrade the operand Loki from v2.6.1 temporarily to main-ec0bf70 to include 2.7.0 targeted features for the operator development:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

@periklis periklis self-assigned this Sep 29, 2022
@periklis periklis requested a review from a team as a code owner September 29, 2022 17:34
@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0.1%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

Copy link
Collaborator

@xperimental xperimental left a comment

Choose a reason for hiding this comment

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

Sorry, did not spot the wrong port in the first pass. Already tested with the changed port, but it does not work with the changed port out of the box as well, because it can not verify the CA certificate. Seems like the clients to the compactor also need the other TLS options if TLS is enabled.

Example error:

loki-querier ts=2022-10-04T09:46:26.347308744Z caller=spanlogger.go:80 user=application level=error msg="failed loading deletes for user" err="Get \"https://lokistack-dev-compactor-http.openshift-logging.svc.cluster.local:3100/loki/api/v1/delete\": x509: certificate signed by unknown authority"

@@ -79,6 +79,11 @@ func ConfigOptions(opt Options) config.Options {
Stack: opt.Stack,
Namespace: opt.Namespace,
Name: opt.Name,
Compactor: config.Address{
FQDN: fqdn(NewCompactorHTTPService(opt).GetName(), opt.Namespace),
Port: grpcPort,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I did not spot this the first time.

Suggested change
Port: grpcPort,
Port: httpPort,

@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0.4%
+               loki	0%

Copy link
Collaborator

@xperimental xperimental left a comment

Choose a reason for hiding this comment

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

Giving the compactor client the necessary TLS configuration needs a change in Loki. We decided to go ahead with this change anyway, even with the error logged, as this change is for unblocking other features and we are currently not using the deletion-endpoint of the compactor.

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.

3 participants