From f88512a66a67c896a7eb1d8e17097e8a817efec6 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Tue, 6 Feb 2024 16:06:55 -0500 Subject: [PATCH 1/2] [NET-6465] Respect connectInject.initContainer.resources for v1 API gateways (#3531) * Respect connectInject.initContainer.resources for v1 API gateways * Add changelog entry * Add test coverage for init container resources on API gateway Pods --- .changelog/3531.txt | 3 ++ .../api-gateway/common/helm_config.go | 4 ++ .../api-gateway/gatekeeper/gatekeeper_test.go | 38 +++++++++++++++++-- control-plane/api-gateway/gatekeeper/init.go | 7 +++- .../subcommand/inject-connect/command.go | 1 + 5 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 .changelog/3531.txt diff --git a/.changelog/3531.txt b/.changelog/3531.txt new file mode 100644 index 0000000000..cd61ff3ce2 --- /dev/null +++ b/.changelog/3531.txt @@ -0,0 +1,3 @@ +```release-note:improvement +api-gateway: Apply `connectInject.initContainer.resources` to the init container for API gateway Pods. +``` diff --git a/control-plane/api-gateway/common/helm_config.go b/control-plane/api-gateway/common/helm_config.go index 7ce8e0778a..9b0ab1d7e8 100644 --- a/control-plane/api-gateway/common/helm_config.go +++ b/control-plane/api-gateway/common/helm_config.go @@ -6,6 +6,8 @@ package common import ( "strings" "time" + + v1 "k8s.io/api/core/v1" ) const componentAuthMethod = "k8s-component-auth-method" @@ -40,6 +42,8 @@ type HelmConfig struct { // MapPrivilegedServicePorts is the value which Consul will add to privileged container port values (ports < 1024) // defined on a Gateway. MapPrivilegedServicePorts int + + InitContainerResources *v1.ResourceRequirements } type ConsulConfig struct { diff --git a/control-plane/api-gateway/gatekeeper/gatekeeper_test.go b/control-plane/api-gateway/gatekeeper/gatekeeper_test.go index 8d61e6948c..79a3050e04 100644 --- a/control-plane/api-gateway/gatekeeper/gatekeeper_test.go +++ b/control-plane/api-gateway/gatekeeper/gatekeeper_test.go @@ -15,6 +15,7 @@ import ( corev1 "k8s.io/api/core/v1" rbac "k8s.io/api/rbac/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -107,6 +108,16 @@ func TestUpsert(t *testing.T) { }, helmConfig: common.HelmConfig{ ImageDataplane: dataplaneImage, + InitContainerResources: &corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: requireQuantity(t, "100m"), + corev1.ResourceMemory: requireQuantity(t, "2Gi"), + }, + Requests: corev1.ResourceList{ + corev1.ResourceCPU: requireQuantity(t, "100m"), + corev1.ResourceMemory: requireQuantity(t, "2Gi"), + }, + }, }, initialResources: resources{}, finalResources: resources{ @@ -762,7 +773,7 @@ func TestUpsert(t *testing.T) { err := gatekeeper.Upsert(context.Background(), tc.gateway, tc.gatewayClassConfig, tc.helmConfig) require.NoError(t, err) - require.NoError(t, validateResourcesExist(t, client, tc.finalResources)) + require.NoError(t, validateResourcesExist(t, client, tc.helmConfig, tc.finalResources)) }) } } @@ -951,7 +962,7 @@ func TestDelete(t *testing.T) { Name: tc.gateway.Name, }) require.NoError(t, err) - require.NoError(t, validateResourcesExist(t, client, tc.finalResources)) + require.NoError(t, validateResourcesExist(t, client, tc.helmConfig, tc.finalResources)) require.NoError(t, validateResourcesAreDeleted(t, client, tc.initialResources)) }) } @@ -981,7 +992,7 @@ func joinResources(resources resources) (objs []client.Object) { return objs } -func validateResourcesExist(t *testing.T, client client.Client, resources resources) error { +func validateResourcesExist(t *testing.T, client client.Client, helmConfig common.HelmConfig, resources resources) error { t.Helper() for _, expected := range resources.deployments { @@ -1008,6 +1019,21 @@ func validateResourcesExist(t *testing.T, client client.Client, resources resour require.EqualValues(t, *expected.Spec.Replicas, *actual.Spec.Replicas) } + // Ensure there is an init container + hasInitContainer := false + for _, container := range actual.Spec.Template.Spec.InitContainers { + if container.Name == injectInitContainerName { + hasInitContainer = true + + // If the Helm config specifies init container resources, verify they are set + if helmConfig.InitContainerResources != nil { + assert.Equal(t, helmConfig.InitContainerResources.Limits, container.Resources.Limits) + assert.Equal(t, helmConfig.InitContainerResources.Requests, container.Resources.Requests) + } + } + } + assert.True(t, hasInitContainer) + // Ensure there is a consul-dataplane container dropping ALL capabilities, adding // back the NET_BIND_SERVICE capability, and establishing a read-only root filesystem hasDataplaneContainer := false @@ -1343,3 +1369,9 @@ func configureServiceAccount(name, namespace string, labels map[string]string, r }, } } + +func requireQuantity(t *testing.T, v string) resource.Quantity { + quantity, err := resource.ParseQuantity(v) + require.NoError(t, err) + return quantity +} diff --git a/control-plane/api-gateway/gatekeeper/init.go b/control-plane/api-gateway/gatekeeper/init.go index f3d4ad1f95..1d57123fed 100644 --- a/control-plane/api-gateway/gatekeeper/init.go +++ b/control-plane/api-gateway/gatekeeper/init.go @@ -11,9 +11,10 @@ import ( corev1 "k8s.io/api/core/v1" + "k8s.io/utils/pointer" + "github.com/hashicorp/consul-k8s/control-plane/api-gateway/common" "github.com/hashicorp/consul-k8s/control-plane/namespaces" - "k8s.io/utils/pointer" ) const ( @@ -168,6 +169,10 @@ func initContainer(config common.HelmConfig, name, namespace string) (corev1.Con }) } + if config.InitContainerResources != nil { + container.Resources = *config.InitContainerResources + } + // Openshift Assigns the security context for us, do not enable if it is enabled. if !config.EnableOpenShift { container.SecurityContext = &corev1.SecurityContext{ diff --git a/control-plane/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index e97d51f03f..3eeb6dd084 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -537,6 +537,7 @@ func (c *Command) Run(args []string) int { ConsulTLSServerName: c.consul.TLSServerName, ConsulPartition: c.consul.Partition, ConsulCACert: string(caCertPem), + InitContainerResources: &initResources, }, AllowK8sNamespacesSet: allowK8sNamespaces, DenyK8sNamespacesSet: denyK8sNamespaces, From 44809cbef5ff728e83eb5e20cf326472c20207ce Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 7 Feb 2024 10:44:42 -0500 Subject: [PATCH 2/2] Include init containers when determining if existing Deployment equal to desired --- .../api-gateway/gatekeeper/deployment.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/control-plane/api-gateway/gatekeeper/deployment.go b/control-plane/api-gateway/gatekeeper/deployment.go index f3e9545e57..5ab5e2a7c6 100644 --- a/control-plane/api-gateway/gatekeeper/deployment.go +++ b/control-plane/api-gateway/gatekeeper/deployment.go @@ -6,18 +6,19 @@ package gatekeeper import ( "context" - "github.com/hashicorp/consul-k8s/control-plane/api-gateway/common" - "github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1" - "k8s.io/apimachinery/pkg/types" - + "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" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + + "github.com/hashicorp/consul-k8s/control-plane/api-gateway/common" + "github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1" ) const ( @@ -168,6 +169,14 @@ func compareDeployments(a, b *appsv1.Deployment) bool { if len(b.Spec.Template.Spec.Containers) != len(a.Spec.Template.Spec.Containers) { return false } + + for i, containerA := range a.Spec.Template.Spec.InitContainers { + containerB := b.Spec.Template.Spec.InitContainers[i] + if !cmp.Equal(containerA, containerB) { + 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) {