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

Fixed tmpfs with mode failure #816

Merged
merged 1 commit into from
Sep 26, 2017
Merged

Conversation

surajnarwade
Copy link
Contributor

Fixes #807
now kompose will ignore mode of tmpfs and will pass only mount path.

@kompose-bot
Copy link
Collaborator

@surajnarwade, thank you for the pull request! We'll request some people to review your PR. @procrypt and @cdrage, please review this.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 14, 2017
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 14, 2017
@@ -351,7 +352,8 @@ func (k *Kubernetes) ConfigTmpfs(name string, service kobject.ServiceConfig) ([]
for index, volume := range service.TmpFs {
//naming volumes if multiple tmpfs are provided
volumeName := fmt.Sprintf("%s-tmpfs%d", name, index)

v := strings.Split(volume, ":")
volume = v[0]
Copy link
Member

Choose a reason for hiding this comment

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

this could be just volume = strings.Split(volume, ":") remove the other line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it will not work,

# github.com/kubernetes/kompose/pkg/transformer/kubernetes
pkg/transformer/kubernetes/kubernetes.go:355: cannot use strings.Split(volume, ":") (type []string) as type string in assignment

Copy link
Member

Choose a reason for hiding this comment

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

@surajnarwade Pretty easy to figure it out, I forgot to add the [0].
Do: strings.Split(volume, ":")[0]

@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 18, 2017
@surajnarwade
Copy link
Contributor Author

issue is being tracked here #807

@cdrage
Copy link
Member

cdrage commented Sep 20, 2017

@surajnarwade can you add tests?

After that, LGTM.

Fixes kubernetes#807
now kompose will ignore mode of tmpfs and will pass only mount path.
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 25, 2017
@surajnarwade
Copy link
Contributor Author

added functional test

@cdrage
Copy link
Member

cdrage commented Sep 26, 2017

LGTM! Thanks for adding the tests.

@cdrage cdrage merged commit 35166b8 into kubernetes:master Sep 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants