diff --git a/flytectl/cmd/config/subcommand/sandbox/config_flags.go b/flytectl/cmd/config/subcommand/sandbox/config_flags.go index 15c7c6f78f..5339211026 100755 --- a/flytectl/cmd/config/subcommand/sandbox/config_flags.go +++ b/flytectl/cmd/config/subcommand/sandbox/config_flags.go @@ -53,5 +53,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.StringVar(&DefaultConfig.Source, fmt.Sprintf("%v%v", prefix, "source"), DefaultConfig.Source, "Path of your source code") cmdFlags.StringVar(&DefaultConfig.Version, fmt.Sprintf("%v%v", prefix, "version"), DefaultConfig.Version, "Version of flyte. Only supports flyte releases greater than v0.10.0") cmdFlags.StringVar(&DefaultConfig.Image, fmt.Sprintf("%v%v", prefix, "image"), DefaultConfig.Image, "Optional. Provide a fully qualified path to a Flyte compliant docker image.") + cmdFlags.Var(&DefaultConfig.ImagePullPolicy, fmt.Sprintf("%v%v", prefix, "imagePullPolicy"), "Optional. Defines the image pull behavior [Always/IfNotPresent/Never]") return cmdFlags } diff --git a/flytectl/cmd/config/subcommand/sandbox/config_flags_test.go b/flytectl/cmd/config/subcommand/sandbox/config_flags_test.go index cd58322bb6..4e1410c228 100755 --- a/flytectl/cmd/config/subcommand/sandbox/config_flags_test.go +++ b/flytectl/cmd/config/subcommand/sandbox/config_flags_test.go @@ -141,4 +141,16 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_imagePullPolicy", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "1" + + cmdFlags.Set("imagePullPolicy", testValue) + if v := cmdFlags.Lookup("imagePullPolicy"); v != nil { + testDecodeJson_Config(t, fmt.Sprintf("%v", v.Value.String()), &actual.ImagePullPolicy) + + } + }) + }) } diff --git a/flytectl/cmd/config/subcommand/sandbox/imagepullpolicy_enumer.go b/flytectl/cmd/config/subcommand/sandbox/imagepullpolicy_enumer.go new file mode 100644 index 0000000000..8416741819 --- /dev/null +++ b/flytectl/cmd/config/subcommand/sandbox/imagepullpolicy_enumer.go @@ -0,0 +1,69 @@ +// Code generated by "enumer -type=ImagePullPolicy -trimprefix=ImagePullPolicy --json"; DO NOT EDIT. + +// +package sandbox + +import ( + "encoding/json" + "fmt" +) + +const _ImagePullPolicyName = "AlwaysIfNotPresentNever" + +var _ImagePullPolicyIndex = [...]uint8{0, 6, 18, 23} + +func (i ImagePullPolicy) String() string { + if i < 0 || i >= ImagePullPolicy(len(_ImagePullPolicyIndex)-1) { + return fmt.Sprintf("ImagePullPolicy(%d)", i) + } + return _ImagePullPolicyName[_ImagePullPolicyIndex[i]:_ImagePullPolicyIndex[i+1]] +} + +var _ImagePullPolicyValues = []ImagePullPolicy{0, 1, 2} + +var _ImagePullPolicyNameToValueMap = map[string]ImagePullPolicy{ + _ImagePullPolicyName[0:6]: 0, + _ImagePullPolicyName[6:18]: 1, + _ImagePullPolicyName[18:23]: 2, +} + +// ImagePullPolicyString retrieves an enum value from the enum constants string name. +// Throws an error if the param is not part of the enum. +func ImagePullPolicyString(s string) (ImagePullPolicy, error) { + if val, ok := _ImagePullPolicyNameToValueMap[s]; ok { + return val, nil + } + return 0, fmt.Errorf("%s does not belong to ImagePullPolicy values", s) +} + +// ImagePullPolicyValues returns all values of the enum +func ImagePullPolicyValues() []ImagePullPolicy { + return _ImagePullPolicyValues +} + +// IsAImagePullPolicy returns "true" if the value is listed in the enum definition. "false" otherwise +func (i ImagePullPolicy) IsAImagePullPolicy() bool { + for _, v := range _ImagePullPolicyValues { + if i == v { + return true + } + } + return false +} + +// MarshalJSON implements the json.Marshaler interface for ImagePullPolicy +func (i ImagePullPolicy) MarshalJSON() ([]byte, error) { + return json.Marshal(i.String()) +} + +// UnmarshalJSON implements the json.Unmarshaler interface for ImagePullPolicy +func (i *ImagePullPolicy) UnmarshalJSON(data []byte) error { + var s string + if err := json.Unmarshal(data, &s); err != nil { + return fmt.Errorf("ImagePullPolicy should be a string, got %s", data) + } + + var err error + *i, err = ImagePullPolicyString(s) + return err +} diff --git a/flytectl/cmd/config/subcommand/sandbox/sandbox_config.go b/flytectl/cmd/config/subcommand/sandbox/sandbox_config.go index 9a787757a7..ff7559b562 100644 --- a/flytectl/cmd/config/subcommand/sandbox/sandbox_config.go +++ b/flytectl/cmd/config/subcommand/sandbox/sandbox_config.go @@ -1,11 +1,36 @@ package sandbox +//go:generate enumer -type=ImagePullPolicy -trimprefix=ImagePullPolicy --json +type ImagePullPolicy int + +const ( + ImagePullPolicyAlways ImagePullPolicy = iota + ImagePullPolicyIfNotPresent + ImagePullPolicyNever +) + +// Set implements PFlag's Value interface to attempt to set the value of the flag from string. +func (i *ImagePullPolicy) Set(val string) error { + policy, err := ImagePullPolicyString(val) + if err != nil { + return err + } + + *i = policy + return nil +} + +// Type implements PFlag's Value interface to return type name. +func (i ImagePullPolicy) Type() string { + return "ImagePullPolicy" +} + //go:generate pflags Config --default-var DefaultConfig --bind-default-var var ( DefaultConfig = &Config{} ) -//Config +//Config holds configuration flags for sandbox command. type Config struct { Source string `json:"source" pflag:",Path of your source code"` @@ -18,4 +43,8 @@ type Config struct { // Flyte compliant sandbox image. Usually useful, if you want to push the image to your own registry and relaunch // from there. Image string `json:"image" pflag:",Optional. Provide a fully qualified path to a Flyte compliant docker image."` + + // Optionally it is possible to use local sandbox image + // If local flag pass then flytectl will not pull image from registry. Usually useful, if you want to test your local images without pushing them to a registry + ImagePullPolicy ImagePullPolicy `json:"imagePullPolicy" pflag:",Optional. Defines the image pull behavior [Always/IfNotPresent/Never]"` } diff --git a/flytectl/cmd/sandbox/start.go b/flytectl/cmd/sandbox/start.go index 8f9518a793..720b6f1a1d 100644 --- a/flytectl/cmd/sandbox/start.go +++ b/flytectl/cmd/sandbox/start.go @@ -56,6 +56,11 @@ Specify a Flyte Sandbox compliant image with the registry. This is useful, in ca flytectl sandbox start --image docker.io/my-override:latest + +Specify a Flyte Sandbox image pull policy. Possible pull policy values are Always, IfNotPresent, or Never +:: + + flytectl sandbox start --image docker.io/my-override:latest --imagePullPolicy Always Usage ` k8sEndpoint = "https://127.0.0.1:30086" @@ -143,7 +148,8 @@ func startSandbox(ctx context.Context, cli docker.Docker, reader io.Reader) (*bu return nil, err } fmt.Printf("%v pulling docker image for release %s\n", emoji.Whale, image) - if err := docker.PullDockerImage(ctx, cli, image); err != nil { + + if err := docker.PullDockerImage(ctx, cli, image, sandboxConfig.DefaultConfig.ImagePullPolicy); err != nil { return nil, err } diff --git a/flytectl/pkg/docker/docker.go b/flytectl/pkg/docker/docker.go index 1cc53b6285..cb08092b5f 100644 --- a/flytectl/pkg/docker/docker.go +++ b/flytectl/pkg/docker/docker.go @@ -24,6 +24,7 @@ type Docker interface { ContainerExecCreate(ctx context.Context, container string, config types.ExecConfig) (types.IDResponse, error) ContainerExecAttach(ctx context.Context, execID string, config types.ExecStartCheck) (types.HijackedResponse, error) ContainerExecInspect(ctx context.Context, execID string) (types.ContainerExecInspect, error) + ImageList(ctx context.Context, listOption types.ImageListOptions) ([]types.ImageSummary, error) } type FlyteDocker struct { diff --git a/flytectl/pkg/docker/docker_util.go b/flytectl/pkg/docker/docker_util.go index a6afbec299..29551decab 100644 --- a/flytectl/pkg/docker/docker_util.go +++ b/flytectl/pkg/docker/docker_util.go @@ -9,6 +9,8 @@ import ( "os" "strings" + sandboxConfig "github.com/flyteorg/flytectl/cmd/config/subcommand/sandbox" + "github.com/flyteorg/flytectl/clierrors" "github.com/docker/docker/api/types" @@ -88,14 +90,30 @@ func GetSandboxPorts() (map[nat.Port]struct{}, map[nat.Port][]nat.PortBinding, e } // PullDockerImage will Pull docker image -func PullDockerImage(ctx context.Context, cli Docker, image string) error { - r, err := cli.ImagePull(ctx, image, types.ImagePullOptions{}) - if err != nil { +func PullDockerImage(ctx context.Context, cli Docker, image string, pullPolicy sandboxConfig.ImagePullPolicy) error { + if pullPolicy == sandboxConfig.ImagePullPolicyAlways || pullPolicy == sandboxConfig.ImagePullPolicyIfNotPresent { + if pullPolicy == sandboxConfig.ImagePullPolicyIfNotPresent { + imageSummary, err := cli.ImageList(ctx, types.ImageListOptions{}) + if err != nil { + return err + } + for _, img := range imageSummary { + for _, tags := range img.RepoTags { + if image == tags { + return nil + } + } + } + } + r, err := cli.ImagePull(ctx, image, types.ImagePullOptions{}) + if err != nil { + return err + } + + _, err = io.Copy(os.Stdout, r) return err } - - _, err = io.Copy(os.Stdout, r) - return err + return nil } //StartContainer will create and start docker container diff --git a/flytectl/pkg/docker/docker_util_test.go b/flytectl/pkg/docker/docker_util_test.go index 5aa2b34896..0a40d55ea2 100644 --- a/flytectl/pkg/docker/docker_util_test.go +++ b/flytectl/pkg/docker/docker_util_test.go @@ -8,6 +8,8 @@ import ( "strings" "testing" + sandboxConfig "github.com/flyteorg/flytectl/cmd/config/subcommand/sandbox" + f "github.com/flyteorg/flytectl/pkg/filesystemutils" "github.com/docker/docker/api/types/container" @@ -101,13 +103,13 @@ func TestRemoveSandboxWithNoReply(t *testing.T) { } func TestPullDockerImage(t *testing.T) { - t.Run("Successfully pull image", func(t *testing.T) { + t.Run("Successfully pull image Always", func(t *testing.T) { setupSandbox() mockDocker := &mocks.Docker{} context := context.Background() // Verify the attributes mockDocker.OnImagePullMatch(context, mock.Anything, types.ImagePullOptions{}).Return(os.Stdin, nil) - err := PullDockerImage(context, mockDocker, "nginx") + err := PullDockerImage(context, mockDocker, "nginx:latest", sandboxConfig.ImagePullPolicyAlways) assert.Nil(t, err) }) @@ -117,10 +119,28 @@ func TestPullDockerImage(t *testing.T) { context := context.Background() // Verify the attributes mockDocker.OnImagePullMatch(context, mock.Anything, types.ImagePullOptions{}).Return(os.Stdin, fmt.Errorf("error")) - err := PullDockerImage(context, mockDocker, "nginx") + err := PullDockerImage(context, mockDocker, "nginx:latest", sandboxConfig.ImagePullPolicyAlways) assert.NotNil(t, err) }) + t.Run("Successfully pull image IfNotPresent", func(t *testing.T) { + setupSandbox() + mockDocker := &mocks.Docker{} + context := context.Background() + // Verify the attributes + mockDocker.OnImagePullMatch(context, mock.Anything, types.ImagePullOptions{}).Return(os.Stdin, nil) + mockDocker.OnImageListMatch(context, types.ImageListOptions{}).Return([]types.ImageSummary{}, nil) + err := PullDockerImage(context, mockDocker, "nginx:latest", sandboxConfig.ImagePullPolicyIfNotPresent) + assert.Nil(t, err) + }) + + t.Run("Successfully pull image Never", func(t *testing.T) { + setupSandbox() + mockDocker := &mocks.Docker{} + context := context.Background() + err := PullDockerImage(context, mockDocker, "nginx:latest", sandboxConfig.ImagePullPolicyNever) + assert.Nil(t, err) + }) } func TestStartContainer(t *testing.T) { diff --git a/flytectl/pkg/docker/mocks/docker.go b/flytectl/pkg/docker/mocks/docker.go index 9655d46a9c..b08cf0d51a 100644 --- a/flytectl/pkg/docker/mocks/docker.go +++ b/flytectl/pkg/docker/mocks/docker.go @@ -368,6 +368,47 @@ func (_m *Docker) ContainerWait(ctx context.Context, containerID string, conditi return r0, r1 } +type Docker_ImageList struct { + *mock.Call +} + +func (_m Docker_ImageList) Return(_a0 []types.ImageSummary, _a1 error) *Docker_ImageList { + return &Docker_ImageList{Call: _m.Call.Return(_a0, _a1)} +} + +func (_m *Docker) OnImageList(ctx context.Context, listOption types.ImageListOptions) *Docker_ImageList { + c := _m.On("ImageList", ctx, listOption) + return &Docker_ImageList{Call: c} +} + +func (_m *Docker) OnImageListMatch(matchers ...interface{}) *Docker_ImageList { + c := _m.On("ImageList", matchers...) + return &Docker_ImageList{Call: c} +} + +// ImageList provides a mock function with given fields: ctx, listOption +func (_m *Docker) ImageList(ctx context.Context, listOption types.ImageListOptions) ([]types.ImageSummary, error) { + ret := _m.Called(ctx, listOption) + + var r0 []types.ImageSummary + if rf, ok := ret.Get(0).(func(context.Context, types.ImageListOptions) []types.ImageSummary); ok { + r0 = rf(ctx, listOption) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]types.ImageSummary) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, types.ImageListOptions) error); ok { + r1 = rf(ctx, listOption) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + type Docker_ImagePull struct { *mock.Call }