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:
us...@infra.apache.org


Reply via email to