Skip to content

Commit

Permalink
ssa: add NormalizeDryRunUnstructured
Browse files Browse the repository at this point in the history 
Signed-off-by: Hidde Beydals <[email protected]>
  • Loading branch information
@hiddeco
hiddeco committed Oct 5, 2023
1 parent 03ccc1a commit c7d0acb
Show file tree
Hide file tree
Showing 4 changed files with 257 additions and 70 deletions.
2 changes: 1 addition & 1 deletion ssa/manager_diff.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func prepareObjectForDiff(object *unstructured.Unstructured) *unstructured.Unstr
deepCopy := object.DeepCopy()
unstructured.RemoveNestedField(deepCopy.Object, "metadata")
unstructured.RemoveNestedField(deepCopy.Object, "status")
if err := fixHorizontalPodAutoscaler(deepCopy); err != nil {
if err := NormalizeDryRunUnstructured(deepCopy); err != nil {
return object
}
return deepCopy
Expand Down
58 changes: 58 additions & 0 deletions ssa/normalize.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ package ssa

import (
appsv1 "k8s.io/api/apps/v1"
hpav2 "k8s.io/api/autoscaling/v2"
hpav2beta2 "k8s.io/api/autoscaling/v2beta2"
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -123,6 +126,61 @@ func NormalizeUnstructuredWithScheme(object *unstructured.Unstructured, scheme *
return nil
}

// NormalizeDryRunUnstructured normalizes an Unstructured object retrieved from
// a dry-run by performing fixes for known upstream issues.
func NormalizeDryRunUnstructured(object *unstructured.Unstructured) error {
// Address an issue with dry-run returning a HorizontalPodAutoscaler
// with the first metric duplicated and an empty metric added at the
// end of the list. Which happens on Kubernetes < 1.27.x.
// xref: https://github.com/kubernetes/kubernetes/issues/118293
if object.GetKind() == "HorizontalPodAutoscaler" {
typedObject, err := FromUnstructured(object)
if err != nil {
return err
}

switch o := typedObject.(type) {
case *hpav2beta2.HorizontalPodAutoscaler:
var metrics []hpav2beta2.MetricSpec
for _, metric := range o.Spec.Metrics {
found := false
for _, existing := range metrics {
if apiequality.Semantic.DeepEqual(metric, existing) {
found = true
break
}
}
if !found && metric.Type != "" {
metrics = append(metrics, metric)
}
}
o.Spec.Metrics = metrics
case *hpav2.HorizontalPodAutoscaler:
var metrics []hpav2.MetricSpec
for _, metric := range o.Spec.Metrics {
found := false
for _, existing := range metrics {
if apiequality.Semantic.DeepEqual(metric, existing) {
found = true
break
}
}
if !found && metric.Type != "" {
metrics = append(metrics, metric)
}
}
o.Spec.Metrics = metrics
}

normalizedObject, err := ToUnstructured(typedObject)
if err != nil {
return err
}
object.Object = normalizedObject.Object
}
return nil
}

// normalizeServiceProtoDefault sets the default protocol for ports in a
// ServiceSpec.
// xref: https://github.com/kubernetes/kubernetes/pull/98576
Expand Down
198 changes: 198 additions & 0 deletions ssa/normalize_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -765,3 +765,201 @@ func TestNormalizeUnstructured(t *testing.T) {
})
}
}

func TestNormalizeDryRunUnstructured(t *testing.T) {
tests := []struct {
name string
object *unstructured.Unstructured
want *unstructured.Unstructured
wantErr bool
}{
{
name: "removes duplicated metrics from v2beta2 HorizontalPodAutoscaler",
object: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "autoscaling/v2beta2",
"kind": "HorizontalPodAutoscaler",
"spec": map[string]interface{}{
"metrics": []interface{}{
map[string]interface{}{
"type": "Resource",
"resource": map[string]interface{}{
"name": "cpu",
"target": map[string]interface{}{
"type": "Utilization",
"averageUtilization": int64(60),
},
},
},
map[string]interface{}{
"type": "ContainerResource",
"containerResource": map[string]interface{}{
"name": "cpu",
"container": "application",
"target": map[string]interface{}{
"type": "Utilization",
"averageUtilization": int64(60),
},
},
},
map[string]interface{}{
"type": "Resource",
"resource": map[string]interface{}{
"name": "cpu",
"target": map[string]interface{}{
"type": "Utilization",
"averageUtilization": int64(60),
},
},
},
map[string]interface{}{},
},
},
},
},
want: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "autoscaling/v2beta2",
"kind": "HorizontalPodAutoscaler",
"metadata": map[string]interface{}{
"creationTimestamp": nil,
},
"spec": map[string]interface{}{
"maxReplicas": int64(0),
"metrics": []interface{}{
map[string]interface{}{
"type": "Resource",
"resource": map[string]interface{}{
"name": "cpu",
"target": map[string]interface{}{
"type": "Utilization",
"averageUtilization": int64(60),
},
},
},
map[string]interface{}{
"type": "ContainerResource",
"containerResource": map[string]interface{}{
"name": "cpu",
"container": "application",
"target": map[string]interface{}{
"type": "Utilization",
"averageUtilization": int64(60),
},
},
},
},
"scaleTargetRef": map[string]interface{}{
"kind": "",
"name": "",
},
},
"status": map[string]interface{}{
"conditions": nil,
"currentMetrics": nil,
"currentReplicas": int64(0),
"desiredReplicas": int64(0),
},
},
},
},
{
name: "removes duplicated metrics from v2 HorizontalPodAutoscaler",
object: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "autoscaling/v2",
"kind": "HorizontalPodAutoscaler",
"spec": map[string]interface{}{
"metrics": []interface{}{
map[string]interface{}{
"type": "Resource",
"resource": map[string]interface{}{
"name": "cpu",
"target": map[string]interface{}{
"type": "Utilization",
"averageUtilization": int64(60),
},
},
},
map[string]interface{}{
"type": "ContainerResource",
"containerResource": map[string]interface{}{
"name": "cpu",
"container": "application",
"target": map[string]interface{}{
"type": "Utilization",
"averageUtilization": int64(60),
},
},
},
map[string]interface{}{
"type": "Resource",
"resource": map[string]interface{}{
"name": "cpu",
"target": map[string]interface{}{
"type": "Utilization",
"averageUtilization": int64(60),
},
},
},
map[string]interface{}{},
},
},
},
},
want: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "autoscaling/v2",
"kind": "HorizontalPodAutoscaler",
"metadata": map[string]interface{}{
"creationTimestamp": nil,
},
"spec": map[string]interface{}{
"maxReplicas": int64(0),
"metrics": []interface{}{
map[string]interface{}{
"type": "Resource",
"resource": map[string]interface{}{
"name": "cpu",
"target": map[string]interface{}{
"type": "Utilization",
"averageUtilization": int64(60),
},
},
},
map[string]interface{}{
"type": "ContainerResource",
"containerResource": map[string]interface{}{
"name": "cpu",
"container": "application",
"target": map[string]interface{}{
"type": "Utilization",
"averageUtilization": int64(60),
},
},
},
},
"scaleTargetRef": map[string]interface{}{
"kind": "",
"name": "",
},
},
"status": map[string]interface{}{
"currentMetrics": nil,
"desiredReplicas": int64(0),
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := NormalizeDryRunUnstructured(tt.object); (err != nil) != tt.wantErr {
t.Errorf("NormalizeDryRunUnstructured() error = %v, wantErr %v", err, tt.wantErr)
}
if diff := cmp.Diff(tt.want, tt.object); diff != "" {
t.Errorf("NormalizeDryRunUnstructured() mismatch (-want +got):\n%s", diff)
}
})
}
}
69 changes: 0 additions & 69 deletions ssa/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ import (
"strings"

"github.com/google/go-cmp/cmp"
hpav2 "k8s.io/api/autoscaling/v2"
hpav2beta2 "k8s.io/api/autoscaling/v2beta2"
apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -336,72 +333,6 @@ func SetNativeKindsDefaults(objects []*unstructured.Unstructured) error {
return nil
}

// Fix bug in server-side dry-run apply that duplicates the first item in the metrics array
// and inserts an empty metric as the last item in the array.
func fixHorizontalPodAutoscaler(object *unstructured.Unstructured) error {
if object.GetKind() == "HorizontalPodAutoscaler" {
switch object.GetAPIVersion() {
case "autoscaling/v2beta2":
var d hpav2beta2.HorizontalPodAutoscaler
err := runtime.DefaultUnstructuredConverter.FromUnstructured(object.Object, &d)
if err != nil {
return fmt.Errorf("%s validation error: %w", FmtUnstructured(object), err)
}

var metrics []hpav2beta2.MetricSpec
for _, metric := range d.Spec.Metrics {
found := false
for _, existing := range metrics {
if apiequality.Semantic.DeepEqual(metric, existing) {
found = true
break
}
}
if !found && metric.Type != "" {
metrics = append(metrics, metric)
}
}

d.Spec.Metrics = metrics

out, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&d)
if err != nil {
return fmt.Errorf("%s validation error: %w", FmtUnstructured(object), err)
}
object.Object = out
case "autoscaling/v2":
var d hpav2.HorizontalPodAutoscaler
err := runtime.DefaultUnstructuredConverter.FromUnstructured(object.Object, &d)
if err != nil {
return fmt.Errorf("%s validation error: %w", FmtUnstructured(object), err)
}

var metrics []hpav2.MetricSpec
for _, metric := range d.Spec.Metrics {
found := false
for _, existing := range metrics {
if apiequality.Semantic.DeepEqual(metric, existing) {
found = true
break
}
}
if !found && metric.Type != "" {
metrics = append(metrics, metric)
}
}

d.Spec.Metrics = metrics

out, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&d)
if err != nil {
return fmt.Errorf("%s validation error: %w", FmtUnstructured(object), err)
}
object.Object = out
}
}
return nil
}

func containsItemString(s []string, e string) bool {
for _, a := range s {
if a == e {
Expand Down

0 comments on commit c7d0acb

Please sign in to comment.