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.

Sign up for GitHub

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

Jump to bottom

added support for docker-compose.yaml besides docker-compose.yml #368

Merged
merged 1 commit into from
Jan 30, 2017
Merged

added support for docker-compose.yaml besides docker-compose.yml #368

merged 1 commit into from
Jan 30, 2017

Conversation

procrypt
Copy link
Contributor

@procrypt procrypt commented Jan 9, 2017 •
edited
Loading

Now kompose will look for docker-compose.yml as well as docker-compose.yaml in the current directory, when -f or --file flag in not given .

$ ls
docker-compose.yml
$ kompose convert
INFO[0000] file "mlbparks-service.yaml" created         
INFO[0000] file "mongodb-service.yaml" created          
INFO[0000] file "mlbparks-deployment.yaml" created      
INFO[0000] file "mongodb-deployment.yaml" created       
INFO[0000] file "mongodb-claim0-persistentvolumeclaim.yaml" created
$ ls
docker-compose.yaml
$ kompose convert
INFO[0000] file "mlbparks-service.yaml" created         
INFO[0000] file "mongodb-service.yaml" created          
INFO[0000] file "mlbparks-deployment.yaml" created      
INFO[0000] file "mongodb-deployment.yaml" created       
INFO[0000] file "mongodb-claim0-persistentvolumeclaim.yaml" created

cc. @containscafeine @surajssd @kadel
Fixes #352 and #369 includes functional tests
I think we can now close #367

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 9, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 46.597% when pulling 4bc88cb on procrypt:yaml_and_yml into 4b3094d on kubernetes-incubator:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 46.597% when pulling 4bc88cb on procrypt:yaml_and_yml into 4b3094d on kubernetes-incubator:master.

@surajssd surajssd mentioned this pull request Jan 9, 2017
Closed
@procrypt procrypt force-pushed the yaml_and_yml branch 3 times, most recently from bb22813 to 6829e6f Compare January 9, 2017 10:14
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 46.924% when pulling 6829e6f on procrypt:yaml_and_yml into 2ba8831 on kubernetes-incubator:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 46.924% when pulling 6829e6f on procrypt:yaml_and_yml into 2ba8831 on kubernetes-incubator:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 46.924% when pulling 6829e6f on procrypt:yaml_and_yml into 2ba8831 on kubernetes-incubator:master.

@kadel
Copy link
Member

kadel commented Jan 9, 2017

This will be broken by #367.

It would be better to decide between docker-compose.yml and docker-compose.yaml somewhere else, outside compose loader.

@surajssd
Copy link
Member

@kadel yes true!

@procrypt procrypt force-pushed the yaml_and_yml branch 2 times, most recently from 4f39c83 to 57638ec Compare January 10, 2017 08:52
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 46.897% when pulling 57638ec on procrypt:yaml_and_yml into 2ba8831 on kubernetes-incubator:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 47.09% when pulling 57638ec on procrypt:yaml_and_yml into 2ba8831 on kubernetes-incubator:master.

@procrypt procrypt force-pushed the yaml_and_yml branch 2 times, most recently from 6dacdfc to 0ba501a Compare January 10, 2017 09:17
@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 47.09% when pulling 0ba501a on procrypt:yaml_and_yml into 2ba8831 on kubernetes-incubator:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 47.09% when pulling 0ba501a on procrypt:yaml_and_yml into 2ba8831 on kubernetes-incubator:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 47.09% when pulling ce72313 on procrypt:yaml_and_yml into 2ba8831 on kubernetes-incubator:master.

@surajssd
Copy link
Member

surajssd commented Jan 10, 2017 •
edited
Loading

@kadel do you think it's the right place to handle default files?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 47.09% when pulling a13549b on procrypt:yaml_and_yml into 7dbf00c on kubernetes-incubator:master.

@concaf
Copy link
Contributor

concaf commented Jan 11, 2017

@procrypt this breaks kompose up and kompose down for me, can you check that out?

@kadel
Copy link
Member

kadel commented Jan 11, 2017 •
edited
Loading

verified it indeed breaks up and down this is because ValidateFlags is called only when doing convert.
It needs deeper investigation

@procrypt procrypt force-pushed the yaml_and_yml branch 2 times, most recently from eb45a7d to d6db74c Compare January 17, 2017 11:35
@procrypt procrypt force-pushed the yaml_and_yml branch 3 times, most recently from 53926d5 to 83efc4d Compare January 17, 2017 12:07
@procrypt
Copy link
Contributor Author

@kadel @containscafeine @surajssd Please review.

cdrage
cdrage requested changes Jan 17, 2017
View reviewed changes
pkg/app/app.go
@@ -133,6 +146,23 @@ func ValidateFlags(bundle string, args []string, cmd *cobra.Command, opt *kobjec
}
}

// ValidateUpDown validates flags for Up and Down
func ValidateUpDown(cmd *cobra.Command, opt *kobject.ConvertOptions) {
Copy link
Member

Choose a reason for hiding this comment

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

I think ValidateUpDown could be worded differently.

How about ValidateComposeFile?

script/test/cmd/tests.sh
convert::expect_success "kompose --provider=openshift convert --stdout -j" "$KOMPOSE_ROOT/script/test/fixtures/yaml-and-yml/yml/output-os.json"
# Return back to the original path
cd $CURRENT_DIR

Copy link
Member

@cdrage cdrage Jan 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

👍

@procrypt
Copy link
Contributor Author

@kadel @cdrage Please review.

@cdrage
Copy link
Member

cdrage commented Jan 24, 2017

@procrypt LGTM!

@procrypt
Copy link
Contributor Author

Example

@cdrage
Copy link
Member

cdrage commented Jan 30, 2017

All green 👍 Merging away!

@cdrage cdrage merged commit 8bf0640 into kubernetes:master Jan 30, 2017
@surajssd surajssd mentioned this pull request Feb 9, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdrage cdrage cdrage approved these changes

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add support for docker-compose.yaml besides docker-compose.yml
7 participants
@procrypt @coveralls @kadel @surajssd @concaf @cdrage @k8s-ci-robot