Split proxy-init into separate repo #2824

alpeb · 2019-05-15T00:09:32Z

The new repo is https://github.com/linkerd/linkerd2-proxy-init, and I
tagged the latest there stable-2.3.0. The plan would be to tag it each
time it is updated, using the same tag of the linkerd2 repo that
required that change. I think that helps keeping track of which
proxy-init version is required by which linkerd2 version. Let me know if
you have a different opinion!

Here, I've removed the /proxy-init dir and pinned the injected
proxy-init version to stable-2.3.0 in the injector code and tests.

/cni-plugin depends on proxy-init, so I updated the import paths
there, and could verify CNI is still working (there is some flakiness
but unrelated to this PR).

For consistency, I added a --init-image-version flag to linkerd inject along with its corresponding override config annotation. If
deemed too much, we can hide it (along with --init-image).

l5d-bot · 2019-05-15T00:32:08Z

Integration test results for 4dc2000: fail 😕
Log output: https://gist.github.com/ba47e2271aaf335be55dfa2588846a0e

siggy

looks (and works) good.

fwiw, l5d-bot is failing because of an issue with l5d-bot, this branch is fine.

i think i have a slight preference for versioning the proxy-init repo separately from the main linkerd2 repo. my primary concern is that it could cause confusion for users, when they install stable-2.4.0, and they see a reference to stable-2.3.0 in their install output.

for our base Docker image, we use a date:

linkerd2/Dockerfile-proxy

Line 1 in b965d0d

ARG RUNTIME_IMAGE=gcr.io/linkerd-io/base:2019-02-19.01

for linkerd2-proxy, we use a sha:

linkerd2/bin/docker-build-proxy

Lines 25 to 26 in b965d0d

    
           # Default to a pinned commit SHA of the proxy. 
        
           PROXY_VERSION="${PROXY_VERSION:-5f89351}"

...or, we could just do regular v0.0.x versioning, which would better conform to Go Modules support ;)

... on a more meta point (and apologies for the lateness) i did not realize until just now that linkerd-cni has a code-level Go dependency on proxy-init. this could make further development in that area more painful. stepping back a bit, what are we trying to achieve with this change? if it's a faster build, it may be worthwhile to explore other options. thoughts @alpeb @olix0r ?

Gopkg.toml

olix0r · 2019-05-15T14:56:52Z

i think i have a slight preference for versioning the proxy-init repo separately from the main linkerd2 repo.

I have a strong preference to keep them separate ;) Can we just call the proxy-init 1.0.0?

siggy · 2019-05-16T09:26:28Z

i did not realize until just now that linkerd-cni has a code-level Go dependency on proxy-init. this could make further development in that area more painful.

reflecting on this a bit, given how infrequently the proxy-init code changes, this is probably not a big deal.

I have a strong preference to keep them separate ;) Can we just call the proxy-init 1.0.0?

agree, let's tag things as v1.0.0 (per https://github.com/golang/go/wiki/Modules#modules)

alpeb · 2019-05-16T14:46:47Z

Thanks for the feedback!
I've tagged https://github.com/linkerd/linkerd2-proxy-init to v1.0.0, but I have no permissions to push the docker image into google cloud, which will make the integration tests fail.
Can anyone assist?

l5d-bot · 2019-05-16T14:56:43Z

Integration test results for 841f1e8: fail 😕
Log output: https://gist.github.com/d536ab939e0f22599485c14b4e5c8558

siggy

i just realized l5d-bot is likely failing due to a few straggling references in /bin:

$ grep -R proxy-init bin
bin/docker-images:for img in cli-bin cni-plugin controller debug grafana proxy proxy-init web  ; do
bin/docker-pull:for img in cli-bin cni-plugin controller debug grafana proxy proxy-init web  ; do
bin/docker-retag-all:for img in cli-bin cni-plugin controller debug grafana proxy proxy-init web  ; do
bin/docker-push:for img in cli-bin cni-plugin controller debug grafana proxy proxy-init web  ; do

there's also a Integration tests: proxy-init section in TEST.md that probably needs updating.

siggy

good to go assuming all tests pass 👍 🚢

left one comment about a comment. also there's a few more references to proxy-init in BUILD.md that should be removed.

proto/config/config.proto

l5d-bot · 2019-05-16T16:20:01Z

Integration test results for 05d6f6c: fail 😕
Log output: https://gist.github.com/e6974b4fe230a371c1320fdf40eb4444

