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.

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

NETOBSERV-382: Added status field in HPA configuration #184

Merged
merged 2 commits into from
Oct 27, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
26 changes: 24 additions & 2 deletions api/v1alpha1/flowcollector_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ type FlowCollectorFLP struct {
// kafkaConsumerAutoscaler spec of a horizontal pod autoscaler to set up for flowlogs-pipeline-transformer, which consumes Kafka messages.
// This setting is ignored when Kafka is disabled.
// +optional
KafkaConsumerAutoscaler *FlowCollectorHPA `json:"kafkaConsumerAutoscaler,omitempty"`
KafkaConsumerAutoscaler FlowCollectorHPA `json:"kafkaConsumerAutoscaler,omitempty"`

//+kubebuilder:default:=1000
// +optional
Expand All @@ -366,7 +366,19 @@ type FlowCollectorFLP struct {
Env map[string]string `json:"env,omitempty"`
}

const (
HPAStatusDisabled = "DISABLED"
HPAStatusEnabled = "ENABLED"
)

type FlowCollectorHPA struct {
// +kubebuilder:validation:Enum:=DISABLED;ENABLED
// +kubebuilder:default:=DISABLED
// AuthToken describe the way to get a token to authenticate to Loki
Copy link
Member

Choose a reason for hiding this comment

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

wrong description, I bet you copy-pasted :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes sorry, fixed.

// DISABLED will not send any token with the requestmode.
// ENABLED will deploy an horizontal pod autoscaler
Status string `json:"status,omitempty"`

// minReplicas is the lower limit for the number of replicas to which the autoscaler
// can scale down. It defaults to 1 pod. minReplicas is allowed to be 0 if the
// alpha feature gate HPAScaleToZero is enabled and at least one Object or External
Expand All @@ -375,12 +387,22 @@ type FlowCollectorHPA struct {
// +optional
MinReplicas *int32 `json:"minReplicas,omitempty" protobuf:"varint,2,opt,name=minReplicas"`
// maxReplicas is the upper limit for the number of pods that can be set by the autoscaler; cannot be smaller than MinReplicas.
// +kubebuilder:default:=3
// +optional
MaxReplicas int32 `json:"maxReplicas" protobuf:"varint,3,opt,name=maxReplicas"`
// metrics used by the pod autoscaler
// +optional
Metrics []ascv2.MetricSpec `json:"metrics"`
}

func (spec *FlowCollectorHPA) Disabled() bool {
return spec.Status == HPAStatusDisabled
}

func (spec *FlowCollectorHPA) Enabled() bool {
return spec.Status == HPAStatusEnabled
}

const (
LokiAuthDisabled = "DISABLED"
LokiAuthUseHostToken = "HOST"
Expand Down Expand Up @@ -510,7 +532,7 @@ type FlowCollectorConsolePlugin struct {

// autoscaler spec of a horizontal pod autoscaler to set up for the plugin Deployment.
// +optional
Autoscaler *FlowCollectorHPA `json:"autoscaler,omitempty"`
Autoscaler FlowCollectorHPA `json:"autoscaler,omitempty"`

//+kubebuilder:default:={enable:true}
// portNaming defines the configuration of the port-to-service name translation
Expand Down
12 changes: 2 additions & 10 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 20 additions & 4 deletions config/crd/bases/flows.netobserv.io_flowcollectors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ spec:
set up for the plugin Deployment.
properties:
maxReplicas:
default: 3
description: maxReplicas is the upper limit for the number
of pods that can be set by the autoscaler; cannot be smaller
than MinReplicas.
Expand Down Expand Up @@ -790,8 +791,15 @@ spec:
active as long as at least one metric value is available.
format: int32
type: integer
required:
- maxReplicas
status:
default: DISABLED
description: AuthToken describe the way to get a token to
authenticate to Loki DISABLED will not send any token with
the requestmode. ENABLED will deploy an horizontal pod autoscaler
enum:
- DISABLED
- ENABLED
type: string
type: object
image:
default: quay.io/netobserv/network-observability-console-plugin:main
Expand Down Expand Up @@ -1189,6 +1197,7 @@ spec:
is disabled.
properties:
maxReplicas:
default: 3
description: maxReplicas is the upper limit for the number
of pods that can be set by the autoscaler; cannot be smaller
than MinReplicas.
Expand Down Expand Up @@ -1701,8 +1710,15 @@ spec:
active as long as at least one metric value is available.
format: int32
type: integer
required:
- maxReplicas
status:
default: DISABLED
description: AuthToken describe the way to get a token to
authenticate to Loki DISABLED will not send any token with
the requestmode. ENABLED will deploy an horizontal pod autoscaler
enum:
- DISABLED
- ENABLED
type: string
type: object
kafkaConsumerBatchSize:
default: 10485760
Expand Down
1 change: 0 additions & 1 deletion config/samples/flows_v1alpha1_flowcollector.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ spec:
imagePullPolicy: IfNotPresent
port: 9001
logLevel: info
autoscaler: null
Copy link
Member

Choose a reason for hiding this comment

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

could you add a sample autoscaler config, but commented out? I find it quite convenient when trying configs that are not obvious

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Member

Choose a reason for hiding this comment

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

thanks! Actually better than commented out :)

portNaming:
enable: true
portNames:
Expand Down
4 changes: 2 additions & 2 deletions controllers/consoleplugin/consoleplugin_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ func (r *CPReconciler) reconcileService(ctx context.Context, builder builder, de

func (r *CPReconciler) reconcileHPA(ctx context.Context, builder builder, desired *flowsv1alpha1.FlowCollectorSpec, ns string) error {
// Delete or Create / Update Autoscaler according to HPA option
if desired.ConsolePlugin.Autoscaler == nil {
if desired.ConsolePlugin.Autoscaler.Disabled() {
r.nobjMngr.TryDelete(ctx, r.owned.hpa)
} else {
newASC := builder.autoScaler()
Expand Down Expand Up @@ -229,7 +229,7 @@ func deploymentNeedsUpdate(depl *appsv1.Deployment, desired *flowsv1alpha1.FlowC
}
return containerNeedsUpdate(&depl.Spec.Template.Spec, &desired.ConsolePlugin, &desired.Loki) ||
configChanged(&depl.Spec.Template, cmDigest) ||
(desired.ConsolePlugin.Autoscaler == nil && *depl.Spec.Replicas != desired.ConsolePlugin.Replicas)
(desired.ConsolePlugin.Autoscaler.Disabled() && *depl.Spec.Replicas != desired.ConsolePlugin.Replicas)
}

func configChanged(tmpl *corev1.PodTemplateSpec, cmDigest string) bool {
Expand Down
3 changes: 2 additions & 1 deletion controllers/consoleplugin/consoleplugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ func getPluginConfig() flowsv1alpha1.FlowCollectorConsolePlugin {
Image: testImage,
ImagePullPolicy: string(testPullPolicy),
Resources: testResources,
Autoscaler: &flowsv1alpha1.FlowCollectorHPA{
Autoscaler: flowsv1alpha1.FlowCollectorHPA{
Status: flowsv1alpha1.HPAStatusEnabled,
MinReplicas: &minReplicas,
MaxReplicas: maxReplicas,
Metrics: []ascv2.MetricSpec{{
Expand Down
5 changes: 3 additions & 2 deletions controllers/flowcollector_controller_console_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ func flowCollectorConsolePluginSpecs() {
ImagePullPolicy: "Never",
Image: "testimg:latest",
Register: true,
Autoscaler: &flowsv1alpha1.FlowCollectorHPA{
Autoscaler: flowsv1alpha1.FlowCollectorHPA{
Status: flowsv1alpha1.HPAStatusEnabled,
MinReplicas: pointer.Int32(1),
MaxReplicas: 1,
Metrics: []ascv2.MetricSpec{{
Expand Down Expand Up @@ -152,7 +153,7 @@ func flowCollectorConsolePluginSpecs() {
}
fc.Spec.ConsolePlugin.Port = 9099
fc.Spec.ConsolePlugin.Replicas = 2
fc.Spec.ConsolePlugin.Autoscaler = nil
fc.Spec.ConsolePlugin.Autoscaler.Status = flowsv1alpha1.HPAStatusDisabled
return k8sClient.Update(ctx, &fc)
}).Should(Succeed())

Expand Down
3 changes: 2 additions & 1 deletion controllers/flowcollector_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,8 @@ func flowCollectorControllerSpecs() {
hpa := ascv2.HorizontalPodAutoscaler{}
It("Should update with HPA", func() {
UpdateCR(crKey, func(fc *flowsv1alpha1.FlowCollector) {
fc.Spec.Processor.KafkaConsumerAutoscaler = &flowsv1alpha1.FlowCollectorHPA{
fc.Spec.Processor.KafkaConsumerAutoscaler = flowsv1alpha1.FlowCollectorHPA{
Status: flowsv1alpha1.HPAStatusEnabled,
MinReplicas: pointer.Int32(1),
MaxReplicas: 1,
Metrics: []ascv2.MetricSpec{{
Expand Down
17 changes: 9 additions & 8 deletions controllers/flowlogspipeline/flp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ func getConfig() v1alpha1.FlowCollectorSpec {
},
},
KafkaConsumerReplicas: 1,
KafkaConsumerAutoscaler: &v1alpha1.FlowCollectorHPA{
KafkaConsumerAutoscaler: v1alpha1.FlowCollectorHPA{
Status: v1alpha1.HPAStatusEnabled,
MinReplicas: &minReplicas,
MaxReplicas: maxReplicas,
Metrics: []ascv2.MetricSpec{{
Expand Down Expand Up @@ -105,7 +106,7 @@ func getConfig() v1alpha1.FlowCollectorSpec {

func getConfigNoHPA() v1alpha1.FlowCollectorSpec {
cfg := getConfig()
cfg.Processor.KafkaConsumerAutoscaler = nil
cfg.Processor.KafkaConsumerAutoscaler.Status = v1alpha1.HPAStatusDisabled
return cfg
}

Expand Down Expand Up @@ -134,7 +135,7 @@ func getAutoScalerSpecs() (ascv2.HorizontalPodAutoscaler, v1alpha1.FlowCollector
},
}

return autoScaler, *getConfig().Processor.KafkaConsumerAutoscaler
return autoScaler, getConfig().Processor.KafkaConsumerAutoscaler
}

func TestDaemonSetNoChange(t *testing.T) {
Expand Down Expand Up @@ -399,27 +400,27 @@ func TestAutoScalerUpdateCheck(t *testing.T) {

//equals specs
autoScalerSpec, hpa := getAutoScalerSpecs()
assert.Equal(autoScalerNeedsUpdate(&autoScalerSpec, &hpa, testNamespace), false)
assert.Equal(autoScalerNeedsUpdate(&autoScalerSpec, hpa, testNamespace), false)

//wrong max replicas
autoScalerSpec, hpa = getAutoScalerSpecs()
autoScalerSpec.Spec.MaxReplicas = 10
assert.Equal(autoScalerNeedsUpdate(&autoScalerSpec, &hpa, testNamespace), true)
assert.Equal(autoScalerNeedsUpdate(&autoScalerSpec, hpa, testNamespace), true)

//missing min replicas
autoScalerSpec, hpa = getAutoScalerSpecs()
autoScalerSpec.Spec.MinReplicas = nil
assert.Equal(autoScalerNeedsUpdate(&autoScalerSpec, &hpa, testNamespace), true)
assert.Equal(autoScalerNeedsUpdate(&autoScalerSpec, hpa, testNamespace), true)

//missing metrics
autoScalerSpec, hpa = getAutoScalerSpecs()
autoScalerSpec.Spec.Metrics = []ascv2.MetricSpec{}
assert.Equal(autoScalerNeedsUpdate(&autoScalerSpec, &hpa, testNamespace), true)
assert.Equal(autoScalerNeedsUpdate(&autoScalerSpec, hpa, testNamespace), true)

//wrong namespace
autoScalerSpec, hpa = getAutoScalerSpecs()
autoScalerSpec.Namespace = "NewNamespace"
assert.Equal(autoScalerNeedsUpdate(&autoScalerSpec, &hpa, testNamespace), true)
assert.Equal(autoScalerNeedsUpdate(&autoScalerSpec, hpa, testNamespace), true)
}

func TestLabels(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions controllers/flowlogspipeline/flp_transfo_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func (r *flpTransformerReconciler) reconcileDeployment(ctx context.Context, desi
}

// Delete or Create / Update Autoscaler according to HPA option
if desiredFLP.KafkaConsumerAutoscaler == nil {
if desiredFLP.KafkaConsumerAutoscaler.Disabled() {
r.nobjMngr.TryDelete(ctx, r.owned.hpa)
} else {
newASC := builder.autoScaler()
Expand Down Expand Up @@ -182,10 +182,10 @@ func (r *flpTransformerReconciler) reconcilePermissions(ctx context.Context, bui
func deploymentNeedsUpdate(depl *appsv1.Deployment, desired *flpSpec, configDigest string) bool {
return containerNeedsUpdate(&depl.Spec.Template.Spec, desired, false) ||
configChanged(&depl.Spec.Template, configDigest) ||
(desired.KafkaConsumerAutoscaler == nil && *depl.Spec.Replicas != desired.KafkaConsumerReplicas)
(desired.KafkaConsumerAutoscaler.Disabled() && *depl.Spec.Replicas != desired.KafkaConsumerReplicas)
}

func autoScalerNeedsUpdate(asc *ascv2.HorizontalPodAutoscaler, desired *flowsv1alpha1.FlowCollectorHPA, ns string) bool {
func autoScalerNeedsUpdate(asc *ascv2.HorizontalPodAutoscaler, desired flowsv1alpha1.FlowCollectorHPA, ns string) bool {
if asc.Namespace != ns {
return true
}
Expand Down
26 changes: 24 additions & 2 deletions docs/FlowCollector.md
Original file line number Diff line number Diff line change
Expand Up @@ -626,8 +626,9 @@ autoscaler spec of a horizontal pod autoscaler to set up for the plugin Deployme
maxReplicas is the upper limit for the number of pods that can be set by the autoscaler; cannot be smaller than MinReplicas.<br/>
<br/>
<i>Format</i>: int32<br/>
<i>Default</i>: 3<br/>
</td>
<td>true</td>
<td>false</td>
</tr><tr>
<td><b><a href="#flowcollectorspecconsolepluginautoscalermetricsindex">metrics</a></b></td>
<td>[]object</td>
Expand All @@ -644,6 +645,16 @@ autoscaler spec of a horizontal pod autoscaler to set up for the plugin Deployme
<i>Format</i>: int32<br/>
</td>
<td>false</td>
</tr><tr>
<td><b>status</b></td>
<td>enum</td>
<td>
AuthToken describe the way to get a token to authenticate to Loki DISABLED will not send any token with the requestmode. ENABLED will deploy an horizontal pod autoscaler<br/>
<br/>
<i>Enum</i>: DISABLED, ENABLED<br/>
<i>Default</i>: DISABLED<br/>
</td>
<td>false</td>
</tr></tbody>
</table>

Expand Down Expand Up @@ -2251,8 +2262,9 @@ kafkaConsumerAutoscaler spec of a horizontal pod autoscaler to set up for flowlo
maxReplicas is the upper limit for the number of pods that can be set by the autoscaler; cannot be smaller than MinReplicas.<br/>
<br/>
<i>Format</i>: int32<br/>
<i>Default</i>: 3<br/>
</td>
<td>true</td>
<td>false</td>
</tr><tr>
<td><b><a href="#flowcollectorspecprocessorkafkaconsumerautoscalermetricsindex">metrics</a></b></td>
<td>[]object</td>
Expand All @@ -2269,6 +2281,16 @@ kafkaConsumerAutoscaler spec of a horizontal pod autoscaler to set up for flowlo
<i>Format</i>: int32<br/>
</td>
<td>false</td>
</tr><tr>
<td><b>status</b></td>
<td>enum</td>
<td>
AuthToken describe the way to get a token to authenticate to Loki DISABLED will not send any token with the requestmode. ENABLED will deploy an horizontal pod autoscaler<br/>
<br/>
<i>Enum</i>: DISABLED, ENABLED<br/>
<i>Default</i>: DISABLED<br/>
</td>
<td>false</td>
</tr></tbody>
</table>

Expand Down