Skip to content

Commit

Permalink
Split the init container into two init containers. One will cp the co…
Browse files Browse the repository at this point in the history
…nsul bin and the other will retain all of the existing behaviours (#441)

Co-authored-by: Iryna Shustava <ishustava@users.noreply.github.com>
  • Loading branch information
2 people authored and Ashwin Venkatesh committed Mar 26, 2021
1 parent 3c5e456 commit d133d0f
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 30 deletions.
32 changes: 24 additions & 8 deletions connect-inject/container_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import (
)

const (
InjectInitContainerName = "consul-connect-inject-init"
MetaKeyPodName = "pod-name"
MetaKeyKubeNS = "k8s-namespace"
InjectInitCopyContainerName = "copy-consul-bin"
InjectInitContainerName = "consul-connect-inject-init"
MetaKeyPodName = "pod-name"
MetaKeyKubeNS = "k8s-namespace"
)

type initContainerCommandData struct {
Expand Down Expand Up @@ -63,6 +64,25 @@ type initContainerCommandUpstreamData struct {
Query string
}

// containerInitCopyContainer returns the init container spec for the copy container which places
// the consul binary into the shared volume.
func (h *Handler) containerInitCopyContainer() corev1.Container {
// Copy the Consul binary from the image to the shared volume.
cmd := "cp /bin/consul /consul/connect-inject/consul"
return corev1.Container{
Name: InjectInitCopyContainerName,
Image: h.ImageConsul,
Resources: h.InitContainerResources,
VolumeMounts: []corev1.VolumeMount{
corev1.VolumeMount{
Name: volumeName,
MountPath: "/consul/connect-inject",
},
},
Command: []string{"/bin/sh", "-ec", cmd},
}
}

// containerInit returns the init container spec for registering the Consul
// service, setting up the Envoy bootstrap, etc.
func (h *Handler) containerInit(pod *corev1.Pod, k8sNamespace string) (corev1.Container, error) {
Expand Down Expand Up @@ -452,8 +472,4 @@ chmod 444 /consul/connect-inject/acl-token
{{- if .ConsulNamespace }}
-namespace="{{ .ConsulNamespace }}" \
{{- end }}
-bootstrap > /consul/connect-inject/envoy-bootstrap.yaml
# Copy the Consul binary
cp /bin/consul /consul/connect-inject/consul
`
-bootstrap > /consul/connect-inject/envoy-bootstrap.yaml`
34 changes: 14 additions & 20 deletions connect-inject/container_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,7 @@ EOF
# Generate the envoy bootstrap code
/bin/consul connect envoy \
-proxy-id="${PROXY_SERVICE_ID}" \
-bootstrap > /consul/connect-inject/envoy-bootstrap.yaml
# Copy the Consul binary
cp /bin/consul /consul/connect-inject/consul`,
-bootstrap > /consul/connect-inject/envoy-bootstrap.yaml`,
"",
},

Expand Down Expand Up @@ -795,10 +792,7 @@ EOF
/bin/consul connect envoy \
-proxy-id="${PROXY_SERVICE_ID}" \
-namespace="default" \
-bootstrap > /consul/connect-inject/envoy-bootstrap.yaml
# Copy the Consul binary
cp /bin/consul /consul/connect-inject/consul`,
-bootstrap > /consul/connect-inject/envoy-bootstrap.yaml`,
"",
},

Expand Down Expand Up @@ -871,10 +865,7 @@ EOF
/bin/consul connect envoy \
-proxy-id="${PROXY_SERVICE_ID}" \
-namespace="non-default" \
-bootstrap > /consul/connect-inject/envoy-bootstrap.yaml
# Copy the Consul binary
cp /bin/consul /consul/connect-inject/consul`,
-bootstrap > /consul/connect-inject/envoy-bootstrap.yaml`,
"",
},