siggy

left a comment around adding one more known k8s event to integration tests.

looks like some unit test files need updating too?
https://travis-ci.org/linkerd/linkerd2/jobs/533554338#L1160

test/install_test.go

l5d-bot · 2019-05-17T15:13:41Z

Integration test results for 6df0df2: success 🎉
Log output: https://gist.github.com/d11e76dc07ff859e23a71a8b8055599a

cli/cmd/root.go

pkg/inject/inject.go

pkg/version/version.go

l5d-bot · 2019-05-28T21:02:10Z

Integration test results for 7843470: success 🎉
Log output: https://gist.github.com/0717d1028fcf40481d979453eb61f9ce

l5d-bot · 2019-05-28T22:35:43Z

Integration test results for 8bbbeca: fail 😕
Log output: https://gist.github.com/b3019d289b251927b96dfdb6b99cbe91

Fixes #2563 The new repo is https://github.com/linkerd/linkerd2-proxy-init, and I tagged the latest there `stable-2.3.0`. The plan would be to tag it each time it is updated, using the same tag of the linkerd2 repo that required that change. I think that helps keeping track of which proxy-init version is required by which linkerd2 version. Let me know if you have a different opinion! Here, I've removed the `/proxy-init` dir and pinned the injected proxy-init version to `stable-2.3.0` in the injector code and tests. `/cni-plugin` depends on proxy-init, so I updated the import paths there, and could verify CNI is still working (there is some flakiness but unrelated to this PR). For consistency, I added a `--init-image-version` flag to `linkerd inject` along with its corresponding override config annotation. If deemed too much, we can hide it (along with `--init-image`). Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

proxy-init in TEST.md into the proxy-init repo Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

…en there are no overrides Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

l5d-bot · 2019-06-03T20:31:32Z

Integration test results for bc603e1: success 🎉
Log output: https://gist.github.com/7a38c37e8140aae0514510ee8eade9c7

Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

ihcsim

👍 Thanks for addressing all the comments. Once the new annotation label is renamed to match the CLI option, feel free to merge this PR.

Here are the different install, inject and upgrade test cases I used:

# expect pods creation to fail due to image tag error
bin/linkerd install --init-image-version=v2.0.0 |k apply -f -

# expect proxy.proxyInitImageVersion to be v2.0.0
k -n linkerd describe cm linkerd-config

# expect pods creation to succeed
bin/linkerd install --ignore-cluster |k apply -f -

# expect proxy.proxyInitImageVersion to be v1.0.0
k -n linkerd describe cm linkerd-config

# expect injected pod to use init image v1.0.0
 curl https://run.linkerd.io/emojivoto.yml | bin/linkerd inject - | k apply -f -

# expect upgrade to succeed
bin/linkerd upgrade|k apply -f -

# expect proxy.proxyInitImageVersion to be v1.0.0
k -n linkerd describe cm linkerd-config

# expect pods creation to fail due to image tag error
bin/linkerd upgrade --init-image-version=v3.0.0|k apply -f -

# expect proxy.proxyInitImageVersion to be v3.0.0
k -n linkerd describe cm linkerd-config

# expect pods creation to fail because it's pulling v3.0.0 from linkerd-config
bin/linkerd upgrade|k apply -f -

# expect pods creation to succeed
bin/linkerd upgrade --init-image-version=v1.0.0|k apply -f -

# expect proxy.proxyInitImageVersion to be v1.0.0
k -n linkerd describe cm linkerd-config

# expect injected pod to use init image v1.0.0
curl https://run.linkerd.io/emojivoto.yml | bin/linkerd inject - | k apply -f -

# install edge-19.5.4, upgrade to HEAD
linkerd version
Client version: edge-19.5.4
Server version: unavailable

# expect pod creation to succeed
linkerd install|k apply -f -

# expect no proxy.proxyInitImageVersion key/value in CM
k -n linkerd describe cm linkerd-config

# expect pods creation to succeed
bin/linkerd upgrade|k apply -f -

# expect proxy.proxyInitImageVersion to be v1.0.0
k -n linkerd describe cm linkerd-config

# test with annotation and auto-inject
# annotations:
#   config.linkerd.io/proxy-init-version: v3.0.0
# ....
# annotations:
#   config.linkerd.io/proxy-init-version: v1.0.0

l5d-bot · 2019-06-03T21:42:44Z

Integration test results for f1b164e: success 🎉
Log output: https://gist.github.com/9ccd538d9bf1401d4262b1897b010519

alpeb self-assigned this May 15, 2019

siggy reviewed May 15, 2019

View reviewed changes

Gopkg.toml Outdated Show resolved Hide resolved

siggy reviewed May 16, 2019

View reviewed changes

alpeb force-pushed the alpeb/spinoff-proxy-init branch from 53036ad to 05d6f6c Compare May 16, 2019 15:49

siggy approved these changes May 16, 2019

View reviewed changes

proto/config/config.proto Show resolved Hide resolved

alpeb force-pushed the alpeb/spinoff-proxy-init branch 2 times, most recently from 1c608e4 to 85d15a5 Compare May 16, 2019 22:36

siggy approved these changes May 17, 2019

View reviewed changes

test/install_test.go Outdated Show resolved Hide resolved

siggy added area/build area/controller labels May 17, 2019

ihcsim reviewed May 22, 2019

View reviewed changes

cli/cmd/root.go Show resolved Hide resolved

pkg/inject/inject.go Show resolved Hide resolved

pkg/version/version.go Show resolved Hide resolved

alpeb force-pushed the alpeb/spinoff-proxy-init branch from 6df0df2 to 7843470 Compare May 28, 2019 20:36

alpeb force-pushed the alpeb/spinoff-proxy-init branch 4 times, most recently from af65676 to 8bbbeca Compare May 28, 2019 21:58

alpeb and others added 3 commits June 3, 2019 14:46

Added comment for ProxyInitVersion

3bc0ea8

Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

Pin proxy-init to v1.0.0 and added comments to Gopkg.toml and version.go

f58bdd0

Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

alpeb and others added 6 commits June 3, 2019 14:46

Remove proxy-init refs in bin/docker-* files, move test info for

e58fa66

proxy-init in TEST.md into the proxy-init repo Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

Fixed comment and remove more proxy-init references

6d08274

Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

Update tests

cc1074f

Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

Validate init image version

5b23216

Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

Include ProxyInitImageVersion in Proxy config, and check its value wh…

91d9660

…en there are no overrides Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

Set default initImageVersion for install

bc603e1

Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

alpeb force-pushed the alpeb/spinoff-proxy-init branch from 8bbbeca to bc603e1 Compare June 3, 2019 20:04

Change annotation key to match CLI option

f1b164e

Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

ihcsim approved these changes Jun 3, 2019

View reviewed changes

alpeb merged commit 74ca92e into master Jun 3, 2019

alpeb deleted the alpeb/spinoff-proxy-init branch June 3, 2019 21:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split proxy-init into separate repo #2824

Split proxy-init into separate repo #2824

alpeb commented May 15, 2019

l5d-bot commented May 15, 2019

siggy left a comment •

edited

Loading

olix0r commented May 15, 2019

siggy commented May 16, 2019

alpeb commented May 16, 2019

l5d-bot commented May 16, 2019

siggy left a comment

siggy left a comment

l5d-bot commented May 16, 2019

siggy left a comment

l5d-bot commented May 17, 2019

l5d-bot commented May 28, 2019

l5d-bot commented May 28, 2019

l5d-bot commented Jun 3, 2019

ihcsim left a comment

l5d-bot commented Jun 3, 2019

	# Default to a pinned commit SHA of the proxy.
	PROXY_VERSION="${PROXY_VERSION:-5f89351}"

Split proxy-init into separate repo #2824

Split proxy-init into separate repo #2824

Conversation

alpeb commented May 15, 2019

l5d-bot commented May 15, 2019

siggy left a comment • edited Loading

Choose a reason for hiding this comment

olix0r commented May 15, 2019

siggy commented May 16, 2019

alpeb commented May 16, 2019

l5d-bot commented May 16, 2019

siggy left a comment

Choose a reason for hiding this comment

siggy left a comment

Choose a reason for hiding this comment

l5d-bot commented May 16, 2019

siggy left a comment

Choose a reason for hiding this comment

l5d-bot commented May 17, 2019

l5d-bot commented May 28, 2019

l5d-bot commented May 28, 2019

l5d-bot commented Jun 3, 2019

ihcsim left a comment

Choose a reason for hiding this comment

l5d-bot commented Jun 3, 2019

siggy left a comment •

edited

Loading