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

Bump Envoy image version to v1.24.0.0-prod #657

Merged
merged 2 commits into from
Nov 21, 2022
Merged

Conversation

ytsssun
Copy link
Contributor

@ytsssun ytsssun commented Nov 17, 2022

Issue #, if available:

Description of changes:
Bump AppMesh Envoy image version from v1.22.2.1-prod to v1.24.0.0-prod
.
Ref: #621

Envoy v1.24.0 release notes: https://www.envoyproxy.io/docs/envoy/latest/version_history/v1.24/v1.24.0

Note: please monitor this aws/aws-app-mesh-roadmap#442 and make sure the v1.24.0.0 Envoy image is available in public ECR before merging this PR.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@BennettJames
Copy link
Contributor

I believe the makefile should be updated as well -

@ytsssun
Copy link
Contributor Author

ytsssun commented Nov 17, 2022

I believe the makefile should be updated as well -

* https://github.com/aws/aws-app-mesh-controller-for-k8s/blob/master/Makefile#L12

Checked with @paulyeo21 offline, looks like that version indicates the beginning of the agent feature waitUntilProxyReady, hence should not be updated.

@suniltheta
Copy link
Contributor

Better to rename that variable & add comments to avoid confusion.

@BennettJames
Copy link
Contributor

Hrm, yeah I think we might need to think through this policy a bit more then. The integration tests as-is will call make integration-test, which in turn will call make helm-deploy which uses that variable. Having the integration tests not use the default-configured envoy does not seem appropriate to me.

@rakeb rakeb requested a review from suniltheta November 18, 2022 09:15
@rakeb rakeb assigned rakeb and unassigned rakeb Nov 18, 2022
@BennettJames
Copy link
Contributor

BennettJames commented Nov 18, 2022

Yeah, having thought about this a bit more, I'm pretty sure we should increment that variable. There are still ways to test older versions by setting that variable (i.e. make integration-test SIDECAR_IMAGE_TAG=v1.22.2.1-prod), but as-is it acts as a default for controller deploys. The default should be consistently set to the latest. The variable in the Makefile can be downgraded if desired by a user.

I think there's a separate value check w/in the sidecar builder that's necessary for waitUntilProxyReady. That should stay the same, but I believe the makefile needs updating.

@ytsssun
Copy link
Contributor Author

ytsssun commented Nov 18, 2022

Synced with @rakeb, I think I was wrong about the Makefile. The Makefile is only related to the Integ test. We need to include the version update. Updated the PR.

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.

4 participants