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

fix: do not error if failed to push image #390

Merged
merged 2 commits into from
Nov 5, 2024
Merged

Conversation

johnstcn
Copy link
Member

@johnstcn johnstcn commented Oct 15, 2024

Relates to #385

This adds an option ENVBUILDER_EXIT_ON_PUSH_FAILURE that controls the previous behaviour of exiting with an error if ENVBUILDER_PUSH_IMAGE was set to a truthy value and we failed to push the image.

Before, Envbuilder would always exit with an error on a failed push.
Now, Envbuilder will only exit with an error if ENVBUILDER_EXIT_ON_PUSH_FAILURE is set to a truthy value. Otherwise, Envbuilder will continue building the image.

#3: Pushing image to localhost:5000/cache
#3: ⚠️️ Failed to push image! [1.835942ms]
#4: 🔄 Updating the ownership of the workspace...
#4: 👤 Updated the ownership of the workspace! [406.355µs]
#5: 🔄 Updating ownership of /home/coder...
#5: 🏡 Updated ownership of /home/coder! [56.162µs]
=== Running init command as user "coder": ["/bin/sh" "-c" "bash"]
🐳 bedd93b40e11 envbuilder-starter-devcontainer # whoami
coder

@johnstcn johnstcn self-assigned this Oct 15, 2024
@johnstcn
Copy link
Member Author

johnstcn commented Oct 16, 2024

Almost done here, just need to get the test for this working properly.

EDIT: I ended up choosing a different solution. My original idea was to expose an environment variable with the error resulting from a push, but that ended up being a less intuitive and more complex change than just adding a companion option to ENVBUILDER_EXIT_ON_BUILD_FAILURE.

@johnstcn johnstcn marked this pull request as ready for review November 4, 2024 22:30
Copy link
Contributor

@SasSwart SasSwart left a comment

Choose a reason for hiding this comment

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

LGTM! Glad we have this now.

options/options.go Outdated Show resolved Hide resolved
@johnstcn johnstcn merged commit b18cd0e into main Nov 5, 2024
4 checks passed
@johnstcn johnstcn deleted the cj/385-push-image-fail branch November 5, 2024 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.

3 participants