image build support #122
image build support #122
Conversation
| exec bool | ||
| volumeFlags string | ||
| // @todo need to think about a better way to handle this. | ||
| runtimeFlags driver.RuntimeFlags |
There was a problem hiding this comment.
Driver must not know about runtime flags. You can save flags in the struct, but move it back to runtime.
| @@ -105,6 +106,7 @@ const ( | |||
| ImagePull // ImagePull - image is being pulled from the registry. | |||
| ImageBuild // ImageBuild - image is being built. | |||
| ImageRemoved // ImageRemoved - image was removed | |||
| ImagePostpone // ImagePostpone - image was removed | |||
| @@ -26,6 +26,7 @@ type ContainerRunner interface { | |||
| ContainerKill(ctx context.Context, cid, signal string) error | |||
| ContainerRemove(ctx context.Context, cid string) error | |||
| Close() error | |||
| SetRuntimeFlags(flags RuntimeFlags) | |||
There was a problem hiding this comment.
It must not be in the driver.
| @@ -156,6 +163,19 @@ func (c *runtimeContainer) GetFlags() *FlagsGroup { | |||
| Type: jsonschema.Boolean, | |||
| Default: false, | |||
| }, | |||
| &DefParameter{ | |||
| Name: containerRegistryURL, | |||
| Title: "k8s Images registry URL", | |||
There was a problem hiding this comment.
Use Kubernetes for all frontend strings. k8s is mostly for development. It seems we need to be able to specify registry in the global config. We may as well have multiple registries.
| @@ -25,7 +26,19 @@ import ( | |||
| "github.com/launchrctl/launchr/internal/launchr" | |||
| ) | |||
|
|
|||
| // RegistryLocal defines a local registry type | |||
| const RegistryLocal = "local" | |||
There was a problem hiding this comment.
I think it can go to Type. Registries are the same for all containers. Define a type specific to these strings like it's done for log level
|
|
||
| script, err := executeTemplate(buildahBuildTemplate, buildData) | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to generate image build script: %w", err) |
There was a problem hiding this comment.
Same here, we must panic because we control the whole process.
| Name: k8sBuildPodContainer, | ||
| Image: "quay.io/buildah/stable:latest", | ||
| SecurityContext: &corev1.SecurityContext{ | ||
| Privileged: &[]bool{true}[0], |
There was a problem hiding this comment.
Consider moving it to a variable, it's hard to read.
| containers = append(containers, buildahContainer) | ||
| } | ||
|
|
||
| // @todo should we add internal type which includes registry as sidecar and builds everything inside pod? |
There was a problem hiding this comment.
Good question. I think it may be the closest approach to Docker if we run actions in the cloud and we may not even specify registry. We may add registry in a separate feature with that. The problem is that when action stops, the registry is lost, but it's not very important for the moment.
| func (k *k8sRuntime) ImageRemove(_ context.Context, _ string, _ ImageRemoveOptions) (*ImageRemoveResponse, error) { | ||
| // @todo it doesn't really work well with current implementation. | ||
| // additional issue here is kubernetes internal cache, additionally to registry storage. | ||
| // should we clean both of them in case of image remove? How 'no-cache' flag should behave? |
There was a problem hiding this comment.
We must at least remove it from the registry. For the cache issue, you may want to specify ImagePullPolicy: Always when creating a container.
| // Store image options inside runtime. | ||
| k.imageOptions = imgOpts | ||
|
|
||
| // Return nothing to silence ImageEnsure(), as real work will be done inside k.ContainerStart(). |
There was a problem hiding this comment.
Normally we can create an image on ContainerCreate because after that we have a working pod. But it seems we may need to define extra step before ContainerCreate to prepare pods and images.
No description provided.