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

Jump to bottom

Introduce ScalerReconciler and refactor HPA reconciliation #1211

Merged
merged 4 commits into from
Jun 8, 2022
Merged

Introduce ScalerReconciler and refactor HPA reconciliation #1211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fa595e1
add ScalerReconciler to canary and refactor hpa out of deployment con…
Jun 5, 2022
f23be1d
add unit tests for hpa reconciler
Jun 6, 2022
9b97bff
add e2e tests for hpa reconciler
Jun 6, 2022
e0e2d5c
refactor hpa reconcile logic to be generic for both versions
Jun 7, 2022
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
22 changes: 0 additions & 22 deletions pkg/canary/deployment_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ type DeploymentController struct {
// Initialize creates the primary deployment, hpa,
// scales to zero the canary deployment and returns the pod selector label and container ports
func (c *DeploymentController) Initialize(cd *flaggerv1.Canary) (err error) {
primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name)
if err := c.createPrimaryDeployment(cd, c.includeLabelPrefix); err != nil {
return fmt.Errorf("createPrimaryDeployment failed: %w", err)
}
Expand All @@ -67,16 +66,6 @@ func (c *DeploymentController) Initialize(cd *flaggerv1.Canary) (err error) {
}
}

if cd.Spec.AutoscalerRef != nil {
if cd.Spec.AutoscalerRef.Kind == "HorizontalPodAutoscaler" {
if err := c.reconcilePrimaryHpa(cd, true); err != nil {
return fmt.Errorf(
"initial reconcilePrimaryHpa for %s.%s failed: %w", primaryName, cd.Namespace, err)
}
} else {
return fmt.Errorf("cd.Spec.AutoscalerRef.Kind is invalid: %s", cd.Spec.AutoscalerRef.Kind)
}
}
return nil
}

Expand Down Expand Up @@ -155,17 +144,6 @@ func (c *DeploymentController) Promote(cd *flaggerv1.Canary) error {
primaryName, cd.Namespace, err)
}

// update HPA
if cd.Spec.AutoscalerRef != nil {
if cd.Spec.AutoscalerRef.Kind == "HorizontalPodAutoscaler" {
if err := c.reconcilePrimaryHpa(cd, false); err != nil {
return fmt.Errorf(
"reconcilePrimaryHpa for %s.%s failed: %w", primaryName, cd.Namespace, err)
}
} else {
return fmt.Errorf("cd.Spec.AutoscalerRef.Kind is invalid: %s", cd.Spec.AutoscalerRef.Kind)
}
}
return nil
}

