-
Notifications
You must be signed in to change notification settings - Fork 183
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
Migration script v2 set remoteWrite remoteTimeout to 5s #1243
Migration script v2 set remoteWrite remoteTimeout to 5s #1243
Conversation
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.
It would be nice to check if value is already set, but LGTM
So I've tried a different approach with merging the temporary file that we work on with Maybe there's a way of doing this in |
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 think we shouldn't change this value at all.
Just check the user config and in case it is set we should display the INFO that we've changed the default value to 5.
Changed to just log if |
It's possible in |
echo | ||
info "kube-prometheus-stack.prometheus.prometheusSpec.remoteWrite.[*].remoteTimeout is set" | ||
info "Please note that we've set it by default to 5s in 2.0.0" | ||
fi |
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.
We should inform if remoteTImeout
is not set as well
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.
cc @perk-sumo
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.
or set it
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.
It is set by default in values.yaml, no need to add this into migrated conf file.
tests/upgrade_v2_script/static/prometheus_remote_write_timeout.input.yaml
Outdated
Show resolved
Hide resolved
a6c05d0
to
205f3bf
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.
👍
Description
Fill in your description here.
Testing performed