Skip to content

Commit

Permalink
refactor hpa reconcile logic to be generic for both versions
Browse files Browse the repository at this point in the history 
Signed-off-by: Sanskar Jaiswal <sanskar.jaiswal@weave.works>

for objectmeta as well
  • Loading branch information
Sanskar Jaiswal committed Jun 8, 2022
1 parent 9b97bff commit e0e2d5c
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 70 deletions.
171 changes: 101 additions & 70 deletions pkg/canary/hpa_reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,41 +82,42 @@ func (hr *HPAReconciler) reconcilePrimaryHpaV2(cd *flaggerv1.Canary, hpa *hpav2.
// create HPA
if errors.IsNotFound(err) {
primaryHpa = &hpav2.HorizontalPodAutoscaler{
ObjectMeta: metav1.ObjectMeta{
Name: primaryHpaName,
Namespace: cd.Namespace,
Labels: filterMetadata(hpa.Labels),
OwnerReferences: []metav1.OwnerReference{
*metav1.NewControllerRef(cd, schema.GroupVersionKind{
Group: flaggerv1.SchemeGroupVersion.Group,
Version: flaggerv1.SchemeGroupVersion.Version,
Kind: flaggerv1.CanaryKind,
}),
},
},
Spec: hpaSpec,
ObjectMeta: makeObjectMeta(primaryHpaName, hpa.Labels, cd),
Spec: hpaSpec,
}

_, err = hr.kubeClient.AutoscalingV2().HorizontalPodAutoscalers(cd.Namespace).Create(context.TODO(), primaryHpa, metav1.CreateOptions{})
if err != nil {
return fmt.Errorf("creating HorizontalPodAutoscaler %s.%s failed: %w",
return fmt.Errorf("creating HorizontalPodAutoscaler v2 %s.%s failed: %w",
primaryHpa.Name, primaryHpa.Namespace, err)
}
hr.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).Infof(
"HorizontalPodAutoscaler %s.%s created", primaryHpa.GetName(), cd.Namespace)
"HorizontalPodAutoscaler v2 %s.%s created", primaryHpa.GetName(), cd.Namespace)
return nil
} else if err != nil {
return fmt.Errorf("HorizontalPodAutoscaler %s.%s get query failed: %w",
return fmt.Errorf("HorizontalPodAutoscaler v2 %s.%s get query failed: %w",
primaryHpa.Name, primaryHpa.Namespace, err)
}

// update HPA
if !init && primaryHpa != nil {
diffMetrics := cmp.Diff(hpaSpec.Metrics, primaryHpa.Spec.Metrics)
diffBehavior := cmp.Diff(hpaSpec.Behavior, primaryHpa.Spec.Behavior)
diffLabels := cmp.Diff(hpa.ObjectMeta.Labels, primaryHpa.ObjectMeta.Labels)
diffAnnotations := cmp.Diff(hpa.ObjectMeta.Annotations, primaryHpa.ObjectMeta.Annotations)
if diffMetrics != "" || diffBehavior != "" || diffLabels != "" || diffAnnotations != "" || int32Default(hpaSpec.MinReplicas) != int32Default(primaryHpa.Spec.MinReplicas) || hpaSpec.MaxReplicas != primaryHpa.Spec.MaxReplicas {
targetFields := hpaFields{
metrics: hpaSpec.Metrics,
behavior: hpaSpec.Behavior,
annotations: hpa.Annotations,
labels: hpa.Labels,
min: hpaSpec.MinReplicas,
max: hpaSpec.MaxReplicas,
}
primaryFields := hpaFields{
metrics: primaryHpa.Spec.Metrics,
behavior: primaryHpa.Spec.Behavior,
annotations: primaryHpa.Annotations,
labels: primaryHpa.Labels,
min: primaryHpa.Spec.MinReplicas,
max: primaryHpa.Spec.MaxReplicas,
}
if hasHPAChanged(targetFields, primaryFields) {
err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
primaryHpa, err := hr.kubeClient.AutoscalingV2().HorizontalPodAutoscalers(cd.Namespace).Get(context.TODO(), primaryHpaName, metav1.GetOptions{})
if err != nil {
Expand All @@ -128,28 +129,17 @@ func (hr *HPAReconciler) reconcilePrimaryHpaV2(cd *flaggerv1.Canary, hpa *hpav2.
hpaClone.Spec.Metrics = hpaSpec.Metrics
hpaClone.Spec.Behavior = hpaSpec.Behavior

// update hpa annotations
hpaClone.ObjectMeta.Annotations = make(map[string]string)
filteredAnnotations := includeLabelsByPrefix(hpa.ObjectMeta.Annotations, hr.includeLabelPrefix)
for k, v := range filteredAnnotations {
hpaClone.ObjectMeta.Annotations[k] = v
}
// update hpa labels
hpaClone.ObjectMeta.Labels = make(map[string]string)
filteredLabels := includeLabelsByPrefix(hpa.ObjectMeta.Labels, hr.includeLabelPrefix)
for k, v := range filteredLabels {
hpaClone.ObjectMeta.Labels[k] = v
}
hr.updateObjectMeta(hpaClone.ObjectMeta)

_, err = hr.kubeClient.AutoscalingV2().HorizontalPodAutoscalers(cd.Namespace).Update(context.TODO(), hpaClone, metav1.UpdateOptions{})
return err
})
if err != nil {
return fmt.Errorf("updating HorizontalPodAutoscaler %s.%s failed: %w",
return fmt.Errorf("updating HorizontalPodAutoscaler v2 %s.%s failed: %w",
primaryHpa.Name, primaryHpa.Namespace, err)
}
hr.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).
Infof("HorizontalPodAutoscaler %s.%s updated", primaryHpa.GetName(), cd.Namespace)
Infof("HorizontalPodAutoscaler v2 %s.%s updated", primaryHpa.GetName(), cd.Namespace)
}
}
return nil
Expand All @@ -176,41 +166,42 @@ func (hr *HPAReconciler) reconcilePrimaryHpaV2Beta2(cd *flaggerv1.Canary, hpa *h
// create HPA
if errors.IsNotFound(err) {
primaryHpa = &hpav2beta2.HorizontalPodAutoscaler{
ObjectMeta: metav1.ObjectMeta{
Name: primaryHpaName,
Namespace: cd.Namespace,
Labels: filterMetadata(hpa.Labels),
OwnerReferences: []metav1.OwnerReference{
*metav1.NewControllerRef(cd, schema.GroupVersionKind{
Group: flaggerv1.SchemeGroupVersion.Group,
Version: flaggerv1.SchemeGroupVersion.Version,
Kind: flaggerv1.CanaryKind,
}),
},
},
Spec: hpaSpec,
ObjectMeta: makeObjectMeta(primaryHpaName, hpa.Labels, cd),
Spec: hpaSpec,
}

_, err = hr.kubeClient.AutoscalingV2beta2().HorizontalPodAutoscalers(cd.Namespace).Create(context.TODO(), primaryHpa, metav1.CreateOptions{})
if err != nil {
return fmt.Errorf("creating HorizontalPodAutoscaler %s.%s failed: %w",
return fmt.Errorf("creating HorizontalPodAutoscaler v2beta2 %s.%s failed: %w",
primaryHpa.Name, primaryHpa.Namespace, err)
}
hr.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).Infof(
"HorizontalPodAutoscaler %s.%s created", primaryHpa.GetName(), cd.Namespace)
"HorizontalPodAutoscaler v2beta2 %s.%s created", primaryHpa.GetName(), cd.Namespace)
return nil
} else if err != nil {
return fmt.Errorf("HorizontalPodAutoscaler %s.%s get query failed: %w",
return fmt.Errorf("HorizontalPodAutoscaler v2beta2 %s.%s get query failed: %w",
primaryHpa.Name, primaryHpa.Namespace, err)
}

// update HPA
if !init && primaryHpa != nil {
diffMetrics := cmp.Diff(hpaSpec.Metrics, primaryHpa.Spec.Metrics)
diffBehavior := cmp.Diff(hpaSpec.Behavior, primaryHpa.Spec.Behavior)
diffLabels := cmp.Diff(hpa.ObjectMeta.Labels, primaryHpa.ObjectMeta.Labels)
diffAnnotations := cmp.Diff(hpa.ObjectMeta.Annotations, primaryHpa.ObjectMeta.Annotations)
if diffMetrics != "" || diffBehavior != "" || diffLabels != "" || diffAnnotations != "" || int32Default(hpaSpec.MinReplicas) != int32Default(primaryHpa.Spec.MinReplicas) || hpaSpec.MaxReplicas != primaryHpa.Spec.MaxReplicas {
targetFields := hpaFields{
metrics: hpaSpec.Metrics,
behavior: hpaSpec.Behavior,
annotations: hpa.Annotations,
labels: hpa.Labels,
min: hpaSpec.MinReplicas,
max: hpaSpec.MaxReplicas,
}
primaryFields := hpaFields{
metrics: primaryHpa.Spec.Metrics,
behavior: primaryHpa.Spec.Behavior,
annotations: primaryHpa.Annotations,
labels: primaryHpa.Labels,
min: primaryHpa.Spec.MinReplicas,
max: primaryHpa.Spec.MaxReplicas,
}
if hasHPAChanged(targetFields, primaryFields) {
err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
primaryHpa, err := hr.kubeClient.AutoscalingV2beta2().HorizontalPodAutoscalers(cd.Namespace).Get(context.TODO(), primaryHpaName, metav1.GetOptions{})
if err != nil {
Expand All @@ -222,28 +213,17 @@ func (hr *HPAReconciler) reconcilePrimaryHpaV2Beta2(cd *flaggerv1.Canary, hpa *h
hpaClone.Spec.Metrics = hpaSpec.Metrics
hpaClone.Spec.Behavior = hpaSpec.Behavior

// update hpa annotations
hpaClone.ObjectMeta.Annotations = make(map[string]string)
filteredAnnotations := includeLabelsByPrefix(hpa.ObjectMeta.Annotations, hr.includeLabelPrefix)
for k, v := range filteredAnnotations {
hpaClone.ObjectMeta.Annotations[k] = v
}
// update hpa labels
hpaClone.ObjectMeta.Labels = make(map[string]string)
filteredLabels := includeLabelsByPrefix(hpa.ObjectMeta.Labels, hr.includeLabelPrefix)
for k, v := range filteredLabels {
hpaClone.ObjectMeta.Labels[k] = v
}
hr.updateObjectMeta(hpaClone.ObjectMeta)

_, err = hr.kubeClient.AutoscalingV2beta2().HorizontalPodAutoscalers(cd.Namespace).Update(context.TODO(), hpaClone, metav1.UpdateOptions{})
return err
})
if err != nil {
return fmt.Errorf("updating HorizontalPodAutoscaler %s.%s failed: %w",
return fmt.Errorf("updating HorizontalPodAutoscaler v2beta2 %s.%s failed: %w",
primaryHpa.Name, primaryHpa.Namespace, err)
}
hr.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).
Infof("HorizontalPodAutoscaler %s.%s updated", primaryHpa.GetName(), cd.Namespace)
Infof("HorizontalPodAutoscaler v2beta2 %s.%s updated", primaryHpa.GetName(), cd.Namespace)
}
}
return nil
Expand All @@ -256,3 +236,54 @@ func (hr *HPAReconciler) PauseTargetScaler(cd *flaggerv1.Canary) error {
func (hr *HPAReconciler) ResumeTargetScaler(cd *flaggerv1.Canary) error {
return nil
}

func (hr *HPAReconciler) updateObjectMeta(meta metav1.ObjectMeta) {
// update hpa annotations
meta.Annotations = make(map[string]string)
filteredAnnotations := includeLabelsByPrefix(meta.Annotations, hr.includeLabelPrefix)
for k, v := range filteredAnnotations {
meta.Annotations[k] = v
}
// update hpa labels
meta.Labels = make(map[string]string)
filteredLabels := includeLabelsByPrefix(meta.Labels, hr.includeLabelPrefix)
for k, v := range filteredLabels {
meta.Labels[k] = v
}
}

type hpaFields struct {
metrics interface{}
behavior interface{}
annotations map[string]string
labels map[string]string
min *int32
max int32
}

func hasHPAChanged(target, primary hpaFields) bool {
diffMetrics := cmp.Diff(target.metrics, primary.metrics)
diffBehavior := cmp.Diff(target.behavior, primary.behavior)
diffLabels := cmp.Diff(target.labels, primary.labels)
diffAnnotations := cmp.Diff(target.annotations, primary.annotations)
if diffMetrics != "" || diffBehavior != "" || diffLabels != "" || diffAnnotations != "" ||
int32Default(target.min) != int32Default(primary.min) || target.max != primary.max {
return true
}
return false
}

func makeObjectMeta(name string, labels map[string]string, cd *flaggerv1.Canary) metav1.ObjectMeta {
return metav1.ObjectMeta{
Name: name,
Namespace: cd.Namespace,
Labels: filterMetadata(labels),
OwnerReferences: []metav1.OwnerReference{
*metav1.NewControllerRef(cd, schema.GroupVersionKind{
Group: flaggerv1.SchemeGroupVersion.Group,
Version: flaggerv1.SchemeGroupVersion.Version,
Kind: flaggerv1.CanaryKind,
}),
},
}
}
4 changes: 4 additions & 0 deletions pkg/canary/hpa_reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ func Test_reconcilePrimaryHpaV2(t *testing.T) {
assert.Equal(t, int(*primaryHPA.Spec.Metrics[0].Resource.Target.AverageUtilization), 99)

hpa.Spec.Metrics[0].Resource.Target = hpav2.MetricTarget{AverageUtilization: int32p(50)}
hpa.Spec.MaxReplicas = 10
_, err = mocks.kubeClient.AutoscalingV2().HorizontalPodAutoscalers("default").Update(context.TODO(), hpa, metav1.UpdateOptions{})
require.NoError(t, err)

Expand All @@ -72,6 +73,7 @@ func Test_reconcilePrimaryHpaV2(t *testing.T) {
primaryHPA, err = mocks.kubeClient.AutoscalingV2().HorizontalPodAutoscalers("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, int(*primaryHPA.Spec.Metrics[0].Resource.Target.AverageUtilization), 50)
assert.Equal(t, int(primaryHPA.Spec.MaxReplicas), 10)
}

func Test_reconcilePrimaryHpaV2Beta2(t *testing.T) {
Expand All @@ -94,6 +96,7 @@ func Test_reconcilePrimaryHpaV2Beta2(t *testing.T) {
assert.Equal(t, int(*primaryHPA.Spec.Metrics[0].Resource.Target.AverageUtilization), 99)

hpa.Spec.Metrics[0].Resource.Target = hpav2beta2.MetricTarget{AverageUtilization: int32p(50)}
hpa.Spec.MaxReplicas = 10
_, err = mocks.kubeClient.AutoscalingV2beta2().HorizontalPodAutoscalers("default").Update(context.TODO(), hpa, metav1.UpdateOptions{})
require.NoError(t, err)

Expand All @@ -103,4 +106,5 @@ func Test_reconcilePrimaryHpaV2Beta2(t *testing.T) {
primaryHPA, err = mocks.kubeClient.AutoscalingV2beta2().HorizontalPodAutoscalers("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, int(*primaryHPA.Spec.Metrics[0].Resource.Target.AverageUtilization), 50)
assert.Equal(t, int(primaryHPA.Spec.MaxReplicas), 10)
}

0 comments on commit e0e2d5c

Please sign in to comment.