Create hpa from CR #1019
Create hpa from CR #1019
Conversation
This PR add make target to deploy/undeploy prometheus instance, and its related service monitor, secrets. --------- Co-authored-by: Roy Paulin <[email protected]>
roypaulin
requested review from
cchen-vertica,
fenic-fawkes,
HaoYang0000,
qindotguan and
LiboYu2
as code owners
|
@LiboYu2 @qindotguan @HaoYang0000 thoughts, comments, questions? |
I will take a look this afternoon. |
api/v1/version.go
Outdated
| @@ -58,6 +58,8 @@ const ( | |||
| // Starting in v24.1.0, the default deployment method supported changes from admintools to vclusterops | |||
| // for official releases of vertica-k8s images | |||
| VcluseropsAsDefaultDeploymentMethodMinVersion = "v24.1.0" | |||
| // The min version that exposed prometheus metrics through the http service | |||
| PrometheusMetricsMinVersion = "v12.0.4" | |||
There was a problem hiding this comment.
So the new auto-scaler can work with both admintools and vcluster?
There was a problem hiding this comment.
To be honest I was not sure what must be the min version. Technically you can use the new field to set non prometheus metrics like cpu/mem and it would work. But it is not going to work with prometheus metrics so I will pit 24.1.0 instead.
| // +kubebuilder:Minimum:=0 | ||
| // +operator-sdk:csv:customresourcedefinitions:type=spec | ||
| // The miminum number of pods when scaling. | ||
| MinReplicas int32 `json:"minReplicas,omitempty"` | ||
| MinReplicas *int32 `json:"minReplicas,omitempty"` |
There was a problem hiding this comment.
Why are we using different types for "MinReplicas" and "MaxReplicas"?
There was a problem hiding this comment.
Because that is how it is in the hpa. I decided to keep the same types as in the hpa.
| @@ -244,7 +243,39 @@ func MakeVAS() *VerticaAutoscaler { | |||
| } | |||
| } | |||
|
|
|||
| func MakeVASWithMetrics() *VerticaAutoscaler { | |||
There was a problem hiding this comment.
Add a comment for this function. Is this function only used for the test?
There was a problem hiding this comment.
Yes, it is only used for testing.
| @@ -863,6 +864,26 @@ func makeScrutinizeInitContainers(vscr *v1beta1.VerticaScrutinize, vdb *vapi.Ver | |||
| return cnts | |||
| } | |||
|
|
|||
| func BuildHorizontalPodAutoscaler(nm types.NamespacedName, vas *v1beta1.VerticaAutoscaler) *autoscalingv2.HorizontalPodAutoscaler { | |||
There was a problem hiding this comment.
Add a comment for this function.
pkg/controllers/vas/hpareconciler.go
Outdated
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| ) | ||
|
|
||
| // RefreshSelectorReconciler is a reconciler to update the pod selector in the |
pkg/controllers/vas/hpareconciler.go
Outdated
| return &HorizontalPodAutoscalerReconciler{VRec: v, Vas: vas, Log: log.WithName("HorizontalPodAutoscalerReconciler")} | ||
| } | ||
|
|
||
| // Reconcile will handle updating the selector in the status portion of a VerticaAutoscaler |
There was a problem hiding this comment.
Where does the reconcile() update the status portion?
There was a problem hiding this comment.
That' a copy/paste error. I will update the comment
| var _ = Describe("subclusterresize_reconcile", func() { | ||
| ctx := context.Background() | ||
|
|
||
| It("should requeue if VerticaDB doesn't exist", func() { |
There was a problem hiding this comment.
Update the description of this test.
A new field
CustomAutoscalerwas added. When set, there is a reconciler that will create an hpa whose values will come from that field. If that field change, the hpa is updated to reflect the changes. If the hpa is deleted, the reconciler will make sure it is recreated.