-
Notifications
You must be signed in to change notification settings - Fork 45
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
Replace metalk8s-loki storageclass with metalk8s one #2760
Replace metalk8s-loki storageclass with metalk8s one #2760
Conversation
We now use a single metalk8s storageclass instead of one per addon (e.g. metalk8s-loki). Refs: #2742
This is needed as Loki chart only allow to target a PV with its storageclass name and we now use a single storageclass for all addons. This change needs to be ported upstream so we can still upgrade Loki chart. Refs: #2742
This also adds a label on the PV so that we can now select the PV based on this label instead of the storageclass name. Refs: #2742
As for now, sometimes the test checking the logging pipeline fails because we're not waiting enough. We now wait 2 minutes, it should avoid flakies.
Hello alexandre-allard-scality,My role is to assist you with the merge of this Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
|
@@ -124,5 +124,9 @@ spec: | |||
requests: | |||
storage: {{ .Values.persistence.size | quote }} | |||
storageClassName: {{ .Values.persistence.storageClassName }} | |||
{{- if .Values.persistence.selector }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully, we don't forget this local patch during Loki chart upgrades...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why I opened an upstream PR, I hope it'll be merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha, I see the commit message now 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/approve |
Build failedThe build for commit did not succeed in branch improvement/2742-metalk8s-storageclass-loki. The following options are set: approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue None. Goodbye alexandre-allard-scality. |
Component: salt, docs, charts
Context: See #2742
Summary:
metalk8s-loki
storageclass withmetalk8s
oneAcceptance criteria:
Closes: #2742