Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge proxy_init image with proxyv2 image #17615

Merged
merged 9 commits into from
Oct 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ initContainers:
{{- else }}
image: "{{ .Values.global.hub }}/{{ .Values.global.proxy_init.image }}:{{ .Values.global.tag }}"
{{- end }}
args:
command:
- istio-iptables
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember exactly - but for 'distroless' ( without a shell ) I thought it's a special format, and includes full path ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Distroless still has $PATH I think, this seemed to work fine, and we have distroless tests

- "-p"
- "15001"
- "-z"
Expand Down
4 changes: 2 additions & 2 deletions install/kubernetes/helm/istio/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,8 @@ global:
tracer: "zipkin"

proxy_init:
# Base name for the proxy_init container, used to configure iptables.
image: proxy_init
# Base name for the istio-init container, used to configure iptables.
image: proxyv2

# imagePullPolicy is applied to istio control plane components.
# local tests require IfNotPresent, to avoid uploading to dockerhub.
Expand Down
2 changes: 2 additions & 0 deletions pilot/docker/Dockerfile.proxy_debug
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ FROM docker.io/istio/base:${BASE_VERSION} as default
ARG proxy_version
ARG istio_version

COPY istio-iptables.sh /usr/local/bin/istio-iptables
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use the same file name we had before ? No reason to rename stuff.

I believe the pilot startup script for mesh expansion is loading this file as well.

I thought we already include it...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need distroless and ubuntu to share the same file name. So I think its either this or make the distroless one .sh when its not really a shell script? Or is there a way around make them the same name?


# Install Envoy.
COPY envoy /usr/local/bin/envoy

Expand Down
27 changes: 0 additions & 27 deletions pilot/docker/Dockerfile.proxy_init

This file was deleted.

3 changes: 1 addition & 2 deletions pilot/docker/Dockerfile.proxytproxy
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,10 @@ COPY envoy /usr/local/bin/envoy

COPY pilot-agent /usr/local/bin/pilot-agent


COPY envoy_pilot.yaml.tmpl /etc/istio/proxy/envoy_pilot.yaml.tmpl
COPY envoy_policy.yaml.tmpl /etc/istio/proxy/envoy_policy.yaml.tmpl
COPY envoy_telemetry.yaml.tmpl /etc/istio/proxy/envoy_telemetry.yaml.tmpl
COPY istio-iptables.sh /usr/local/bin/istio-iptables.sh
COPY istio-iptables.sh /usr/local/bin/istio-iptables

# Copy Envoy bootstrap templates used by pilot-agent
COPY envoy_bootstrap_v2.json /var/lib/istio/envoy/envoy_bootstrap_tmpl.json
Expand Down
11 changes: 10 additions & 1 deletion pilot/docker/Dockerfile.proxyv2
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,20 @@ COPY gcp_envoy_bootstrap.json /var/lib/istio/envoy/gcp_envoy_bootstrap_tmpl.json

RUN chown -R istio-proxy /var/lib/istio

COPY istio-iptables.sh /usr/local/bin/istio-iptables

# The following section is used as base image if BASE_DISTRIBUTION=distroless
# hadolint ignore=DL3007
FROM gcr.io/distroless/cc:latest as distroless

# TODO(https://github.com/istio/istio/issues/17656) clean up this hack
COPY --from=default /sbin/xtables-multi /sbin/iptables* /sbin/ip6tables* /sbin/ip /sbin/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the part I don't like about distroless, and I'm pretty worried this will create problems.

This will override the 'distroless' lib with the libs from default. Is the default on the same glibc as distroless ?

