-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add ability to build images in play kube #11180
Conversation
if err != nil { | ||
return nil, err | ||
} | ||
if (len(buildFile) > 0 && !existsLocally) || (len(buildFile) > 0 && options.Build) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a point to exitstsLocally
is this triggers with or without it? Are options.Build
and buildFile
exclusive to one another?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems incorrect - the previous code used a pull policy of Newer, which will pull if a newer image is available on the registry - this will not happen because of the existsLocally
check here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basic thinking: if an image exists locally and you run in a directory that contains a folder with the name of the image, even if --build
is not given, we will not pull the image if a newer version exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@baude Did you see this one?
e3dbdb4
to
f19038b
Compare
return prefix | ||
} | ||
|
||
func getBuildFile(imageName string, cwd string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do a similar thing in Buildah. Might be nice at some point to have a routine in some common area that both projects could use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: baude, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
When playing a kube YAML file, it can be desirable to be able to build an image on the fly. This is good for development of an image and YAML files and somewhat mocks what compose does. Signed-off-by: Brent Baude <bbaude@redhat.com>
/lgtm |
When playing a kube YAML file, it can be desirable to be able to build
an image on the fly. This is good for development of an image and YAML
files and somewhat mocks what compose does.
Signed-off-by: Brent Baude bbaude@redhat.com