diff --git a/control-plane/api-gateway/gatekeeper/deployment.go b/control-plane/api-gateway/gatekeeper/deployment.go index 5ab5e2a7c6..38207dc819 100644 --- a/control-plane/api-gateway/gatekeeper/deployment.go +++ b/control-plane/api-gateway/gatekeeper/deployment.go @@ -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) { diff --git a/control-plane/api-gateway/gatekeeper/deployment_test.go b/control-plane/api-gateway/gatekeeper/deployment_test.go new file mode 100644 index 0000000000..0e4c2f53f0 --- /dev/null +++ b/control-plane/api-gateway/gatekeeper/deployment_test.go @@ -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") + } + }) + } +}