-
Notifications
You must be signed in to change notification settings - Fork 771
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
Merge top level non-service keys #1187
Conversation
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
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. I understand the commands that are listed here. |
Welcome @ChanderG! |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
docker-compose default behaviour is to merge these top level keys such as `networks`
Recheck CLA. |
Thanks for your PR @ChanderG . A few tests if still needed, I can give you some examples; file: # Tests merge multiple docker-compose files
cmd="kompose convert -f $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/base.yml -f $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/dev.yml --stdout -j"
sed -e "s;%VERSION%;$version;g" -e "s;%CMD%;$cmd;g" $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/output-base-template.json > /tmp/output-k8s.json
convert::expect_success "$cmd" "/tmp/output-k8s.json"
cmd="kompose convert -f $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/compose-port-base.yml -f $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/compose-port-prod.yml --stdout -j"
sed -e "s;%VERSION%;$version;g" -e "s;%CMD%;$cmd;g" $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/output-compose-port-template.json > /tmp/output-k8s.json
convert::expect_success "$cmd" "/tmp/output-k8s.json"
cmd="kompose convert -f $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/compose-port-base.yml -f $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/compose-new-service-prob.yml --stdout -j"
sed -e "s;%VERSION%;$version;g" -e "s;%CMD%;$cmd;g" $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/output-compose-new-service-template.json > /tmp/output-k8s.json
convert::expect_success "$cmd" "/tmp/output-k8s.json"
cmd="kompose --provider=openshift convert -f $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/compose-port-base.yml -f $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/compose-new-service-prob.yml --stdout -j"
sed -e "s;%VERSION%;$version;g" -e "s;%CMD%;$cmd;g" $KOMPOSE_ROOT/script/test/fixtures/merge-multiple-compose/output-openshift-template.json > /tmp/output-os.json
convert::expect_success "$cmd" "/tmp/output-os.json"
|
adds 2 tests: 1. merge - when 2 files have un-related top level keys, they are merged 2. override - when 2 files have the same keys in the other top level keys such as volumes and configs, the second file overrides the first
@hangyan Have added tests covering both the merge and override scenarios. Let me know if I should change something. |
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.
These all looks great @ChanderG . Thanks for your contribution!
Tests are not added yet.
Fix for #1185.