Expand Down
29 changes: 0 additions & 29 deletions pkg/canary/deployment_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ func TestDeploymentController_Sync_ConsistentNaming(t *testing.T) {

annotation := depPrimary.Annotations["kustomize.toolkit.fluxcd.io/checksum"]
assert.Equal(t, "", annotation)

hpaPrimary, err := mocks.kubeClient.AutoscalingV2beta2().HorizontalPodAutoscalers("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, depPrimary.Name, hpaPrimary.Spec.ScaleTargetRef.Name)
}

func TestDeploymentController_Sync_InconsistentNaming(t *testing.T) {
Expand All @@ -71,10 +67,6 @@ func TestDeploymentController_Sync_InconsistentNaming(t *testing.T) {

primarySelectorValue := depPrimary.Spec.Selector.MatchLabels[dc.label]
assert.Equal(t, primarySelectorValue, fmt.Sprintf("%s-primary", dc.labelValue))

hpaPrimary, err := mocks.kubeClient.AutoscalingV2beta2().HorizontalPodAutoscalers("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, depPrimary.Name, hpaPrimary.Spec.ScaleTargetRef.Name)
}

func TestDeploymentController_Promote(t *testing.T) {
Expand All @@ -90,15 +82,6 @@ func TestDeploymentController_Promote(t *testing.T) {
_, err = mocks.kubeClient.CoreV1().ConfigMaps("default").Update(context.TODO(), config2, metav1.UpdateOptions{})
require.NoError(t, err)

hpa, err := mocks.kubeClient.AutoscalingV2beta2().HorizontalPodAutoscalers("default").Get(context.TODO(), "podinfo", metav1.GetOptions{})
require.NoError(t, err)

hpaClone := hpa.DeepCopy()
hpaClone.Spec.MaxReplicas = 2

_, err = mocks.kubeClient.AutoscalingV2beta2().HorizontalPodAutoscalers("default").Update(context.TODO(), hpaClone, metav1.UpdateOptions{})
require.NoError(t, err)

err = mocks.controller.Promote(mocks.canary)
require.NoError(t, err)

Expand All @@ -121,18 +104,6 @@ func TestDeploymentController_Promote(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, config2.Data["color"], configPrimary.Data["color"])

hpaPrimary, err := mocks.kubeClient.AutoscalingV2beta2().HorizontalPodAutoscalers("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, int32(2), hpaPrimary.Spec.MaxReplicas)

hpaPrimaryLabels := hpaPrimary.ObjectMeta.Labels
hpaSourceLabels := hpa.ObjectMeta.Labels
assert.Equal(t, hpaSourceLabels["app.kubernetes.io/test-label-1"], hpaPrimaryLabels["app.kubernetes.io/test-label-1"])

hpaPrimaryAnnotations := hpaPrimary.ObjectMeta.Annotations
hpaSourceAnnotations := hpa.ObjectMeta.Annotations
assert.Equal(t, hpaSourceAnnotations["app.kubernetes.io/test-annotation-1"], hpaPrimaryAnnotations["app.kubernetes.io/test-annotation-1"])

value := depPrimary.Spec.Template.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution[0].PodAffinityTerm.LabelSelector.MatchExpressions[0].Values[0]
assert.Equal(t, "podinfo-primary", value)

Expand Down
32 changes: 0 additions & 32 deletions pkg/canary/deployment_fixture_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"github.com/stretchr/testify/require"
"go.uber.org/zap"
appsv1 "k8s.io/api/apps/v1"
hpav2 "k8s.io/api/autoscaling/v2beta2"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -91,7 +90,6 @@ func newCustomizableFixture(dc deploymentConfigs) (deploymentControllerFixture,
// init kube clientset and register mock objects
kubeClient := fake.NewSimpleClientset(
newDeploymentControllerTest(dc),
newDeploymentControllerTestHPA(),
newDeploymentControllerTestConfigMap(),
newDeploymentControllerTestConfigMapEnv(),
newDeploymentControllerTestConfigMapVol(),
Expand Down Expand Up @@ -1000,33 +998,3 @@ func newDeploymentControllerTestV2() *appsv1.Deployment {

return d
}

func newDeploymentControllerTestHPA() *hpav2.HorizontalPodAutoscaler {
h := &hpav2.HorizontalPodAutoscaler{
TypeMeta: metav1.TypeMeta{APIVersion: hpav2.SchemeGroupVersion.String()},
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "podinfo",
},
Spec: hpav2.HorizontalPodAutoscalerSpec{
ScaleTargetRef: hpav2.CrossVersionObjectReference{
Name: "podinfo",
APIVersion: "apps/v1",
Kind: "Deployment",
},
Metrics: []hpav2.MetricSpec{
{
Type: "Resource",
Resource: &hpav2.ResourceMetricSource{
Name: "cpu",
Target: hpav2.MetricTarget{
AverageUtilization: int32p(99),
},
},
},
},
},
}

return h
}
16 changes: 16 additions & 0 deletions pkg/canary/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,19 @@ func (factory *Factory) Controller(kind string) Controller {
return deploymentCtrl
}
}

func (factory *Factory) ScalerReconciler(kind string) ScalerReconciler {
hpaReconciler := &HPAReconciler{
logger: factory.logger,
kubeClient: factory.kubeClient,
flaggerClient: factory.flaggerClient,
includeLabelPrefix: factory.includeLabelPrefix,
}

switch kind {
case "HorizontalPodAutoscaler":
return hpaReconciler
default:
return nil
}
}
Loading