( we do have this problem for envoy as well, but only in the sense that envoy build may be on an incompatible glibc and not work - we're not replacing/overriding .so files in the distroless base ).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this either, but for what its worth this is not new in this PR, it is copied over from proxy_init.

COPY --from=default /usr/lib/x86_64-linux-gnu/xtables/ /usr/lib/x86_64-linux-gnu/xtables
COPY --from=default /usr/lib/x86_64-linux-gnu/ /usr/lib/x86_64-linux-gnu
COPY --from=default /etc/iproute2 /etc/iproute2

COPY istio-iptables /usr/local/bin/istio-iptables

# Copy Envoy bootstrap templates used by pilot-agent
COPY envoy_bootstrap_v2.json /var/lib/istio/envoy/envoy_bootstrap_tmpl.json
COPY envoy_bootstrap_drain.json /var/lib/istio/envoy/envoy_bootstrap_drain.json
Expand All @@ -43,7 +53,6 @@ COPY pilot-agent /usr/local/bin/pilot-agent
COPY envoy_pilot.yaml.tmpl /etc/istio/proxy/envoy_pilot.yaml.tmpl
COPY envoy_policy.yaml.tmpl /etc/istio/proxy/envoy_policy.yaml.tmpl
COPY envoy_telemetry.yaml.tmpl /etc/istio/proxy/envoy_telemetry.yaml.tmpl
COPY istio-iptables.sh /usr/local/bin/istio-iptables.sh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confused - why do you move this line up ? (and again, why rename )

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In distroless we use the golang implementation, in ubuntu we use bash. If it is here then its adding the bash one to distroless


# The pilot-agent will bootstrap Envoy.
ENTRYPOINT ["/usr/local/bin/pilot-agent"]
16 changes: 0 additions & 16 deletions pkg/kube/inject/inject.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,22 +165,6 @@ type SidecarTemplateData struct {
Values map[string]interface{}
}

// InitImageName returns the fully qualified image name for the istio
// init image given a docker hub and tag and debug flag
func InitImageName(hub string, tag string, _ bool) string {
return hub + "/proxy_init:" + tag
}

// ProxyImageName returns the fully qualified image name for the istio
// proxy image given a docker hub and tag and whether to use debug or not.
func ProxyImageName(hub string, tag string, debug bool) string {
// Allow overriding the proxy image.
if debug {
return hub + "/proxy_debug:" + tag
}
return hub + "/proxyv2:" + tag
}

// Params describes configurable parameters for injecting istio proxy
// into a kubernetes resource.
type Params struct {
Expand Down
18 changes: 18 additions & 0 deletions pkg/kube/inject/inject_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,24 @@ var (
statusPattern = regexp.MustCompile("sidecar.istio.io/status: '{\"version\":\"([0-9a-f]+)\",")
)

// InitImageName returns the fully qualified image name for the istio
// init image given a docker hub and tag and debug flag
// This is used for testing only
func InitImageName(hub string, tag string, _ bool) string {
return hub + "/proxy_init:" + tag
}

// ProxyImageName returns the fully qualified image name for the istio
// proxy image given a docker hub and tag and whether to use debug or not.
// This is used for testing
func ProxyImageName(hub string, tag string, debug bool) string {
// Allow overriding the proxy image.
if debug {
return hub + "/proxy_debug:" + tag
}
return hub + "/proxyv2:" + tag
}

func TestImageName(t *testing.T) {
want := "docker.io/istio/proxy_init:latest"
if got := InitImageName("docker.io/istio", "latest", true); got != want {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ spec:
annotations:
sidecar.istio.io/interceptionMode: REDIRECT
sidecar.istio.io/rewriteAppHTTPProbers: "true"
sidecar.istio.io/status: '{"version":"3ae2237c99cecd220c8a89781e3929298793b967a9e628986e30bf4ec51a57db","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
sidecar.istio.io/status: '{"version":"c42a97ce43602dca6a2d3e26520fbc36ee25f621e95186edb4b65f0231b11301","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
traffic.sidecar.istio.io/excludeInboundPorts: "15020"
traffic.sidecar.istio.io/includeInboundPorts: 80,90
creationTimestamp: null
Expand Down Expand Up @@ -169,7 +169,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ spec:
annotations:
sidecar.istio.io/interceptionMode: REDIRECT
sidecar.istio.io/rewriteAppHTTPProbers: "false"
sidecar.istio.io/status: '{"version":"3ae2237c99cecd220c8a89781e3929298793b967a9e628986e30bf4ec51a57db","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
sidecar.istio.io/status: '{"version":"c42a97ce43602dca6a2d3e26520fbc36ee25f621e95186edb4b65f0231b11301","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
traffic.sidecar.istio.io/excludeInboundPorts: "15020"
traffic.sidecar.istio.io/includeInboundPorts: 80,90
creationTimestamp: null
Expand Down Expand Up @@ -164,7 +164,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ spec:
metadata:
annotations:
sidecar.istio.io/interceptionMode: REDIRECT
sidecar.istio.io/status: '{"version":"3ae2237c99cecd220c8a89781e3929298793b967a9e628986e30bf4ec51a57db","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
sidecar.istio.io/status: '{"version":"c42a97ce43602dca6a2d3e26520fbc36ee25f621e95186edb4b65f0231b11301","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
traffic.sidecar.istio.io/excludeInboundPorts: "15020"
traffic.sidecar.istio.io/includeInboundPorts: 80,90
creationTimestamp: null
Expand Down Expand Up @@ -165,7 +165,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ spec:
metadata:
annotations:
sidecar.istio.io/interceptionMode: REDIRECT
sidecar.istio.io/status: '{"version":"3ae2237c99cecd220c8a89781e3929298793b967a9e628986e30bf4ec51a57db","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
sidecar.istio.io/status: '{"version":"c42a97ce43602dca6a2d3e26520fbc36ee25f621e95186edb4b65f0231b11301","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
traffic.sidecar.istio.io/excludeInboundPorts: "15020"
traffic.sidecar.istio.io/includeInboundPorts: "80"
creationTimestamp: null
Expand Down Expand Up @@ -145,7 +145,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ spec:
metadata:
annotations:
sidecar.istio.io/interceptionMode: REDIRECT
sidecar.istio.io/status: '{"version":"3ae2237c99cecd220c8a89781e3929298793b967a9e628986e30bf4ec51a57db","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
sidecar.istio.io/status: '{"version":"c42a97ce43602dca6a2d3e26520fbc36ee25f621e95186edb4b65f0231b11301","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
traffic.sidecar.istio.io/excludeInboundPorts: "15020"
traffic.sidecar.istio.io/includeInboundPorts: 80,90
creationTimestamp: null
Expand Down Expand Up @@ -166,7 +166,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ spec:
metadata:
annotations:
sidecar.istio.io/interceptionMode: REDIRECT
sidecar.istio.io/status: '{"version":"3ae2237c99cecd220c8a89781e3929298793b967a9e628986e30bf4ec51a57db","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
sidecar.istio.io/status: '{"version":"c42a97ce43602dca6a2d3e26520fbc36ee25f621e95186edb4b65f0231b11301","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
traffic.sidecar.istio.io/excludeInboundPorts: "15020"
traffic.sidecar.istio.io/includeInboundPorts: "80"
creationTimestamp: null
Expand Down Expand Up @@ -145,7 +145,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ spec:
metadata:
annotations:
sidecar.istio.io/interceptionMode: REDIRECT
sidecar.istio.io/status: '{"version":"3ae2237c99cecd220c8a89781e3929298793b967a9e628986e30bf4ec51a57db","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
sidecar.istio.io/status: '{"version":"c42a97ce43602dca6a2d3e26520fbc36ee25f621e95186edb4b65f0231b11301","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
traffic.sidecar.istio.io/excludeInboundPorts: "15020"
traffic.sidecar.istio.io/includeInboundPorts: "80"
creationTimestamp: null
Expand Down Expand Up @@ -149,7 +149,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ spec:
metadata:
annotations:
sidecar.istio.io/interceptionMode: REDIRECT
sidecar.istio.io/status: '{"version":"f8a379b90185a9584d1a682f9b9c68e07eb9578c18965f315f25424994bf6458","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
sidecar.istio.io/status: '{"version":"c42a97ce43602dca6a2d3e26520fbc36ee25f621e95186edb4b65f0231b11301","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
traffic.sidecar.istio.io/excludeInboundPorts: "15020"
traffic.sidecar.istio.io/includeInboundPorts: 80,90
creationTimestamp: null
Expand Down Expand Up @@ -165,7 +165,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ spec:
metadata:
annotations:
sidecar.istio.io/interceptionMode: REDIRECT
sidecar.istio.io/status: '{"version":"3ae2237c99cecd220c8a89781e3929298793b967a9e628986e30bf4ec51a57db","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
sidecar.istio.io/status: '{"version":"c42a97ce43602dca6a2d3e26520fbc36ee25f621e95186edb4b65f0231b11301","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
traffic.sidecar.istio.io/excludeInboundPorts: "15020"
traffic.sidecar.istio.io/includeInboundPorts: "80"
creationTimestamp: null
Expand Down Expand Up @@ -145,7 +145,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ spec:
metadata:
annotations:
sidecar.istio.io/interceptionMode: REDIRECT
sidecar.istio.io/status: '{"version":"3ae2237c99cecd220c8a89781e3929298793b967a9e628986e30bf4ec51a57db","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
sidecar.istio.io/status: '{"version":"c42a97ce43602dca6a2d3e26520fbc36ee25f621e95186edb4b65f0231b11301","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'
traffic.sidecar.istio.io/excludeInboundPorts: "15020"
traffic.sidecar.istio.io/includeInboundPorts: 80,90
creationTimestamp: null
Expand Down Expand Up @@ -156,7 +156,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
3 changes: 2 additions & 1 deletion pkg/kube/inject/testdata/inject/auth.yaml.injected
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
3 changes: 2 additions & 1 deletion pkg/kube/inject/testdata/inject/cronjob.yaml.injected
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
3 changes: 2 additions & 1 deletion pkg/kube/inject/testdata/inject/daemonset.yaml.injected
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ spec:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ items:
name: istio-certs
readOnly: true
initContainers:
- args:
- command:
- istio-iptables
- -p
- "15001"
- -z
Expand Down
Loading