Skip to content
This repository was archived by the owner on Feb 24, 2020. It is now read-only.

Conversation

fabiokung
Copy link
Contributor

When it is nil, the Volume info at the Pod level (with the same name)
should be used. Without this patch rkt list --format=json panics on a
nil pointer when Apps reference Volumes from the Pod level.

Signed-off-by: Fabio Kung fabio.kung@gmail.com

@ghost
Copy link

ghost commented Jun 7, 2017

Can one of the admins verify this patch?

@euank
Copy link
Member

euank commented Jun 8, 2017

ok to test

Copy link
Member

@lucab lucab left a comment

Choose a reason for hiding this comment

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

Thanks, bugfix looks fine, just two minor style comments.

lib/app.go Outdated
var (
podVols = podManifest.Volumes
podVolsByName = make(map[types.ACName]types.Volume, len(podVols))
)
Copy link
Member

Choose a reason for hiding this comment

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

I would avoid this inline var( ... ) block and just go for two plain := assignments.

lib/app.go Outdated
for _, mnt := range ra.Mounts {
var (
hostPath string
readOnly bool // false by default, since it is optional
Copy link
Member

Choose a reason for hiding this comment

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

Same here, let's just make it explicit as readOnly := false.

When it is nil, the Volume info at the Pod level (with the same name)
should be used. Without this patch `rkt list --format=json` panics on a
nil pointer when Apps reference Volumes from the Pod level.

Signed-off-by: Fabio Kung <fabio.kung@gmail.com>
@fabiokung
Copy link
Contributor Author

@lucab I made the changes you suggested. PTAL

Copy link
Member

@euank euank left a comment

Choose a reason for hiding this comment

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

LGTM.

A regression test, either here or as a followup, would be nice as well.

Copy link
Member

@lucab lucab left a comment

Choose a reason for hiding this comment

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

LGTM

@lucab lucab merged commit 5f42e6b into rkt:master Jun 12, 2017
@fabiokung fabiokung deleted the panic-json branch June 14, 2017 23:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants