Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inner loop dockerfile support #346

Closed
l0rd opened this issue Feb 15, 2021 · 19 comments
Closed

Inner loop dockerfile support #346

l0rd opened this issue Feb 15, 2021 · 19 comments
Milestone

Comments

@l0rd
Copy link
Contributor

l0rd commented Feb 15, 2021

Is your feature request related to a problem? Please describe.

Currently it's possible to specify a container used in the inner loop (from within an IDE for example) referring it's image.

That requires that the image has been built and published somewhere. But projects often include a Dockerfile with build and runtime tools. The development flow in this case is usually:

  1. build the Dockerfile to locally create an image with build and runtime tools
  2. start the corresponding container, mounting the source code from a local folder
  3. run the project build from within the container
  4. run the project application from within the container

There may be some variants to this flow, including:

  • 2 distinct images are used for build and run
  • Build of the application is done as part of the container build
  • Use of multi-stage build

Even if the flow may be different, for all those variants building and running an image from a Dockerfile (without publishing the image) is part of the inner loop.

Describe the solution you'd like

Specify a container component using a reference to an image component that exist in the same Devfile or in the parent:

components:
  - name: "myapp-container"
    container:
      imageReference: "myapp-image"
      mountSources: true
      command: "tail -f /dev/null"
  - name: "myapp-image"
    image:
      dockerfile:
        location: Dockerfile
@l0rd l0rd changed the title Inner loop Dockerfile component support Inner loop dockerfile support Feb 15, 2021
@resios
Copy link

resios commented Sep 15, 2021

I think we should update the experience based on #51 Namely, instead of introducing a dockerfile field under the container component, we should reference an image component.

components:
  - name: "myapp-container"
    container:
      imageReference: "myapp-image"
      mountSources: true
      command: "tail -f /dev/null"
  - name: "myapp-image"
    image:
      dockerfile:
        location: Dockerfile

What do you think @l0rd @elsony ?

@l0rd
Copy link
Contributor Author

l0rd commented Sep 15, 2021

@resios I agree. I have updated the description with the new proposal.

@resios
Copy link

resios commented Sep 16, 2021

@l0rd Thank you. We'll pick this up and use #580 as a stepping stone. Are you ok with adding imageReference as a new field?

Other alternatives:

@l0rd
Copy link
Contributor Author

l0rd commented Sep 16, 2021

@resios I am ok with imageReference but can you present that during next Devfile Cabal to get a broader feedback?

@resios
Copy link

resios commented Sep 16, 2021

@l0rd Will do. I'll add it to the agenda.

@AlexKhotian
Copy link

AlexKhotian commented Oct 4, 2021

Is there someone working on this? Or I can go and try to add this? (For sure after the discussion during the next cabal meeting)

@l0rd
Copy link
Contributor Author

l0rd commented Oct 4, 2021

@AlexKhotian no, I don't think anyone is looking at it. Go ahead! cc @elsony

@scottkurz
Copy link
Contributor

To state my concern another way:

The outer loop proposal suggests there's one lifecycle for kubernetes-typed components referenced via apply commands, relative to image type components, and a separate lifeycle for other kubernetes-typed components.

Now this proposal implies there's going to be a different lifecycle for container-typed components relative to image-typed components.

My concern is that this multi-component lifecycle isn't spelled out in detail and one place, making it hard to feel comfortable with enhancements like this.

@kadel
Copy link
Member

kadel commented Oct 6, 2021

The outer loop proposal suggests there's one lifecycle for kubernetes-typed components referenced via apply commands, relative to image type components, and a separate lifeycle for other kubernetes-typed components.

@scottkurz are you referring this?

"The apply command with the image component is optional. If the apply command that references an image component does not exist, then the image build will be done at the startup automatically (similar to the behaviour of the existing kubernetes components). If there is an apply command that references the image component, then the user will need to create a composite command to chain the image build apply command with the deploy command (see the deployment command section) to complete the build and deploy."

