[GitHub] [camel-k] nicolaferraro commented on a change in pull request #1827: Add command line support for creating, building and running containerized integrations locally

[GitBox <gi...@apache.org>]

nicolaferraro commented on a change in pull request #1827:
URL: https://github.com/apache/camel-k/pull/1827#discussion_r531539386



##########
File path: pkg/cmd/local_run.go
##########
@@ -55,31 +55,60 @@ func newCmdLocalRun(rootCmdOptions *RootCmdOptions) (*cobra.Command, *localRunCm
 		},
 	}
 
+	cmd.Flags().Bool("containerize", false, "Run integration in a local container.")
+	cmd.Flags().String("image-name", "", "Integration image name.")
+	cmd.Flags().String("docker-registry", "", "Docker registry to store intermediate images.")

Review comment:
       Same comment as before.. in all places

##########
File path: pkg/util/docker/docker.go
##########
@@ -0,0 +1,141 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package docker
+
+import (
+	"os/exec"
+	"path"
+	"strings"
+
+	"github.com/apache/camel-k/pkg/util"
+)
+
+// CreateBaseImageDockerFile --
+func CreateBaseImageDockerFile() error {
+	dockerFile := []string{}
+
+	// Base image is a java-only image since the integration command is just a java command.
+	dockerFile = append(dockerFile, FROM("adoptopenjdk/openjdk11:alpine"))

Review comment:
       You should use the base image from `defaults.go`

##########
File path: pkg/util/util.go
##########
@@ -35,6 +36,23 @@ import (
 	yaml2 "gopkg.in/yaml.v2"
 )
 
+/// Directories and file names:
+
+// MavenWorkingDirectory --  Directory used by Maven for an invocation of the kamel local command. By default a temporary folder will be used.
+var MavenWorkingDirectory string = ""
+
+// DefaultDependenciesDirectoryName --
+const DefaultDependenciesDirectoryName = "dependencies"
+
+// DefaultPropertiesDirectoryName --
+const DefaultPropertiesDirectoryName = "properties"
+
+// DefaultRoutesDirectoryName --
+const DefaultRoutesDirectoryName = "routes"
+
+// DefaultWorkingDirectoryName --
+const DefaultWorkingDirectoryName = "workspace"

Review comment:
       Can we use the same structure found in the kubernetes image? There's a small structure under `/deployments` there, plus many of other things are present in `/etc/camel`.
   
   The reason why this is important is that many users would like to use this command to freeze a container image that is then used in test and production, so it would be nice if a base image built here can be used as base image in kube as well

##########
File path: pkg/cmd/local_create.go
##########
@@ -0,0 +1,168 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cmd
+
+import (
+	"fmt"
+
+	"github.com/pkg/errors"
+	"github.com/spf13/cobra"
+)
+
+func newCmdLocalCreate(rootCmdOptions *RootCmdOptions) (*cobra.Command, *localCreateCmdOptions) {
+	options := localCreateCmdOptions{
+		RootCmdOptions: rootCmdOptions,
+	}
+
+	cmd := cobra.Command{
+		Use:     "create [options]",
+		Short:   "Create integration images locally.",
+		Long:    `Create integration images locally for containerized integrations.`,
+		PreRunE: decode(&options),
+		RunE: func(_ *cobra.Command, args []string) error {
+			if err := options.validate(args); err != nil {
+				return err
+			}
+			if err := options.init(args); err != nil {
+				return err
+			}
+			if err := options.run(args); err != nil {
+				fmt.Println(err.Error())
+			}
+			if err := options.deinit(args); err != nil {
+				return err
+			}
+
+			return nil
+		},
+		Annotations: map[string]string{
+			offlineCommandLabel: "true",
+		},
+	}
+
+	cmd.Flags().Bool("base-image", false, "Create base image used as a starting point for any integration.")

Review comment:
       I like the idea of building a base-image for an integration (without embedding the sources) or an image with source embedded.
   
   However, I've tested this with an integration (base-image=false) and sources are not included. If this is the idea, I think there's something still not working correctly.

##########
File path: pkg/cmd/local_create.go
##########
@@ -0,0 +1,168 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cmd
+
+import (
+	"fmt"
+
+	"github.com/pkg/errors"
+	"github.com/spf13/cobra"
+)
+
+func newCmdLocalCreate(rootCmdOptions *RootCmdOptions) (*cobra.Command, *localCreateCmdOptions) {
+	options := localCreateCmdOptions{
+		RootCmdOptions: rootCmdOptions,
+	}
+
+	cmd := cobra.Command{
+		Use:     "create [options]",
+		Short:   "Create integration images locally.",
+		Long:    `Create integration images locally for containerized integrations.`,
+		PreRunE: decode(&options),
+		RunE: func(_ *cobra.Command, args []string) error {
+			if err := options.validate(args); err != nil {
+				return err
+			}
+			if err := options.init(args); err != nil {
+				return err
+			}
+			if err := options.run(args); err != nil {
+				fmt.Println(err.Error())
+			}
+			if err := options.deinit(args); err != nil {
+				return err
+			}
+
+			return nil
+		},
+		Annotations: map[string]string{
+			offlineCommandLabel: "true",
+		},
+	}
+
+	cmd.Flags().Bool("base-image", false, "Create base image used as a starting point for any integration.")
+	cmd.Flags().String("image-name", "", "Integration image name.")
+	cmd.Flags().String("docker-registry", "", "Docker registry to store intermediate images.")

Review comment:
       I'd prefer using the word "container" instead of Docker here.
   
   Also, I wonder if it would be better to have a single parameter for the image name and registry. Simplifying the command as: `kamel local create it.yaml --image docker.io/myrepo/myimage`, i.e. treat is as a tag.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org