-
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
Bump Loki helm chart to 3.4.2 and Loki image to 2.7.0 #3921
Conversation
Loki image also get bumped to 2.7.0 The Loki helm chart get totally rewritten in version 3, so this commit also update the values file and dashboard
Since the label selector for the Loki StatefulSet changed in the version we embed compare to 124.0/123.0 we need to delete this StatefulSet before upgrading/downgrading it This commit should be reverted in `development/126.0`
Hello teddyandrieux,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: |
Since the Loki helm chart changed, we have some object that get added and removed, so let's clean those right after upgrade and downgrade. This commit should be reverted in `development/126.0`
003d85c
to
f1387c6
Compare
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 |
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 |
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
@@ -420,6 +420,7 @@ def __call__(self, parser, args, values, option_string=None): | |||
"networking.k8s.io/v1/Ingress", | |||
# Used by ServiceMonitor and other monitoring objects | |||
"monitoring.coreos.com/v1", | |||
"monitoring.coreos.com/v1/ServiceMonitor", |
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.
Sad that the Capabilities.APIVersions
can't deduce from the above that ServiceMonitor is indeed available 😕 I guess it's mostly working for running K8s clusters
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.
Yes exactly, this is just because we use helm template
with no cluster
relabelings: | ||
- replacement: metalk8s-logging/$1 | ||
sourceLabels: | ||
- job | ||
targetLabel: job | ||
- replacement: loki | ||
targetLabel: cluster |
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.
Oh, that's nice 👍 we'll stop seeing Loki in the etcd dashboard :)
{#- Due to a change of Loki StatefulSet labelSelector in 124.1.0, which is immutable field | ||
Manually delete the Loki StatefulSet object if dest_version < 124.1.0 | ||
NOTE: This logic can be removed in `development/126.0` #} | ||
{%- if salt.pkg.version_cmp(dest_version, '124.1.0') == -1 %} |
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.
I guess we could also check if the current version of the statefulset is higher or equal to 124.1 (in case we are re-running the orchestrate), but it's fine, downgrade doesn't need to be super optimized IMO
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.
Yes we could, but anyway, the state is idempotent so it will just do nothing since the object does not exist
(lol, too late) |
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 teddyandrieux. |
No description provided.