I agree that this is starting to get necessary complicated.

  • kubernetes component: created on startup if it is not referenced by apply command
  • image component: created (build) on startup if it is not referenced by apply command
  • container component: always created on startup? (<- I'm actually not sure about this one)

We should not try to complicate this even more.

I actually think that with introduction of image components i the outerloop proposal and variables we already have everything that is required for "innerloop dockerfile support"

variables:
  myimage: myregistry:5000/inerloop-image:latest
components:
  - name: innerloop-image
    image:
      imageName: {{myimage}}
      dockerfile:
        location: Dockerfile
  - name: runtime
    container:
      image: {{myimage}}
      mountSources: true

The above should theoretical already work (once tools have support for image components) , we just need to make sure that the image components are build before other components are executed.

@AlexKhotian
Copy link

AlexKhotian commented Oct 6, 2021

@kadel So you suggest to build innerloop-image and run it as a container? I assume that the original proposal was about references between container and image components. Is my understanding correct?

components:
  - name: innerloop-image
    image:
      imageName: test-image
      dockerfile:
        location: Dockerfile
  - name: runtime
    container:
      image: test-image
      mountSources: true

@AlexKhotian
Copy link

AlexKhotian commented Oct 6, 2021

@kadel @elsony @resios Would be great if we can conclude on the logic.
Personally I think that proposed solution in this ticket (without new field imageReference) makes sense.

There is an image that will be build and there is a component that will use a built image.

components:
  - name: innerloop-image
    image:
      imageName: test-image
      dockerfile:
        uri: Dockerfile
  - name: runtime
    container:
      image: test-image
      mountSources: true

Note: seems like there is no location property for the dockerfile but there has been introduced uri property instead

@kadel
Copy link
Member

kadel commented Oct 7, 2021

@kadel So you suggest to build innerloop-image and run it as a container?

yes

I assume that the original proposal was about references between container and image components. Is my understanding correct?

Yes, but I don't think that given the rules for components we don't need any extra references. Also, the use of recently added variables helps reduce hardcoded value duplication.

@AlexKhotian
Copy link

I see.
What about a usecase when I want to build 1 image and run it with different args. Example can be - I have an image with my monolith-service code that I want to start in different modes:

components:
  - name: service-image
    image:
      imageName: service:1.1
      dockerfile:
        uri: Dockerfile
  - name: service-dev # no command - I just want to write code for my new service flavor here
      container:
      image: service:1.1
      mountSources: true
  - name: service-queue
    container:
      image: service:1.1
      command: "./start-queue"
      mountSources: true
 - name: service-publisher
    container:
      image: service:1.1
      command: "./start-queue"
      mountSources: true

Referencing seems to make it clear - what is a build time and what is a runtime. Wdyt?

Note: I think this type of referencing already works with the changes from the outer-loop support.

@AlexKhotian
Copy link

cc @elsony @resios @kadel
What do you think about the proposal above? I think that we have all mechanism. We have to align on the semantics.

@kadel
Copy link
Member

kadel commented Oct 26, 2021

What do you think about the proposal above? I think that we have all mechanism. We have to align on the semantics.

To me this makes sense.

Referencing seems to make it clear - what is a build time and what is a runtime. Wdyt?

If I'm not mistaken we already have rules around what component should be automatically created on startup. If component is not reference by any apply command then it will be started automatically on startup.
We will just need to extend this rule a bit, and define that the image components will be executed before any container component

@yangcao77
Copy link
Contributor

yangcao77 commented Jan 27, 2022

We will just need to extend this rule a bit, and define that the image components will be executed before any container component

Correct, we have introduced AutoBuild for image component, which defines if the image should be build during startup:

// Defines if the image should be built during startup.
//
// Default value is `false`
// +optional
// +devfile:default:value=false
AutoBuild *bool `json:"autoBuild,omitempty"`

Tools can choose always to build images(with AutoBuild = true) before containers during startup, so containers can reference images for innerloop scenarios.

If I understands correctly, the innerloop scenarios requirements have been fulfilled by the design of current image component? Anything else left for this issue?

@l0rd
Copy link
Contributor Author

l0rd commented Jan 27, 2022

One question I have for @AlexKhotian and @resios is if they expect that in the devfile registry samples like java spring boot basic:

components:
  - name: outerloop-build
    image:
      imageName: java-springboot-image:latest
      dockerfile:
        uri: docker/Dockerfile
        buildContext: .
        rootRequired: false
  - name: outerloop-deploy
    kubernetes:
      uri: outerloop-deploy.yaml # ==> references java-springboot-image:latest

we add an innerloop-deploy component too:

components:
  (...)
  - name: innerloop-deploy
    container:
      image: java-springboot-image:latest # ==> reference to the image

that would mean that we would have also some extra commands for "inner-loop" build-image:

  - id: run-container
    apply:
      component: innerloop-deploy
  - id: deploy-container
    composite:
      commands:
        - build-image
        - deploy-container
      group:
        kind: deploy
        isDefault: true

I am asking to understand if the docker-plugin will be a client of the devfile registry samples or if it will have it's own samples/registry.

@elsony elsony added this to the 2.2 milestone Mar 28, 2022
@elsony
Copy link
Contributor

elsony commented Apr 8, 2022

The original requirement for devfile has already been implemented: #346 (comment).

@elsony
Copy link
Contributor

elsony commented Sep 15, 2022

Devfile community call discussion:
Closing this one since the work is done. Will also need to clean up the mentioning of innerloop and outerloop from the devfile samples.

@elsony elsony closed this as completed Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants