stage1: app add, status didn't work with empty vols #3451
Conversation
|
@euank do you mind a pass on this PR and a check if there are some leftover issues at runtime? |
|
semaphore failure is a flake |
| for _, m := range mounts { | ||
| AppAddOneMount(p, ra, m.Volume.Source, m.Mount.Path, m.ReadOnly, enterCmd) | ||
| shPath := filepath.Join(common.SharedVolumesPath(p.Root), m.Volume.Name.String()) | ||
| mntPath, err := EvaluateSymlinksInsideApp(appRootfs, m.Mount.Path) |
There was a problem hiding this comment.
Add a TODO similar to https://github.com/coreos/rkt/blob/a252c4c3078f746eca2739f3560428aa59b89da2/stage1/init/common/pod.go#L501-L504 if that is relevant here as well.
I'm not sure it is given this isn't so closely related to nspawn, but if not I'd appreciate a comment explaining why this is needed
There was a problem hiding this comment.
While that comment is technically correct, we still need the real mount path ourselves - so we can't get rid of it.
| log.FatalE("could not prepare mountpoint", err) | ||
| } | ||
| source := m.Volume.Source | ||
| if m.Volume.Kind == "empty" { |
There was a problem hiding this comment.
might as well use a switch statement like in https://github.com/coreos/rkt/blob/a252c4c3078f746eca2739f3560428aa59b89da2/stage1/init/common/pod.go#L522, right?
There was a problem hiding this comment.
Indeed. I don't know why this isn't correctly in GenerateMounts anyways. Bah.
squeed
force-pushed
the
sandbox-empty-vols
branch
from
d175e17
to
20afd7f
Compare
|
corrected, @euank PTAL! |
squeed
force-pushed
the
sandbox-empty-vols
branch
from
20afd7f
to
632b923
Compare
|
LGTM |
Fixes #3398