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

logs: bump jsonnet dep to start using ConfigMap and Services from loki.mixins #450

Merged
merged 3 commits into from
Apr 17, 2023

Conversation

@JoaoBraveCoding
Copy link
Contributor Author

Needs observatorium/observatorium#515

Copy link
Contributor

@periklis periklis left a comment

Choose a reason for hiding this comment

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

Looks good to me just one entry in the config that should stay as is namely the index_queries_cache and a couple of nits.

@@ -750,7 +737,6 @@ objects:
"limits_config":
"cardinality_limit": 100000
"creation_grace_period": "10m"
"deletion_mode": "disabled"
Copy link
Contributor

Choose a reason for hiding this comment

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

Although disabled is the default, I suggest to keep this here because the Deletion API has caused numerous issues upstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-added and left a not to move this to observatorium/observatorium since I think it makes more sense to live there

Comment on lines 853 to 859
"index_queries_cache_config":
"memcached":
"batch_size": 100
"parallelism": 100
"memcached_client":
"addresses": "dns+observatorium-loki-index-query-cache.${NAMESPACE}.svc.cluster.local:11211"
"consistent_hash": true
Copy link
Contributor

Choose a reason for hiding this comment

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

This should stay here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-added this, only defaults were removed

services/observatorium-logs-template-overwrites.libsonnet Outdated Show resolved Hide resolved
services/observatorium-logs-template-overwrites.libsonnet Outdated Show resolved Hide resolved
@JoaoBraveCoding JoaoBraveCoding force-pushed the bump-loki-logs branch 4 times, most recently from 6114895 to 8d91b31 Compare April 11, 2023 09:48
@JoaoBraveCoding JoaoBraveCoding force-pushed the bump-loki-logs branch 3 times, most recently from c6587ad to 67abc19 Compare April 13, 2023 17:35
@JoaoBraveCoding
Copy link
Contributor Author

Needs observatorium/observatorium#520

@JoaoBraveCoding JoaoBraveCoding force-pushed the bump-loki-logs branch 3 times, most recently from 81779f6 to 3c6a9aa Compare April 14, 2023 08:34
Copy link
Contributor

@periklis periklis left a comment

Choose a reason for hiding this comment

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

LGTM let's ensure we have a clear path to overcome the two rollout issues we found on rhobs-testing and this is good to go:

  1. How to rollout the serviceName change on StatefulSets on app-interface?
  2. How to keep the old http service referenced in observatorium-template.yaml in tact to minimize rollout dependencies.

@JoaoBraveCoding
Copy link
Contributor Author

JoaoBraveCoding commented Apr 17, 2023

How to rollout the serviceName change on StatefulSets on app-interface?

We should be able to leverage app-sre/qontract-reconcile#1627

How to keep the old http service referenced in observatorium-template.yaml intact to minimize rollout dependencies.

To solve this from my POV we would:

  1. Include in this PR the old services referenced in observatorium-template.yaml
  2. Deploy
  3. Create a new PR where we update the services in observatorium-template.yaml
  4. Deploy
  5. Create a new PR where we remove the old services
  6. Deploy

@periklis
Copy link
Contributor

How to rollout the serviceName change on StatefulSets on app-interface?

We should be able to leverage app-sre/qontract-reconcile#1627

ACK

How to keep the old http service referenced in observatorium-template.yaml intact to minimize rollout dependencies.

To solve this from my POV we would:

1. Include in this PR the old services referenced in `observatorium-template.yaml`

2. Deploy

3. Create a new PR where we update the services in `observatorium-template.yaml`

4. Deploy

5. Create a new PR where we remove the old services

6. Deploy

ACK

@JoaoBraveCoding
Copy link
Contributor Author

Once we have observatorium/observatorium#521 merged we will be able to progress on this

deployment.

First we will deploy the new services while keeping the old
Then we will update observatorium api to change to the new services
Finally we will then remove the old services

Signed-off-by: JoaoBraveCoding <[email protected]>
@periklis periklis merged commit 8d6ebd7 into rhobs:main Apr 17, 2023
JoaoBraveCoding added a commit that referenced this pull request Apr 18, 2023
JoaoBraveCoding added a commit that referenced this pull request Apr 19, 2023
JoaoBraveCoding added a commit that referenced this pull request Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants