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

[wip] adding compose component to devfile/api #882

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

kernelpanic77
Copy link

@kernelpanic77 kernelpanic77 commented Jul 6, 2022

What does this PR do?:

Changes for including Compose Component to reference a compose yaml file. Generated devworkspace CRDs and schemas for corresponding changes in v1alpha2 api.

I have create an example devfile in samples/devfiles/docker-compose-devfile.yaml

schemaVersion: 2.1.0
metadata:
  name: microservices-demo
attributes:
  controller.devfile.io/storage-type: ephemeral
  controller.devfile.io/scc: container-build
components:
  - name: frontend-dev
    docker-compose:
      uri: ./utils/docker-compose.yml
commands:
  - id: init-project
    exec:
      component: frontend
      commandLine: yarn install
      workingDir: $PROJECT_SOURCE
events:
  postStart:
    - init-project

⚠️ Do not merge this PR yet.

Which issue(s) this PR fixes:

#501

PR acceptance criteria:

Testing and documentation do not need to be complete in order for this PR to be approved. We just need to ensure tracking issues are opened.

  • Open new test/doc issues under the devfile/api repo
  • Check each criteria if:
  • There is a separate tracking issue. Add the issue link under the criteria
    or
  • test/doc updates are made as part of this PR
  • If unchecked, explain why it's not needed

How to test changes / Special notes to the reviewer:

@openshift-ci
Copy link

openshift-ci bot commented Jul 6, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kernelpanic77

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

pkg/apis/workspaces/v1alpha2/component_compose.go Outdated Show resolved Hide resolved
pkg/apis/workspaces/v1alpha2/components.go Outdated Show resolved Hide resolved
pkg/validation/components_test.go Outdated Show resolved Hide resolved
kernelpanic77 and others added 2 commits July 12, 2022 23:29
Co-authored-by: Angel Misevski <amisevsk@redhat.com>
Co-authored-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: kernelpanic77 <shanware.ishan@gmail.com>
Signed-off-by: kernelpanic77 <shanware.ishan@gmail.com>
Copy link
Contributor

@l0rd l0rd left a comment

Choose a reason for hiding this comment

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

I have provided some suggestions. Those should be applied to v1alpha1 too.

pkg/apis/workspaces/v1alpha2/component_compose.go Outdated Show resolved Hide resolved
pkg/apis/workspaces/v1alpha2/component_compose.go Outdated Show resolved Hide resolved
pkg/apis/workspaces/v1alpha2/component_compose.go Outdated Show resolved Hide resolved
pkg/apis/workspaces/v1alpha2/component_compose.go Outdated Show resolved Hide resolved
pkg/apis/workspaces/v1alpha2/component_compose.go Outdated Show resolved Hide resolved
Comment on lines 58 to 61
// Allows importing into the devworkspace docker-compose files
// defined in a given manifest. For example this allows the reuse of previously
// docker-compose files used to define configuration for managing
// multiple containers at the same time.
Copy link
Contributor

Choose a reason for hiding this comment

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

Trying to address the points raised (rightfully) by @amisevsk

Suggested change
// Allows importing into the devworkspace docker-compose files
// defined in a given manifest. For example this allows the reuse of previously
// docker-compose files used to define configuration for managing
// multiple containers at the same time.
// Allows referencing an existing Compose files so that its abstractions as
// Services and Volumes will be included in the resulting development
// environment.

@kadel
Copy link
Member

kadel commented Aug 11, 2022

Can we please pause and think about how this is actually going to work? Please see and continue discussion at #501 (comment)

kernelpanic77 and others added 6 commits October 27, 2022 16:33
Co-authored-by: Mario Loriedo <mario.loriedo@gmail.com>
Co-authored-by: Angel Misevski <amisevsk@redhat.com>
Co-authored-by: Mario Loriedo <mario.loriedo@gmail.com>
Co-authored-by: Mario Loriedo <mario.loriedo@gmail.com>
Co-authored-by: Mario Loriedo <mario.loriedo@gmail.com>
Co-authored-by: Mario Loriedo <mario.loriedo@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants