-
Notifications
You must be signed in to change notification settings - Fork 35
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
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Mahajanet 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 |
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. |
There was a problem hiding this 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 |
There was a problem hiding this comment.
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.
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" |
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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.
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:
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.
Unit/Functional tests
QE Integration test
Documentation
Client Impact
Gosec scans
How to test changes / Special notes to the reviewer: