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

Add datasource for each Loki instance #3681

Merged

Conversation

alexandre-allard
Copy link
Contributor

Component: salt,logs

Context:

Summary:

Acceptance criteria:

@alexandre-allard alexandre-allard requested a review from a team as a code owner January 25, 2022 09:10
@bert-e

This comment has been minimized.

@bert-e

This comment has been minimized.

@bert-e

This comment has been minimized.

Copy link
Collaborator

@TeddyAndrieux TeddyAndrieux left a comment

Choose a reason for hiding this comment

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

LGTM

Can you just add a changelog, and maybe we want to add a simple test just to be sure the "loki-x" service properly redirect to Loki
(I think about maybe one day a label in the Loki statefulset that change so the extra service we deploy does not get updated and we do not catch it)

@alexandre-allard alexandre-allard force-pushed the improvement/add-loki-datasources-for-each-instance branch from 331964e to ad487fa Compare January 31, 2022 14:35
@bert-e

This comment has been minimized.

CHANGELOG.md Outdated Show resolved Hide resolved
@alexandre-allard alexandre-allard force-pushed the improvement/add-loki-datasources-for-each-instance branch from ad487fa to faf2e1a Compare January 31, 2022 15:06
@alexandre-allard
Copy link
Contributor Author

/reset

@bert-e

This comment has been minimized.

@bert-e

This comment has been minimized.

@bert-e

This comment has been minimized.

@alexandre-allard alexandre-allard force-pushed the improvement/add-loki-datasources-for-each-instance branch from faf2e1a to 762ed00 Compare February 1, 2022 09:05
@bert-e

This comment has been minimized.

@alexandre-allard
Copy link
Contributor Author

/reset

@bert-e

This comment has been minimized.

@bert-e

This comment has been minimized.

@bert-e

This comment has been minimized.

TeddyAndrieux
TeddyAndrieux previously approved these changes Feb 1, 2022
Copy link
Collaborator

@TeddyAndrieux TeddyAndrieux left a comment

Choose a reason for hiding this comment

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

Ok for me !

@bert-e
Copy link
Contributor

bert-e commented Feb 1, 2022

Conflict

There is a conflict between your branch improvement/add-loki-datasources-for-each-instance and the
destination branch development/2.11.

Please resolve the conflict on the feature branch (improvement/add-loki-datasources-for-each-instance).

git fetch && \
git checkout origin/improvement/add-loki-datasources-for-each-instance && \
git merge origin/development/2.11

Resolve merge conflicts and commit

git push origin HEAD:improvement/add-loki-datasources-for-each-instance

These services allow to communicate which
each Pod individually, this is needed to
deploy datasources to target a specific
Loki instance.
This is a workaround to allow user to access a
specific Loki instance in case one would not
have all the logs (disk loss or node down for
a long period of time). This way the user can
select a healthy instance having all the data.
Display a message on Loki and Logs Grafana
dashboards to tell the user he can change
the datasource to target a specific Loki instance
This test is to ensure we can access a specific Loki
instance through the Services.
@bert-e
Copy link
Contributor

bert-e commented Feb 1, 2022

History mismatch

Merge commit #bd94a75082ab8717356cff4c088fcccc3fb885e1 on the integration branch
w/123.0/improvement/add-loki-datasources-for-each-instance is merging a branch which is neither the current
branch improvement/add-loki-datasources-for-each-instance nor the development branch
development/123.0.

It is likely due to a rebase of the branch improvement/add-loki-datasources-for-each-instance and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

@alexandre-allard
Copy link
Contributor Author

/reset

@bert-e
Copy link
Contributor

bert-e commented Feb 1, 2022

Reset complete

I have successfully deleted this pull request's integration branches.

@bert-e
Copy link
Contributor

bert-e commented Feb 1, 2022

Integration data created

I have created the integration data for the additional destination branches.

The following branches will NOT be impacted:

  • development/2.0
  • development/2.1
  • development/2.10
  • development/2.2
  • development/2.3
  • development/2.4
  • development/2.5
  • development/2.6
  • development/2.7
  • development/2.8
  • development/2.9

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

@bert-e
Copy link
Contributor

bert-e commented Feb 1, 2022

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

@alexandre-allard
Copy link
Contributor Author

/approve

@bert-e
Copy link
Contributor

bert-e commented Feb 1, 2022

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

  • ✔️ development/2.11

  • ✔️ development/123.0

The following branches will NOT be impacted:

  • development/2.0
  • development/2.1
  • development/2.10
  • development/2.2
  • development/2.3
  • development/2.4
  • development/2.5
  • development/2.6
  • development/2.7
  • development/2.8
  • development/2.9

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

  • Any commit you add on the source branch will trigger a new cycle after the
    current queue is merged.
  • Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

@bert-e
Copy link
Contributor

bert-e commented Feb 1, 2022

I have successfully merged the changeset of this pull request
into targetted development branches:

  • ✔️ development/2.11

  • ✔️ development/123.0

The following branches have NOT changed:

  • development/2.0
  • development/2.1
  • development/2.10
  • development/2.2
  • development/2.3
  • development/2.4
  • development/2.5
  • development/2.6
  • development/2.7
  • development/2.8
  • development/2.9

Please check the status of the associated issue None.

Goodbye alexandre-allard-scality.

@bert-e bert-e merged commit a1c93eb into development/2.11 Feb 1, 2022
@bert-e bert-e deleted the improvement/add-loki-datasources-for-each-instance branch February 1, 2022 18:57
TeddyAndrieux added a commit that referenced this pull request Feb 28, 2022
Fix a bug that deletes the Loki service instance post-upgrade when it
shouldn't.

This Loki service instance and grafana datasources are deployed to
mitigate a Loki limitation when working with multi instances

See: abc293a
See: #3681
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.

4 participants