-
Notifications
You must be signed in to change notification settings - Fork 37
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
Remove "Enable" functionality from autoscaling #238
Remove "Enable" functionality from autoscaling #238
Conversation
/retest |
d3f2f21
to
2b86343
Compare
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/38259d9e64f1420ba7d2b52f7186f1df ✔️ openstack-k8s-operators-content-provider SUCCESS in 29m 16s |
/retest |
recheck |
@@ -34,33 +34,6 @@ import ( | |||
autoscaling "github.com/openstack-k8s-operators/telemetry-operator/pkg/autoscaling" | |||
) | |||
|
|||
func (r *AutoscalingReconciler) reconcileDisabledPrometheus( |
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.
Shouldn't this be kept as we can disable prometheus independently using the deployPrometheus: false switch?
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.
Nice catch. I believe I deleted the function, because I determined, that it's called only from the now deleted reconcileDisabled. This was the original intent of the naming of the function, that reconcileDisabled would call <prometheus/aodh/...>Disabled. I'll take a further look later today and test how the deployPrometheus: false behaves with this change and I'll get to you later.
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.
The deployPrometheus switch is handled in the reconcileNormalPrometheus:
if instance.Spec.Prometheus.DeployPrometheus { |
Each time the autoscaling runs through the reconciling loop, it checks the switch and it either creates the prometheus or deletes it (the deleting code is a few lines further in the "else" part). This code stays without change. The reconcileDisabledPrometheus was there to handle only the autoscaling switch enabled: false
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jlarriba, vyzigold The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
First introduced with Merge pull request openstack-k8s-operators#238 from vyzigold/remove_enable_autoscaling The changes to the telemetry-operator.clusterserviceversion.yaml were left in the repo after running make bundle. This commit updated the repo with the changes so that after make bundle there are not leftover changes left in local repo. Signed-off-by: Jon Schlueter <[email protected]>
This removes the enable/disable functionality from autoscaling. This functionality fill be added to the telemetry resource in a future PR.
Jira: OSPRH-1350