From 4e7aead605a5e1e93b0e8b0f3cada0bdd7a5bfb3 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Tue, 8 Nov 2022 09:58:53 +0100 Subject: [PATCH] Use PodmanCmd and DockerCmd from context --- pkg/deploy/deploy.go | 11 +++--- pkg/deploy/interface.go | 4 ++- pkg/dev/dev.go | 6 ++-- .../adapters/kubernetes/component/adapter.go | 4 ++- .../adapters/kubernetes/component/handler.go | 6 +++- .../kubernetes/component/interface.go | 4 ++- pkg/devfile/image/image.go | 26 ++++++-------- pkg/devfile/image/image_test.go | 34 +++++++------------ pkg/odo/cli/build_images/build_images.go | 2 +- pkg/odo/cli/deploy/deploy.go | 2 +- pkg/watch/watch.go | 15 ++++---- pkg/watch/watch_test.go | 2 +- 12 files changed, 58 insertions(+), 58 deletions(-) diff --git a/pkg/deploy/deploy.go b/pkg/deploy/deploy.go index 1d751771785..62aba0952d1 100644 --- a/pkg/deploy/deploy.go +++ b/pkg/deploy/deploy.go @@ -1,6 +1,7 @@ package deploy import ( + "context" "errors" "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" @@ -26,12 +27,13 @@ func NewDeployClient(kubeClient kclient.ClientInterface) *DeployClient { } } -func (o *DeployClient) Deploy(fs filesystem.Filesystem, devfileObj parser.DevfileObj, path string, appName string, componentName string) error { - deployHandler := newDeployHandler(fs, devfileObj, path, o.kubeClient, appName, componentName) +func (o *DeployClient) Deploy(ctx context.Context, fs filesystem.Filesystem, devfileObj parser.DevfileObj, path string, appName string, componentName string) error { + deployHandler := newDeployHandler(ctx, fs, devfileObj, path, o.kubeClient, appName, componentName) return libdevfile.Deploy(devfileObj, deployHandler) } type deployHandler struct { + ctx context.Context fs filesystem.Filesystem devfileObj parser.DevfileObj path string @@ -42,8 +44,9 @@ type deployHandler struct { var _ libdevfile.Handler = (*deployHandler)(nil) -func newDeployHandler(fs filesystem.Filesystem, devfileObj parser.DevfileObj, path string, kubeClient kclient.ClientInterface, appName string, componentName string) *deployHandler { +func newDeployHandler(ctx context.Context, fs filesystem.Filesystem, devfileObj parser.DevfileObj, path string, kubeClient kclient.ClientInterface, appName string, componentName string) *deployHandler { return &deployHandler{ + ctx: ctx, fs: fs, devfileObj: devfileObj, path: path, @@ -55,7 +58,7 @@ func newDeployHandler(fs filesystem.Filesystem, devfileObj parser.DevfileObj, pa // ApplyImage builds and pushes the OCI image to be used on Kubernetes func (o *deployHandler) ApplyImage(img v1alpha2.Component) error { - return image.BuildPushSpecificImage(o.fs, o.path, img, true) + return image.BuildPushSpecificImage(o.ctx, o.fs, o.path, img, true) } // ApplyKubernetes applies inline Kubernetes YAML from the devfile.yaml file diff --git a/pkg/deploy/interface.go b/pkg/deploy/interface.go index bf324ffab6f..aa91e0bbf1d 100644 --- a/pkg/deploy/interface.go +++ b/pkg/deploy/interface.go @@ -1,6 +1,8 @@ package deploy import ( + "context" + "github.com/devfile/library/pkg/devfile/parser" "github.com/redhat-developer/odo/pkg/testingutil/filesystem" @@ -10,5 +12,5 @@ type Client interface { // Deploy resources from a devfile located in path, for the specified appName. // The filesystem specified is used to download and store the Dockerfiles needed to build the necessary container images, // in case such Dockerfiles are referenced as remote URLs in the Devfile. - Deploy(fs filesystem.Filesystem, devfileObj parser.DevfileObj, path string, appName string, componentName string) error + Deploy(ctx context.Context, fs filesystem.Filesystem, devfileObj parser.DevfileObj, path string, appName string, componentName string) error } diff --git a/pkg/dev/dev.go b/pkg/dev/dev.go index fbaaf4df575..17aa1b87992 100644 --- a/pkg/dev/dev.go +++ b/pkg/dev/dev.go @@ -100,7 +100,7 @@ func (o *DevClient) Start( klog.V(4).Infoln("Creating inner-loop resources for the component") componentStatus := watch.ComponentStatus{} - err = adapter.Push(pushParameters, &componentStatus) + err = adapter.Push(ctx, pushParameters, &componentStatus) if err != nil { return err } @@ -128,7 +128,7 @@ func (o *DevClient) Start( } // RegenerateAdapterAndPush regenerates the adapter and pushes the files to remote pod -func (o *DevClient) regenerateAdapterAndPush(pushParams adapters.PushParameters, watchParams watch.WatchParameters, componentStatus *watch.ComponentStatus) error { +func (o *DevClient) regenerateAdapterAndPush(ctx context.Context, pushParams adapters.PushParameters, watchParams watch.WatchParameters, componentStatus *watch.ComponentStatus) error { var adapter component.ComponentAdapter adapter, err := o.regenerateComponentAdapterFromWatchParams(watchParams) @@ -136,7 +136,7 @@ func (o *DevClient) regenerateAdapterAndPush(pushParams adapters.PushParameters, return fmt.Errorf("unable to generate component from watch parameters: %w", err) } - err = adapter.Push(pushParams, componentStatus) + err = adapter.Push(ctx, pushParams, componentStatus) if err != nil { return fmt.Errorf("watch command was unable to push component: %w", err) } diff --git a/pkg/devfile/adapters/kubernetes/component/adapter.go b/pkg/devfile/adapters/kubernetes/component/adapter.go index 0360e2a87a9..82062d89edf 100644 --- a/pkg/devfile/adapters/kubernetes/component/adapter.go +++ b/pkg/devfile/adapters/kubernetes/component/adapter.go @@ -1,6 +1,7 @@ package component import ( + "context" "errors" "fmt" "path/filepath" @@ -90,7 +91,7 @@ func NewKubernetesAdapter( // Push updates the component if a matching component exists or creates one if it doesn't exist // Once the component has started, it will sync the source code to it. // The componentStatus will be modified to reflect the status of the component when the function returns -func (a Adapter) Push(parameters adapters.PushParameters, componentStatus *watch.ComponentStatus) (err error) { +func (a Adapter) Push(ctx context.Context, parameters adapters.PushParameters, componentStatus *watch.ComponentStatus) (err error) { // preliminary checks err = dfutil.ValidateK8sResourceName("component name", a.ComponentName) @@ -262,6 +263,7 @@ func (a Adapter) Push(parameters adapters.PushParameters, componentStatus *watch devfile: a.Devfile, path: parameters.Path, podName: pod.GetName(), + ctx: ctx, } if commandType == devfilev1.ExecCommandType { diff --git a/pkg/devfile/adapters/kubernetes/component/handler.go b/pkg/devfile/adapters/kubernetes/component/handler.go index 540c3286e51..a0bb9ff70d2 100644 --- a/pkg/devfile/adapters/kubernetes/component/handler.go +++ b/pkg/devfile/adapters/kubernetes/component/handler.go @@ -1,6 +1,8 @@ package component import ( + "context" + devfilev1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/library/pkg/devfile/parser" "github.com/redhat-developer/odo/pkg/component" @@ -23,12 +25,14 @@ type runHandler struct { path string componentExists bool podName string + + ctx context.Context } var _ libdevfile.Handler = (*runHandler)(nil) func (a *runHandler) ApplyImage(img devfilev1.Component) error { - return image.BuildPushSpecificImage(a.fs, a.path, img, true) + return image.BuildPushSpecificImage(a.ctx, a.fs, a.path, img, true) } func (a *runHandler) ApplyKubernetes(kubernetes devfilev1.Component) error { diff --git a/pkg/devfile/adapters/kubernetes/component/interface.go b/pkg/devfile/adapters/kubernetes/component/interface.go index c6be00f3483..40b3a3c88d5 100644 --- a/pkg/devfile/adapters/kubernetes/component/interface.go +++ b/pkg/devfile/adapters/kubernetes/component/interface.go @@ -1,11 +1,13 @@ package component import ( + "context" + "github.com/redhat-developer/odo/pkg/devfile/adapters" "github.com/redhat-developer/odo/pkg/watch" ) // ComponentAdapter defines the functions that platform-specific adapters must implement type ComponentAdapter interface { - Push(parameters adapters.PushParameters, componentStatus *watch.ComponentStatus) error + Push(ctx context.Context, parameters adapters.PushParameters, componentStatus *watch.ComponentStatus) error } diff --git a/pkg/devfile/image/image.go b/pkg/devfile/image/image.go index 1e182d258d1..7bd07a3360e 100644 --- a/pkg/devfile/image/image.go +++ b/pkg/devfile/image/image.go @@ -2,17 +2,20 @@ package image import ( + "context" "errors" - "os" "os/exec" devfile "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/library/pkg/devfile/parser" "github.com/devfile/library/pkg/devfile/parser/data/v2/common" + envcontext "github.com/redhat-developer/odo/pkg/config/context" "github.com/redhat-developer/odo/pkg/libdevfile" "github.com/redhat-developer/odo/pkg/log" "github.com/redhat-developer/odo/pkg/testingutil/filesystem" + + "k8s.io/utils/pointer" ) // Backend is in interface that must be implemented by container runtimes @@ -27,13 +30,12 @@ type Backend interface { } var lookPathCmd = exec.LookPath -var getEnvFunc = os.Getenv // BuildPushImages build all images defined in the devfile with the detected backend // If push is true, also push the images to their registries -func BuildPushImages(fs filesystem.Filesystem, devfileObj parser.DevfileObj, path string, push bool) error { +func BuildPushImages(ctx context.Context, fs filesystem.Filesystem, devfileObj parser.DevfileObj, path string, push bool) error { - backend, err := selectBackend() + backend, err := selectBackend(ctx) if err != nil { return err } @@ -59,8 +61,8 @@ func BuildPushImages(fs filesystem.Filesystem, devfileObj parser.DevfileObj, pat // BuildPushSpecificImage build an image defined in the devfile present in devfilePath // If push is true, also push the image to its registry -func BuildPushSpecificImage(fs filesystem.Filesystem, devfilePath string, component devfile.Component, push bool) error { - backend, err := selectBackend() +func BuildPushSpecificImage(ctx context.Context, fs filesystem.Filesystem, devfilePath string, component devfile.Component, push bool) error { + backend, err := selectBackend(ctx) if err != nil { return err } @@ -91,12 +93,9 @@ func buildPushImage(backend Backend, fs filesystem.Filesystem, image *devfile.Im // selectBackend selects the container backend to use for building and pushing images // It will detect podman and docker CLIs (in this order), // or return an error if none are present locally -func selectBackend() (Backend, error) { +func selectBackend(ctx context.Context) (Backend, error) { - podmanCmd := getEnvFunc("PODMAN_CMD") - if podmanCmd == "" { - podmanCmd = "podman" - } + podmanCmd := pointer.StringDeref(envcontext.GetEnvConfig(ctx).PodmanCmd, "podman") if _, err := lookPathCmd(podmanCmd); err == nil { // Podman does NOT build x86 images on Apple Silicon / M1 and we must *WARN* the user that this will not work. @@ -116,10 +115,7 @@ func selectBackend() (Backend, error) { return NewDockerCompatibleBackend(podmanCmd), nil } - dockerCmd := getEnvFunc("DOCKER_CMD") - if dockerCmd == "" { - dockerCmd = "docker" - } + dockerCmd := pointer.StringDeref(envcontext.GetEnvConfig(ctx).DockerCmd, "docker") if _, err := lookPathCmd(dockerCmd); err == nil { return NewDockerCompatibleBackend(dockerCmd), nil } diff --git a/pkg/devfile/image/image_test.go b/pkg/devfile/image/image_test.go index 4918fca3306..ea0aa5addc0 100644 --- a/pkg/devfile/image/image_test.go +++ b/pkg/devfile/image/image_test.go @@ -1,14 +1,17 @@ package image import ( + "context" "errors" - "os" "os/exec" "testing" devfile "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" gomock "github.com/golang/mock/gomock" + "k8s.io/utils/pointer" + "github.com/redhat-developer/odo/pkg/config" + envcontext "github.com/redhat-developer/odo/pkg/config/context" "github.com/redhat-developer/odo/pkg/testingutil/filesystem" ) @@ -120,7 +123,7 @@ func TestBuildPushImage(t *testing.T) { func TestSelectBackend(t *testing.T) { tests := []struct { name string - getEnvFunc func(string) string + envConfig config.Configuration lookPathCmd func(string) (string, error) wantType string wantErr bool @@ -164,11 +167,8 @@ func TestSelectBackend(t *testing.T) { }, { name: "value of PODMAN_CMD envvar is returned if it points to a valid command", - getEnvFunc: func(name string) string { - if name == "PODMAN_CMD" { - return "my-alternate-podman-command" - } - return "" + envConfig: config.Configuration{ + PodmanCmd: pointer.String("my-alternate-podman-command"), }, lookPathCmd: func(name string) (string, error) { if name == "my-alternate-podman-command" { @@ -181,11 +181,8 @@ func TestSelectBackend(t *testing.T) { }, { name: "docker if PODMAN_CMD points to an invalid command", - getEnvFunc: func(name string) string { - if name == "PODMAN_CMD" { - return "no-such-command" - } - return "" + envConfig: config.Configuration{ + PodmanCmd: pointer.String("no-such-command"), }, lookPathCmd: func(name string) (string, error) { if name == "docker" { @@ -200,18 +197,11 @@ func TestSelectBackend(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.getEnvFunc != nil { - getEnvFunc = tt.getEnvFunc - } else { - getEnvFunc = func(string) string { - //Empty environment - return "" - } - } - defer func() { getEnvFunc = os.Getenv }() lookPathCmd = tt.lookPathCmd defer func() { lookPathCmd = exec.LookPath }() - backend, err := selectBackend() + ctx := context.Background() + ctx = envcontext.WithEnvConfig(ctx, tt.envConfig) + backend, err := selectBackend(ctx) if tt.wantErr != (err != nil) { t.Errorf("%s: Error result wanted %v, got %v", tt.name, tt.wantErr, err != nil) } diff --git a/pkg/odo/cli/build_images/build_images.go b/pkg/odo/cli/build_images/build_images.go index adce90d137f..18519aef4e8 100644 --- a/pkg/odo/cli/build_images/build_images.go +++ b/pkg/odo/cli/build_images/build_images.go @@ -68,7 +68,7 @@ func (o *BuildImagesOptions) Run(ctx context.Context) (err error) { devfilePath = odocontext.GetDevfilePath(ctx) path = filepath.Dir(devfilePath) ) - return image.BuildPushImages(o.clientset.FS, *devfileObj, path, o.pushFlag) + return image.BuildPushImages(ctx, o.clientset.FS, *devfileObj, path, o.pushFlag) } // NewCmdBuildImages implements the odo command diff --git a/pkg/odo/cli/deploy/deploy.go b/pkg/odo/cli/deploy/deploy.go index 3571c1167d2..fd6d839307e 100644 --- a/pkg/odo/cli/deploy/deploy.go +++ b/pkg/odo/cli/deploy/deploy.go @@ -85,7 +85,7 @@ func (o *DeployOptions) Run(ctx context.Context) error { "odo version: "+version.VERSION) // Run actual deploy command to be used - err := o.clientset.DeployClient.Deploy(o.clientset.FS, *devfileObj, path, appName, devfileName) + err := o.clientset.DeployClient.Deploy(ctx, o.clientset.FS, *devfileObj, path, appName, devfileName) if err == nil { log.Info("\nYour Devfile has been successfully deployed") diff --git a/pkg/watch/watch.go b/pkg/watch/watch.go index bf54bffff0d..fba0f1f64c4 100644 --- a/pkg/watch/watch.go +++ b/pkg/watch/watch.go @@ -84,7 +84,7 @@ type WatchParameters struct { // Custom function that can be used to push detected changes to remote pod. For more info about what each of the parameters to this function, please refer, pkg/component/component.go#PushLocal // WatchHandler func(kclient.ClientInterface, string, string, string, io.Writer, []string, []string, bool, []string, bool) error // Custom function that can be used to push detected changes to remote devfile pod. For more info about what each of the parameters to this function, please refer, pkg/devfile/adapters/interface.go#PlatformAdapter - DevfileWatchHandler func(adapters.PushParameters, WatchParameters, *ComponentStatus) error + DevfileWatchHandler func(context.Context, adapters.PushParameters, WatchParameters, *ComponentStatus) error // Parameter whether or not to show build logs Show bool // EnvSpecificInfo contains information about devfile @@ -118,7 +118,7 @@ type evaluateChangesFunc func(events []fsnotify.Event, path string, fileIgnores // processEventsFunc processes the events received on the watcher. It uses the WatchParameters to trigger watch handler and writes to out // It returns a Duration after which to recall in case of error -type processEventsFunc func(changedFiles, deletedPaths []string, parameters WatchParameters, out io.Writer, componentStatus *ComponentStatus, backoff *ExpBackoff) (*time.Duration, error) +type processEventsFunc func(ctx context.Context, changedFiles, deletedPaths []string, parameters WatchParameters, out io.Writer, componentStatus *ComponentStatus, backoff *ExpBackoff) (*time.Duration, error) func (o *WatchClient) WatchAndPush(out io.Writer, parameters WatchParameters, ctx context.Context, componentStatus ComponentStatus) error { klog.V(4).Infof("starting WatchAndPush, path: %s, component: %s, ignores %s", parameters.Path, parameters.ComponentName, parameters.FileIgnores) @@ -244,7 +244,7 @@ func (o *WatchClient) eventWatcher( componentStatus.State = StateSyncOutdated fmt.Fprintf(out, "Pushing files...\n\n") - retry, err := processEventsHandler(changedFiles, deletedPaths, parameters, out, &componentStatus, expBackoff) + retry, err := processEventsHandler(ctx, changedFiles, deletedPaths, parameters, out, &componentStatus, expBackoff) o.forceSync = false if err != nil { return err @@ -282,7 +282,7 @@ func (o *WatchClient) eventWatcher( } case <-deployTimer.C: - retry, err := processEventsHandler(nil, nil, parameters, out, &componentStatus, expBackoff) + retry, err := processEventsHandler(ctx, nil, nil, parameters, out, &componentStatus, expBackoff) if err != nil { return err } @@ -298,7 +298,7 @@ func (o *WatchClient) eventWatcher( case <-devfileTimer.C: fmt.Fprintf(out, "Updating Component...\n\n") - retry, err := processEventsHandler(nil, nil, parameters, out, &componentStatus, expBackoff) + retry, err := processEventsHandler(ctx, nil, nil, parameters, out, &componentStatus, expBackoff) if err != nil { return err } @@ -310,7 +310,7 @@ func (o *WatchClient) eventWatcher( } case <-retryTimer.C: - retry, err := processEventsHandler(nil, nil, parameters, out, &componentStatus, expBackoff) + retry, err := processEventsHandler(ctx, nil, nil, parameters, out, &componentStatus, expBackoff) if err != nil { return err } @@ -427,6 +427,7 @@ func evaluateFileChanges(events []fsnotify.Event, path string, fileIgnores []str } func (o *WatchClient) processEvents( + ctx context.Context, changedFiles, deletedPaths []string, parameters WatchParameters, out io.Writer, @@ -456,7 +457,7 @@ func (o *WatchClient) processEvents( ErrOut: parameters.ErrOut, } oldStatus := *componentStatus - err := parameters.DevfileWatchHandler(pushParams, parameters, componentStatus) + err := parameters.DevfileWatchHandler(ctx, pushParams, parameters, componentStatus) if err != nil { if isFatal(err) { return nil, err diff --git a/pkg/watch/watch_test.go b/pkg/watch/watch_test.go index 44ba34a2ec3..33247f6f4d2 100644 --- a/pkg/watch/watch_test.go +++ b/pkg/watch/watch_test.go @@ -40,7 +40,7 @@ func evaluateChangesHandler(events []fsnotify.Event, path string, fileIgnores [] return changedFiles, deletedPaths } -func processEventsHandler(changedFiles, deletedPaths []string, _ WatchParameters, out io.Writer, componentStatus *ComponentStatus, backo *ExpBackoff) (*time.Duration, error) { +func processEventsHandler(ctx context.Context, changedFiles, deletedPaths []string, _ WatchParameters, out io.Writer, componentStatus *ComponentStatus, backo *ExpBackoff) (*time.Duration, error) { fmt.Fprintf(out, "changedFiles %s deletedPaths %s\n", changedFiles, deletedPaths) return nil, nil }