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

cni-plugin: v1.1.3 #10855

Merged
merged 7 commits into from
May 5, 2023
Merged

cni-plugin: v1.1.3 #10855

merged 7 commits into from
May 5, 2023

Conversation

hawkw
Copy link
Contributor

@hawkw hawkw commented May 4, 2023

This release of the CNI plugin changes the base runtime Docker image
from debian:bullseye-slim to alpine:3.17.3.


This release of the CNI plugin changes the base runtime Docker image
from `debian:bullseye-slim` to `scratch.

---

* cni: use `scratch` as the base runtime docker image (linkerd/linkerd2-proxy-init#237)
@hawkw hawkw marked this pull request as ready for review May 4, 2023 17:25
@hawkw hawkw requested a review from a team as a code owner May 4, 2023 17:25
Copy link
Member

@alpeb alpeb left a comment

Choose a reason for hiding this comment

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

There are still entries for v1.1.1 in

cli/cmd/testdata/install_cni_helm_override_output.golden
cli/cmd/install_cni_helm_test.go

(those tests don't actually test the plugin itself, just the charts rendering, but it's still nice to keep 'em up to date).

Copy link
Member

Choose a reason for hiding this comment

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

I think this file was added by mistake

@hawkw hawkw marked this pull request as draft May 4, 2023 19:39
hawkw added a commit to linkerd/linkerd2-proxy-init that referenced this pull request May 4, 2023
PR #237 changed the base runtime image for the CNI init container from
`debian:bullseye-slim` to `scratch`, in the hopes of reducing the
dependency footprint of that image. Unfortunately, it turns out that
this breaks the CNI init container, since it must execute shell scripts
and therefore requires a shell (specifically, `bash`), which does not
exist in the `scratch` image. This was not detected by CI in this repo
on PR #237, because it turns out that the CI jobs don't actually try to
run the CNI init container. Instead, we didn't discover that this breaks
stuff until the change was fully integrated in linkerd/linkerd2#10855.

This commit changes the base runtime image again, to `alpine:3.17.3`,
similarly to the proxy-init dockerfile. We now install the necessary
dependencies using `apk add` in the alpine layer. This way, we should
have a shell, and be able to run the install script again.
hawkw added a commit to linkerd/linkerd2-proxy-init that referenced this pull request May 4, 2023
PR #237 changed the base runtime image for the CNI init container from
`debian:bullseye-slim` to `scratch`, in the hopes of reducing the
dependency footprint of that image. Unfortunately, it turns out that
this breaks the CNI init container, since it must execute shell scripts
and therefore requires a shell (specifically, `bash`), which does not
exist in the `scratch` image. This was not detected by CI in this repo
on PR #237, because it turns out that the CI jobs don't actually try to
run the CNI init container. Instead, we didn't discover that this breaks
stuff until the change was fully integrated in linkerd/linkerd2#10855.

This commit changes the base runtime image again, to `alpine:3.17.3`,
similarly to the proxy-init dockerfile. We now install the necessary
dependencies using `apk add` in the alpine layer. We also install `bash`,
because Alpine doesn't ship with `bash` by default. This way, we should
have a shell, and be able to run the install script again.
@hawkw hawkw marked this pull request as ready for review May 4, 2023 23:16
@hawkw hawkw requested review from alpeb and a team May 4, 2023 23:19
@hawkw hawkw changed the title cni-plugin: v1.1.2 cni-plugin: v1.1.3 May 4, 2023
@hawkw hawkw enabled auto-merge (squash) May 4, 2023 23:22
Copy link
Member

@alpeb alpeb left a comment

Choose a reason for hiding this comment

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

LGTM 👍 🌮

@hawkw hawkw merged commit 4c7a9ab into main May 5, 2023
@hawkw hawkw deleted the eliza/cni-v1.1.2 branch May 5, 2023 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants