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

Set PVC volume size with kompose.volume.size #867

Merged
merged 1 commit into from
Nov 15, 2017
Merged

Set PVC volume size with kompose.volume.size #867

merged 1 commit into from
Nov 15, 2017

Conversation

abitrolly
Copy link
Contributor

#235

Example:

version: '3'
services:
  cs-btcd:
    image: cybernode/bitcoin-btcd:temp
    labels:
      kompose.volume.size: 500Mi
    volumes:
      - /cyberdata:/cyberdata

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 8, 2017
@cdrage
Copy link
Member

cdrage commented Nov 8, 2017

Hey @abitrolly

The code LGTM. May be good to get another quick review by @surajnarwade or @kadel

Are you perhaps able to add any tests?

To be honest, creating unit tests is a bit of a mess right-now, as we're still waiting for #805 to be merged in.

If not, I don't mind creating a PR after your merge, as of right now, it's quite a manual process to create integration tests.

@cdrage
Copy link
Member

cdrage commented Nov 8, 2017

Also: please combine commits to 1 commit!

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 12, 2017
@abitrolly
Set service volume size with kompose.volume.size label (kubernetes#235)
1975c32 
1. Copy labels from compose to kobject
2. If kompose.volume.size is set on service level, use it

Internal API changes:

- Add PVCSize to kobject Volumes struct
- Pass default volume size as CreatePVC() param
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 12, 2017
@abitrolly
Copy link
Contributor Author

Squashed and rebased.

For tests, there is no info how to run them. =/
https://github.com/kubernetes/kompose

@cdrage
Copy link
Member

cdrage commented Nov 13, 2017

@abitrolly No worries on that, I'll push the PR tomorrow (it's a holiday here in Canada today) and see what tests I can write. I'll have to update #805 and add some documentation for future cases for adding tests.

Thanks for the contribution!

@abitrolly
Copy link
Contributor Author

abitrolly commented Nov 15, 2017 •
edited
Loading

Looks like Travis check was not reported to GitHub correctly.

I am right that tests are now shell-based?

@cdrage
Copy link
Member

cdrage commented Nov 15, 2017 •
edited
Loading

Yup. that is correct! Pushing the PR now.

EDIT: Travis looked to have reported it incorrectly, but all green from here.

@cdrage cdrage merged commit d7c515b into kubernetes:master Nov 15, 2017
@cdrage cdrage mentioned this pull request Nov 15, 2017
Closed
2 tasks
@hangyan hangyan mentioned this pull request Mar 31, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@abitrolly @cdrage @k8s-ci-robot