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

Allow image name overrides in environment override files #618

Merged
merged 3 commits into from
Aug 19, 2021

Conversation

chriswalker
Copy link
Contributor

Allow users to override the images in the environment override files, and persist these in the generated k8s manifests. This allows users to specify different images for different environments.

This change adds a new Image field to the ServiceConfig struct, and modifies the post-reconciliation merging to copy over override image names, if present. Unit tests are modeled on those checking env var overrides, and ensure that the reconciliation processing does not remove any specified image names in the overrides.

This PR replaces #610.

@marcinc marcinc self-requested a review August 19, 2021 09:45
Copy link
Contributor

@marcinc marcinc left a comment

Choose a reason for hiding this comment

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

1 minor comment. Otherwise LGTM.

@@ -314,6 +314,12 @@ func (o *composeOverride) mergeServicesInto(p *ComposeProject) error {

envVarsFromNilToBlankInService(base)

// Copy over image name if one has bee defined in the override. In
// future this may expand to invlude other fields.
if override.Image != "" && override.Image != base.Image {
Copy link
Contributor

Choose a reason for hiding this comment

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

minor nitpick. It'd be better to trim whitespaces and check for length instead in this condition I think.

Copy link
Contributor

@marcinc marcinc left a comment

Choose a reason for hiding this comment

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

LGTM

@chriswalker chriswalker merged commit 93fb3d2 into master Aug 19, 2021
@chriswalker chriswalker deleted the 601-update-image branch August 19, 2021 12:50
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.

2 participants