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

Stack vs sample proposal and outer loop #474

Closed
wants to merge 0 commits into from
Closed

Conversation

elsony
Copy link
Contributor

@elsony elsony commented May 31, 2021

Signed-off-by: eyuen eyuen@redhat.com

What does this PR do?

Move design doc from gdoc (https://docs.google.com/document/d/1tbzZK9aWtpw5AtoZmUl1Jy8x8w9N5hhjXu3c26VYiYY/edit?usp=sharing and https://docs.google.com/document/d/1SL3ZJyAy8lp973EzQpY36VjTJHoyGXl2zUXokPiXwK8/edit?usp=sharing) to design doc in repo for public access.

What issues does this PR fix or reference?

Design doc for #279 and #346

Is your PR tested? Consider putting some instruction how to test your changes

Design doc changes only

Docs PR

Design doc changes only

@elsony elsony requested a review from GeekArthur May 31, 2021 18:21
@elsony elsony changed the title Stack vs sample proposal Stack vs sample proposal and outer loop May 31, 2021

`buildContext`: Path of source directory to establish build context. Default to ${PROJECT_ROOT} (optional)

`location`: Dockerfile location which can be an URL or a path relative to buildContext
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this default to Dockerfile to replicate the semantics of docker build ${buildContext}?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When running docker build command, it either uses the current working directory or the build context specified when running the command. When tools are trying to execute the dockerfile, we need to define the default directory that the tools can run the docker build command. With the described behaviour, it means the tools can find the Dockerfile in the project root by default which is the most common setup of the project root.


__Note:__
1. A common pattern will be using a global variable to define the image name (`myimage` in the example above) so that it can be easily referred to in the deploy step later.
2. 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which apply command is this referring to?

1. A common pattern will be using a global variable to define the image name (`myimage` in the example above) so that it can be easily referred to in the deploy step later.
2. 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.

### Example using a file Dockerfile with registry and secret for the image push:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Example using a file Dockerfile with registry and secret for the image push:
### Example using a Dockerfile with registry and secret for the image push:

Comment on lines 43 to 45
- name: mydockerfileimage
image:
imageName: {{myimage}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- name: mydockerfileimage
image:
imageName: {{myimage}}
- name: mydockerfileimage
image:
imageName: {{myimage}}

Comment on lines 111 to 116
- name: mydockerfileimage
image:
imageName: {{myimage}}
dockerfile:
git:
remotes:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- name: mydockerfileimage
image:
imageName: {{myimage}}
dockerfile:
git:
remotes:
- name: mydockerfileimage
image:
imageName: {{myimage}}
dockerfile:
git:
remotes:

location: Dockerfile
args: [ ]

The git definition will be the same as the one in `starterProjects` definition that supports `checkoutFrom`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just reuse projects/starterProjects here?

Copy link
Contributor Author

@elsony elsony Jun 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are just sharing the git definition with starterProjects/projects. This section is not related to starter project so it does not apply.


The git definition will be the same as the one in `starterProjects` definition that supports `checkoutFrom`

`location`: the location of the dockerfile within the repo. If `checkoutFrom` is being used, the location will be relative to the `checkoutFrom`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unclear to me -- checkoutFrom refers to a remote, so what does "relative" mean in this context?

labels:
app: {{.COMPONENT_NAME}}
spec:
containers:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you fix the indentation errors throughout -- I don't know if it's a misplaced tab character or something.


## Overview

The existing devfile registry focuses on the stack support to provide generic language/framework/runtime, e.g. Node, Java maven, Quarkus, etc., support to build and run user applications. These devfiles are called ___*stack devfiles*___. There is another kind of devfile in a devfile registry that is tailored for a building and running a specific application. These devfiles are referred to as ___*sample devfiles*___ (example: https://github.com/redhat-developer/devfile-sample).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think e.g. ***sample devfiles*** looks more markdown-y and renders the same as ___*sample devfiles*___

Comment on lines 192 to 195




Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trim trailing blank lines

Suggested change


Alternatively, if we assume the devfile contained in the sample folder may refer to the original stack instead of the specific example, we may need to include a way for the user to specify the metadata associated with the sample as part of the sample definition.

schemaVersion: 1.0.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better we use our current extraDevfileEntries.yaml in the registry resource repo as example:

schemaVersion: 1.0.0
samples:
  - name: nodejs-basic
    displayName: Basic NodeJS
    description: A simple Hello World Node.js application
    icon: https://github.com/maysunfaisal/node-bulletin-board-2/blob/main/nodejs-icon.png
    tags: ["NodeJS", "Express"]
    projectType: nodejs
    language: nodejs
    git:
      remotes:
        origin: https://github.com/redhat-developer/devfile-sample.git
  - name: code-with-quarkus
    displayName: Basic Quarkus
    description: A simple Hello World Java application using Quarkus
    icon: .devfile/icon/quarkus.png
    tags: ["Java", "Quarkus"]
    projectType: quarkus
    language: java
    git:
      remotes:
        origin: https://github.com/elsony/devfile-sample-code-with-quarkus.git
  - name: java-springboot-basic
    displayName: Basic Spring Boot
    description: A simple Hello World Java Spring Boot application using Maven
    icon: .devfile/icon/spring-logo.png
    tags: ["Java", "Spring"]
    projectType: springboot
    language: java
    git:
      remotes:
        origin: https://github.com/elsony/devfile-sample-java-springboot-basic.git
  - name: python-basic
    displayName: Basic Python
    description: A simple Hello World application using Python
    icon: .devfile/icon/python.png
    tags: ["Python"]
    projectType: python
    language: python
    git:
      remotes:
        origin: https://github.com/elsony/devfile-sample-python-basic.git

}
]

The bold entries highlighted the differences between a stack entry vs a sample entry in the devfile index registry.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bold entries are not displayed after moving docs from google docs, we need manually added them again by using markdown.

@openshift-ci
Copy link

openshift-ci bot commented Jun 3, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: elsony

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

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

Successfully merging this pull request may close these issues.

3 participants