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

Switch to distroless consul-dataplane image with Envoy 1.24 #1676

Merged
merged 23 commits into from
Nov 10, 2022

Conversation

boxofrad
Copy link
Contributor

@boxofrad boxofrad commented Nov 3, 2022

Changes proposed in this PR:

This updates consul-k8s to use distroless images for consul-dataplane (hashicorp/consul-dataplane#15). This also updates Envoy to 1.24, and switches consul-dataplane commands to pass -proxy-service-id-path.

How I've tested this PR:

How I expect reviewers to test this PR:

Checklist:

  • Tests added
  • CHANGELOG entry added

    HashiCorp engineers only, community PRs should not add a changelog entry.
    Entries should use present tense (e.g. Add support for...)

Comment on lines 356 to 360
expArgs := []string{
"-addresses 1.1.1.1 -grpc-port=8502 -proxy-service-id-path=/consul/connect-inject/proxyid-web " +
"-service-node-name=k8s-service-mesh -log-level=info -log-json=false -envoy-concurrency=0 -tls-disabled -envoy-admin-bind-port=19000 -- --base-id 0",
"-addresses 1.1.1.1 -grpc-port=8502 -proxy-service-id-path=/consul/connect-inject/proxyid-web-admin " +
"-service-node-name=k8s-service-mesh -log-level=info -log-json=false -envoy-concurrency=0 -tls-disabled -envoy-admin-bind-port=19001 -- --base-id 1",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did this for expediency, but we should probably match on the Args slice rather than joining it into a string.

@@ -572,7 +572,7 @@ global:
# The name (and tag) of the consul-dataplane Docker image used for the
# connect-injected sidecar proxies and mesh, terminating, and ingress gateways.
# @default: hashicorp/consul-dataplane:<latest supported version>
imageConsulDataplane: hashicorp/consul-dataplane:1.0.0-beta2
imageConsulDataplane: duptonhashicorp/consul-dataplane:distroless-f7c2a05
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need a "real" image 😅 (this one is from my laptop and has some extra debugging nonsense in)

@pglass pglass marked this pull request as ready for review November 8, 2022 15:48
@pglass pglass changed the title Switch to distroless consul-dataplane image Switch to distroless consul-dataplane image with Envoy 1.24 Nov 8, 2022
Copy link
Contributor

@ishustava ishustava left a comment

Choose a reason for hiding this comment

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

Looks great!! For the failing metrics test, would you mind removing the image that we're setting here because we now want to use the newer image:

"global.imageConsulDataplane": "hashicorppreview/consul-dataplane:1.0-dev",
?

@@ -33,6 +33,7 @@ func TestCreateOrUpdateACLPolicy_ErrorsIfDescriptionDoesNotMatch(t *testing.T) {
c.ACL.Tokens.InitialManagement = bootToken
})
require.NoError(err)
defer svr.Stop()
Copy link
Contributor

Choose a reason for hiding this comment

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

🙏

@david-yu
Copy link
Contributor

david-yu commented Nov 8, 2022

Thank you @boxofrad and @pglass! Amazing work!

@pglass Could we make sure to include a changelog prior to merging this in? Would like to ensure that we've covered that so folks know we now run on a distroless version of Envoy.

@pglass
Copy link
Contributor

pglass commented Nov 8, 2022

I merged the consul-dataplane change, so the latest hashicorppreview/consul-dataplane:1.0-dev is distroless, so I've updated this to use that image for now.

CHANGELOG.md Show resolved Hide resolved
Copy link
Contributor

@thisisnotashwin thisisnotashwin left a comment

Choose a reason for hiding this comment

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

💥

- name: DP_CREDENTIAL_LOGIN_META1
value: pod=$(NAMESPACE)/$(POD_NAME)
- name: DP_CREDENTIAL_LOGIN_META2
value: component=ingress-gateway
Copy link
Contributor

Choose a reason for hiding this comment

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

Because there is no shell in the distroless image, we can't read environment variables within the command. Instead, we set env vars that consul-dataplane understands.

Depends on hashicorp/consul-dataplane#47

@pglass pglass merged commit aa5a8dc into main Nov 10, 2022
@pglass pglass deleted the boxofrad/distroless branch November 10, 2022 23:17
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.

6 participants