Skip to content

Commit

Permalink
Backport of [NET-6465] Consider init container resources when determi…
Browse files Browse the repository at this point in the history 
…ning if existing + desired deployments are equal into release/1.2.x (#3577)

* backport of commit fafd900

* Remove now-redundant code

* Backport tests added in source PR

---------

Co-authored-by: Nathan Coleman <[email protected]>
  • Loading branch information
@hc-github-team-consul-core @nathancoleman
hc-github-team-consul-core and nathancoleman authored Feb 7, 2024
1 parent 96d6b08 commit 7b4510d
Show file tree
Hide file tree
Showing 2 changed files with 230 additions and 5 deletions.
19 changes: 14 additions & 5 deletions control-plane/api-gateway/gatekeeper/deployment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,21 +162,30 @@ func mergeDeployments(gcc v1alpha1.GatewayClassConfig, a, b *appsv1.Deployment)
return b
}

// compareDeployments determines whether two Deployments are equal for all
// of the fields that we care about. There are some differences between a
// Deployment returned by the Kubernetes API and one that we would create
// in memory which are perfectly fine. We want to ignore those differences.
func compareDeployments(a, b *appsv1.Deployment) bool {
// since K8s adds a bunch of defaults when we create a deployment, check that
// they don't differ by the things that we may actually change, namely container
// ports
if len(b.Spec.Template.Spec.Containers) != len(a.Spec.Template.Spec.Containers) {
if len(b.Spec.Template.Spec.InitContainers) != len(a.Spec.Template.Spec.InitContainers) {
return false
}

for i, containerA := range a.Spec.Template.Spec.InitContainers {
containerB := b.Spec.Template.Spec.InitContainers[i]
if !cmp.Equal(containerA, containerB) {
if !cmp.Equal(containerA.Resources.Limits, containerB.Resources.Limits) {
return false
}

if !cmp.Equal(containerA.Resources.Requests, containerB.Resources.Requests) {
return false
}
}

if len(b.Spec.Template.Spec.Containers) != len(a.Spec.Template.Spec.Containers) {
return false
}

for i, container := range a.Spec.Template.Spec.Containers {
otherPorts := b.Spec.Template.Spec.Containers[i].Ports
if len(container.Ports) != len(otherPorts) {
Expand Down
216 changes: 216 additions & 0 deletions control-plane/api-gateway/gatekeeper/deployment_test.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
package gatekeeper

import (
"testing"

"github.com/stretchr/testify/assert"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"

"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
)

func Test_compareDeployments(t *testing.T) {
testCases := []struct {
name string
a, b *appsv1.Deployment
shouldBeEqual bool
}{
{
name: "zero-state deployments",
a: &appsv1.Deployment{},
b: &appsv1.Deployment{},
shouldBeEqual: true,
},
{
name: "different replicas",
a: &appsv1.Deployment{
Spec: appsv1.DeploymentSpec{
Replicas: common.PointerTo(int32(1)),
},
},
b: &appsv1.Deployment{
Spec: appsv1.DeploymentSpec{
Replicas: common.PointerTo(int32(2)),
},
},
shouldBeEqual: false,
},
{
name: "same replicas",
a: &appsv1.Deployment{
Spec: appsv1.DeploymentSpec{
Replicas: common.PointerTo(int32(1)),
},
},
b: &appsv1.Deployment{
Spec: appsv1.DeploymentSpec{
Replicas: common.PointerTo(int32(1)),
},
},
shouldBeEqual: true,
},
{
name: "different init container resources",
a: &appsv1.Deployment{
Spec: appsv1.DeploymentSpec{
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"cpu": requireQuantity(t, "111m"),
"memory": requireQuantity(t, "111Mi"),
},
},
},
},
},
},
},
},
b: &appsv1.Deployment{
Spec: appsv1.DeploymentSpec{
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"cpu": requireQuantity(t, "222m"),
"memory": requireQuantity(t, "111Mi"),
},
},
},
},
},
},
},
},
shouldBeEqual: false,
},
{
name: "same init container resources",
a: &appsv1.Deployment{
Spec: appsv1.DeploymentSpec{
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"cpu": requireQuantity(t, "111m"),
"memory": requireQuantity(t, "111Mi"),
},
},
},
},
},
},
},
},
b: &appsv1.Deployment{
Spec: appsv1.DeploymentSpec{
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"cpu": requireQuantity(t, "111m"),
"memory": requireQuantity(t, "111Mi"),
},
},
},
},
},
},
},
},
shouldBeEqual: true,
},
{
name: "different container ports",
a: &appsv1.Deployment{
Spec: appsv1.DeploymentSpec{
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Ports: []corev1.ContainerPort{
{ContainerPort: 7070},
{ContainerPort: 9090},
},
},
},
},
},
},
},
b: &appsv1.Deployment{
Spec: appsv1.DeploymentSpec{
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Ports: []corev1.ContainerPort{
{ContainerPort: 8080},
{ContainerPort: 9090},
},
},
},
},
},
},
},
shouldBeEqual: false,
},
{
name: "same container ports",
a: &appsv1.Deployment{
Spec: appsv1.DeploymentSpec{
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Ports: []corev1.ContainerPort{
{ContainerPort: 8080},
{ContainerPort: 9090},
},
},
},
},
},
},
},
b: &appsv1.Deployment{
Spec: appsv1.DeploymentSpec{
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Ports: []corev1.ContainerPort{
{ContainerPort: 8080},
{ContainerPort: 9090},
},
},
},
},
},
},
},
shouldBeEqual: true,
},
}

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
if testCase.shouldBeEqual {
assert.True(t, compareDeployments(testCase.a, testCase.b), "expected deployments to be equal but they were not")
} else {
assert.False(t, compareDeployments(testCase.a, testCase.b), "expected deployments to be different but they were not")
}
})
}
}

0 comments on commit 7b4510d

Please sign in to comment.