diff --git a/control-plane/api-gateway/gatekeeper/deployment.go b/control-plane/api-gateway/gatekeeper/deployment.go index 6bc1402eed..d85fbf8ef1 100644 --- a/control-plane/api-gateway/gatekeeper/deployment.go +++ b/control-plane/api-gateway/gatekeeper/deployment.go @@ -6,6 +6,7 @@ package gatekeeper import ( "context" + "github.com/google/go-cmp/cmp" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -164,13 +165,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.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.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") + } + }) + } +}