Expand Down Expand Up @@ -956,10 +947,7 @@ chmod 444 /consul/connect-inject/acl-token
-proxy-id="${PROXY_SERVICE_ID}" \
-token-file="/consul/connect-inject/acl-token" \
-namespace="non-default" \
-bootstrap > /consul/connect-inject/envoy-bootstrap.yaml
# Copy the Consul binary
cp /bin/consul /consul/connect-inject/consul`,
-bootstrap > /consul/connect-inject/envoy-bootstrap.yaml`,
"",
},

Expand Down Expand Up @@ -1042,10 +1030,7 @@ chmod 444 /consul/connect-inject/acl-token
-proxy-id="${PROXY_SERVICE_ID}" \
-token-file="/consul/connect-inject/acl-token" \
-namespace="k8snamespace" \
-bootstrap > /consul/connect-inject/envoy-bootstrap.yaml
# Copy the Consul binary
cp /bin/consul /consul/connect-inject/consul`,
-bootstrap > /consul/connect-inject/envoy-bootstrap.yaml`,
"",
},

Expand Down Expand Up @@ -1446,3 +1431,12 @@ func TestHandlerContainerInit_MeshGatewayModeErrors(t *testing.T) {
}

}

// Test that the init copy container has the correct command.
func TestHandlerContainerInitCopyContainer(t *testing.T) {
require := require.New(t)
h := Handler{}
container := h.containerInitCopyContainer()
actual := strings.Join(container.Command, " ")
require.Contains(actual, `cp /bin/consul /consul/connect-inject/consul`)
}
7 changes: 5 additions & 2 deletions connect-inject/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,9 +348,12 @@ func (h *Handler) Mutate(req *v1beta1.AdmissionRequest) *v1beta1.AdmissionRespon
fmt.Sprintf("/spec/containers/%d/env", i))...)
}

// Add the init container which copies the consul binary.
initCopyContainer := h.containerInitCopyContainer()

// Add the init container that registers the service and sets up
// the Envoy configuration.
container, err := h.containerInit(&pod, req.Namespace)
initContainer, err := h.containerInit(&pod, req.Namespace)
if err != nil {
h.Log.Error("Error configuring injection init container", "err", err, "Request Name", req.Name)
return &v1beta1.AdmissionResponse{
Expand All @@ -361,7 +364,7 @@ func (h *Handler) Mutate(req *v1beta1.AdmissionRequest) *v1beta1.AdmissionRespon
}
patches = append(patches, addContainer(
pod.Spec.InitContainers,
[]corev1.Container{container},
[]corev1.Container{initCopyContainer, initContainer},
"/spec/initContainers")...)

// Add the Envoy and Consul sidecars.
Expand Down
20 changes: 20 additions & 0 deletions connect-inject/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ func TestHandlerHandle(t *testing.T) {
Operation: "add",
Path: "/spec/initContainers",
},
{
Operation: "add",
Path: "/spec/initContainers/-",
},
{
Operation: "add",
Path: "/spec/containers/-",
Expand Down Expand Up @@ -164,6 +168,10 @@ func TestHandlerHandle(t *testing.T) {
Operation: "add",
Path: "/spec/initContainers",
},
{
Operation: "add",
Path: "/spec/initContainers/-",
},
{
Operation: "add",
Path: "/spec/containers/-",
Expand Down Expand Up @@ -237,6 +245,10 @@ func TestHandlerHandle(t *testing.T) {
Operation: "add",
Path: "/spec/initContainers",
},
{
Operation: "add",
Path: "/spec/initContainers/-",
},
{
Operation: "add",
Path: "/spec/containers/-",
Expand Down Expand Up @@ -283,6 +295,10 @@ func TestHandlerHandle(t *testing.T) {
Operation: "add",
Path: "/spec/initContainers",
},
{
Operation: "add",
Path: "/spec/initContainers/-",
},
{
Operation: "add",
Path: "/spec/containers/-",
Expand Down Expand Up @@ -334,6 +350,10 @@ func TestHandlerHandle(t *testing.T) {
Operation: "add",
Path: "/spec/initContainers",
},
{
Operation: "add",
Path: "/spec/initContainers/-",
},
{
Operation: "add",
Path: "/spec/containers/-",
Expand Down

0 comments on commit d133d0f

Please sign in to comment.