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

Docker compose to devfile #177

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

Conversation

Mahajanet
Copy link

What does this PR do?:

This PR provides library support for a project with a docker-compose file in it while supporting existing containers, single service & Multi-service with no dependencies.
We are using Kompose and it's framework instead of using it as a CLI.
For the issue in reference, the testing for converting docker-compose references for inline, URI, and both absolute and relative URL cases is completed. The next steps would include:

  1. Using the apply and deploy on the kubernetes components created
  2. Cleaning up the temoprary folder created for the intermediate kubernetes representation
  3. Unit Testing.

Which issue(s) this PR fixes:

This PR fixes the issue 1122, which is under the Devfile to Docker Compose Epic 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 Aug 4, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Mahajanet
Once this PR has been reviewed and has the lgtm label, please assign maysunfaisal for approval. For more information see the Kubernetes Code Review Process.

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

@openshift-merge-robot
Copy link

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link
Collaborator

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

Unclear from an initial reading of this PR, but is there a way to use docker compose support without relying on a filesystem? E.g. a way to pass in a devfile and get k8s components back out of it?

Something like

func convertDockerComposeToKubernetes(composeComponents []devfile.Component) ([]Client.Object, error)

args = parser.ParserArgs{
Path: "devfile.yaml",
Path: "devfile.yaml",
ConvertDockerComposeToKubernetes: &trueFlag, //setting flag to true for parser
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could use the k8s.io/utils/pointer or k8s.io/utils/ptr package for this, e.g.

Suggested change
ConvertDockerComposeToKubernetes: &trueFlag, //setting flag to true for parser
ConvertDockerComposeToKubernetes: pointer.Bool(true), //setting flag to true for parser

const (
DEPLOYMENT KubernetesController = "deployment"
DAEMONSET KubernetesController = "daemonSet"
REPLICATION_CONTROLLER KubernetesController = "replicationController"
Copy link
Collaborator

Choose a reason for hiding this comment

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

In what cases is a replicationController to be used? My understanding is that a replication controller is an older way to have the functionality provided by deployments (+ replicaSets).

return err
}
//iterate through each docker compose component
for _, dockercomposeComp := range dockercomposeComponents {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider breaking this up into multiple functions (e.g. a function for handling URIs), I'm finding it hard to parse with all the conditionals.

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.

3 participants