Skip to content

engine: add flag to disable push on custom build#1341

Merged
jazzdan merged 4 commits into
masterfrom
dmiller/ch1940/add-flag-to-disable-push-on-custom-build
Mar 22, 2019
Merged

engine: add flag to disable push on custom build#1341
jazzdan merged 4 commits into
masterfrom
dmiller/ch1940/add-flag-to-disable-push-on-custom-build

Conversation

@jazzdan

@jazzdan jazzdan commented Mar 20, 2019

Copy link
Copy Markdown
Contributor

@rohansingh needs this for his bazel + KIND setup. For the moment bazel is much better at doing the layer negotiation and pushing only what is necessary. kind load is less efficient. @rohansingh has a workaround where he tees the image to both docker and KIND, allowing custom_build to verify that the image was created correctly and allowing KIND to get the image in a fast way. What he needs is a way to disable the image push in this case.

I decided to add this as a flag to custom_build because its a pretty special case, only is used from custom_builds right now and fits in with custom_build's evolving, experimental API.

@jazzdan
jazzdan requested review from dbentley and maiamcc March 20, 2019 20:36
_, err := f.ibd.BuildAndDeploy(f.ctx, f.st, buildTargets(manifest), store.BuildStateSet{})
// the image doesn't exist, so the custom build fails
// TODO(dmiller): change this test to use the static tag parameter when that's added
assert.Error(t, err)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you add an assertion that this is the error you expect, and not some other error?

@jazzdan jazzdan Mar 20, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, but I probably should have made this more clear: this test doesn't do anything at the moment because the tag is generated by the build and deployer and I can't know what it is to inject it in to the fake docker client.

Thus the TODO to change this test to use the static tag parameter that @maiamcc is going to add. So really this is more of a stub, and once that parameter is added I can inject the static image tag in to the fake docker client and the build and deployer won't return an error at all.

@maiamcc maiamcc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm. i apologize in advance for the merge conflicts.

if iTarget.IsCustomBuild() {
cbSkip = iTarget.CustomBuildInfo().DisablePush
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

tbh you could just to cbSkip := iTarget.CustomBuildInfo().DisablePush (or even take all this out and in the conditional below, just put iTarget.CustomBuildInfo().DisablePush)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Couldn't it be nil though?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nah, if it's not a custom build, this func returns an EMPTY CustomBuild (not a nil one). though i guess you're right that that's maybe not functionality we want to rely on...?

@jazzdan

jazzdan commented Mar 21, 2019

Copy link
Copy Markdown
Contributor Author

@maiamcc no worries, I'll wait for you to merge yours in and then resolve the conflicts over here. :D

@maiamcc

maiamcc commented Mar 22, 2019

Copy link
Copy Markdown
Contributor

k, it's merged! glhfdd.

@jazzdan
jazzdan force-pushed the dmiller/ch1940/add-flag-to-disable-push-on-custom-build branch from 900f80e to 00f0e63 Compare March 22, 2019 15:52
@jazzdan
jazzdan merged commit 0bed070 into master Mar 22, 2019
@nicks
nicks deleted the dmiller/ch1940/add-flag-to-disable-push-on-custom-build branch March 31, 2021 17:30
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