Skip to content
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.

Sign up for GitHub

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

initial draft for review before generating code #1015

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

initial draft for review before generating code #1015

Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
95fd882
initial draft for review before generating code
LiboYu2 Jan 2, 2025
05e0725
address concerns from code review
LiboYu2 Jan 2, 2025
7229e4e
use pointer for int32 and fix case issue
LiboYu2 Jan 2, 2025
540bd53
new changes made per design doc update
LiboYu2 Jan 2, 2025
7501146
changes made per code review
LiboYu2 Jan 3, 2025
abcb046
renamed custom
LiboYu2 Jan 3, 2025
45cc307
fix a comment error
LiboYu2 Jan 6, 2025
e9d6a72
address review comments
LiboYu2 Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions api/v1beta1/verticaautoscaler_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ limitations under the License.
package v1beta1

import (
autoscalingv2 "k8s.io/api/autoscaling/v2"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -77,6 +78,53 @@ type VerticaAutoscalerSpec struct {
// left as zero. It will get initialized in the operator and then modified
// via the /scale subresource.
TargetSize int32 `json:"targetSize"`

// +kubebuilder:validation:Optional
// +operator-sdk:csv:customresourcedefinitions:type=spec
// This struct allows customization of autoscaling. Custom metrics can be used instead of the memory and cpu metrics.
// The scaling behavior can also be customized to meet different performance requirements. The maximum and mininum of
// sizes of the replica sets can be specified to limit the use of resources.
CustomAutoscalerSpec *CustomAutoscalerSpec `json:"customAutoscalerSpec,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
CustomAutoscalerSpec *CustomAutoscalerSpec `json:"customAutoscalerSpec,omitempty"`
CustomAutoscaler *CustomAutoscalerSpec `json:"customAutoscalerSpec,omitempty"`

}

// CustomAutoscalerSpec customizes VerticaAutoscaler
type CustomAutoscalerSpec struct {

// +kubebuilder:Minimum:=0
// +kubebuilder:validation:Optional
// +operator-sdk:csv:customresourcedefinitions:type=spec
// the mininum size of replica set
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// the mininum size of replica set
// The miminum number of pods when scaling.

MinReplicas *int32 `json:"minReplicas"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
MinReplicas *int32 `json:"minReplicas"`
MinReplicas *int32 `json:"minReplicas,omitempty"`


// +kubebuilder:validation:Optional
Copy link
Collaborator

@roypaulin roypaulin Jan 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// +kubebuilder:validation:Optional
// +kubebuilder:Minimum:=0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// +kubebuilder:validation:Optional

// +kubebuilder:Minimum:=0
// +operator-sdk:csv:customresourcedefinitions:type=spec
// the maximum size of replica set
MaxReplicas *int32 `json:"maxReplicas"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// +operator-sdk:csv:customresourcedefinitions:type=spec
// the maximum size of replica set
MaxReplicas *int32 `json:"maxReplicas"`
// +operator-sdk:csv:customresourcedefinitions:type=spec
// The maximum number of pods when scaling.
MaxReplicas int32 `json:"maxReplicas"`

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
MaxReplicas *int32 `json:"maxReplicas"`
MaxReplicas *int32 `json:"maxReplicas,omitempty"`


// +kubebuilder:validation:Optional
// +operator-sdk:csv:customresourcedefinitions:type=spec
// the custom metric and increment to be used for autocaling
Metrics []MetricDefinition `json:"metrics"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// the custom metric and increment to be used for autocaling
Metrics []MetricDefinition `json:"metrics"`
// The custom metric to be used for autocaling.
Metrics []MetricDefinition `json:"metrics,omitempty"`


// +kubebuilder:validation:Optional
// +operator-sdk:csv:customresourcedefinitions:type=spec
// define how the autocaler handles the scaleup and scaledown
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// define how the autocaler handles the scaleup and scaledown
// Specifies the scaling behavior for both scale up and down.

Behavior *autoscalingv2.HorizontalPodAutoscalerBehavior `json:"behavior,omitempty"`
}

// MetricDefinition defines increment and metric to be used for autoscaling
type MetricDefinition struct {

// +kubebuilder:validation:Optional
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// +kubebuilder:validation:Optional
// +kubebuilder:validation:Optional
// +kubebuilder:Minimum:=0

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the increment can only be positive, we can only increase threshold and cannot decrease it. That will work for scale up but not scale down.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is no longer an increment but a value. For scale up it will be used to decrease.

// +operator-sdk:csv:customresourcedefinitions:type=spec
// the value used to increase the threshold after after a scale up or a scale down
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// the value used to increase the threshold after after a scale up or a scale down
// The value used to increase the threshold after a scale up or a scale down.

Increment *int32 `json:"increment"`
Copy link
Collaborator

@roypaulin roypaulin Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Increment *int32 `json:"increment"`
ThresholdAdjustmentValue int32 `json:"thresholdAdjustmentValue,omitempty"`

I changed this field's name after your comment, it is always positive


// +kubebuilder:validation:Optional
// +operator-sdk:csv:customresourcedefinitions:type=spec
// the custom metric to be used for autocaling
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// the custom metric to be used for autocaling
// The custom metric to be used for autocaling.

Metric autoscalingv2.MetricSpec `json:"metric"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Metric autoscalingv2.MetricSpec `json:"metric"`
Metric autoscalingv2.MetricSpec `json:"metric,omitempty"`

}

type ScalingGranularityType string
Expand Down
Loading