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

code cleanup #3496

Merged
merged 1 commit into from
Jul 8, 2019
Merged

code cleanup #3496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ install.libseccomp.sudo:


cmd/podman/varlink/iopodman.go: cmd/podman/varlink/io.podman.varlink
$(GO) generate ./cmd/podman/varlink/...
GO111MODULE=off $(GO) generate ./cmd/podman/varlink/...

API.md: cmd/podman/varlink/io.podman.varlink
$(GO) generate ./docs/...
Expand Down
7 changes: 3 additions & 4 deletions libpod/boltdb_state_internal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,6 @@ func getRuntimeConfigBucket(tx *bolt.Tx) (*bolt.Bucket, error) {
}

func (s *BoltState) getContainerFromDB(id []byte, ctr *Container, ctrsBkt *bolt.Bucket) error {
valid := true
ctrBkt := ctrsBkt.Bucket(id)
if ctrBkt == nil {
return errors.Wrapf(define.ErrNoSuchCtr, "container %s not found in DB", string(id))
Expand Down Expand Up @@ -386,7 +385,7 @@ func (s *BoltState) getContainerFromDB(id []byte, ctr *Container, ctrsBkt *bolt.
}

ctr.runtime = s.runtime
ctr.valid = valid
ctr.valid = true

return nil
}
Expand Down Expand Up @@ -639,7 +638,7 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error {
}

// Add ctr to pod
if pod != nil {
if pod != nil && podCtrs != nil {
if err := podCtrs.Put(ctrID, ctrName); err != nil {
return errors.Wrapf(err, "error adding container %s to pod %s", ctr.ID(), pod.ID())
}
Expand Down Expand Up @@ -737,7 +736,7 @@ func (s *BoltState) removeContainer(ctr *Container, pod *Pod, tx *bolt.Tx) error
}
}

if podDB != nil {
if podDB != nil && pod != nil {
// Check if the container is in the pod, remove it if it is
podCtrs := podDB.Bucket(containersBkt)
if podCtrs == nil {
Expand Down
16 changes: 8 additions & 8 deletions libpod/common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,13 @@ func getTestContainer(id, name string, manager lock.Manager) (*Container, error)

ctr.config.Labels["test"] = "testing"

// Allocate a lock for the container
lock, err := manager.AllocateLock()
// Allocate a containerLock for the container
containerLock, err := manager.AllocateLock()
if err != nil {
return nil, err
}
ctr.lock = lock
ctr.config.LockID = lock.ID()
ctr.lock = containerLock
ctr.config.LockID = containerLock.ID()

return ctr, nil
}
Expand All @@ -114,13 +114,13 @@ func getTestPod(id, name string, manager lock.Manager) (*Pod, error) {
valid: true,
}

// Allocate a lock for the pod
lock, err := manager.AllocateLock()
// Allocate a podLock for the pod
podLock, err := manager.AllocateLock()
if err != nil {
return nil, err
}
pod.lock = lock
pod.config.LockID = lock.ID()
pod.lock = podLock
pod.config.LockID = podLock.ID()

return pod, nil
}
Expand Down
4 changes: 3 additions & 1 deletion libpod/container_attach_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,9 @@ func redirectResponseToOutputStreams(outputStream, errorStream io.Writer, writeO
default:
logrus.Infof("Received unexpected attach type %+d", buf[0])
}

if dst == nil {
return errors.New("output destination cannot be nil")
}
if doWrite {
nw, ew := dst.Write(buf[1:nr])
if ew != nil {
Expand Down
16 changes: 8 additions & 8 deletions libpod/container_inspect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,12 @@ func (c *Container) Inspect(size bool) (*InspectContainerData, error) {
func (c *Container) getContainerInspectData(size bool, driverData *driver.Data) (*InspectContainerData, error) {
config := c.config
runtimeInfo := c.state
spec, err := c.specFromState()
stateSpec, err := c.specFromState()
if err != nil {
return nil, err
}

// Process is allowed to be nil in the spec
// Process is allowed to be nil in the stateSpec
args := []string{}
if config.Spec.Process != nil {
args = config.Spec.Process.Args
Expand Down Expand Up @@ -244,7 +244,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *driver.Data)
}
}

mounts, err := c.getInspectMounts(spec)
mounts, err := c.getInspectMounts(stateSpec)
if err != nil {
return nil, err
}
Expand All @@ -255,7 +255,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *driver.Data)
Path: path,
Args: args,
State: &InspectContainerState{
OciVersion: spec.Version,
OciVersion: stateSpec.Version,
Status: runtimeInfo.State.String(),
Running: runtimeInfo.State == define.ContainerStateRunning,
Paused: runtimeInfo.State == define.ContainerStatePaused,
Expand Down Expand Up @@ -285,9 +285,9 @@ func (c *Container) getContainerInspectData(size bool, driverData *driver.Data)
Driver: driverData.Name,
MountLabel: config.MountLabel,
ProcessLabel: config.ProcessLabel,
EffectiveCaps: spec.Process.Capabilities.Effective,
BoundingCaps: spec.Process.Capabilities.Bounding,
AppArmorProfile: spec.Process.ApparmorProfile,
EffectiveCaps: stateSpec.Process.Capabilities.Effective,
BoundingCaps: stateSpec.Process.Capabilities.Bounding,
AppArmorProfile: stateSpec.Process.ApparmorProfile,
ExecIDs: execIDs,
GraphDriver: driverData,
Mounts: mounts,
Expand Down Expand Up @@ -338,7 +338,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *driver.Data)
// Get information on the container's network namespace (if present)
data = c.getContainerNetworkInfo(data)

inspectConfig, err := c.generateInspectContainerConfig(spec)
inspectConfig, err := c.generateInspectContainerConfig(stateSpec)
if err != nil {
return nil, err
}
Expand Down
28 changes: 14 additions & 14 deletions libpod/container_internal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ func (c *Container) removeConmonFiles() error {
if !os.IsNotExist(err) {
return errors.Wrapf(err, "error running stat on container %s exit file", c.ID())
}
} else if err == nil {
} else {
// Rename should replace the old exit file (if it exists)
if err := os.Rename(exitFile, oldExitFile); err != nil {
return errors.Wrapf(err, "error renaming container %s exit file", c.ID())
Expand All @@ -568,11 +568,11 @@ func (c *Container) removeConmonFiles() error {
func (c *Container) export(path string) error {
mountPoint := c.state.Mountpoint
if !c.state.Mounted {
mount, err := c.runtime.store.Mount(c.ID(), c.config.MountLabel)
containerMount, err := c.runtime.store.Mount(c.ID(), c.config.MountLabel)
if err != nil {
return errors.Wrapf(err, "error mounting container %q", c.ID())
}
mountPoint = mount
mountPoint = containerMount
defer func() {
if _, err := c.runtime.store.Unmount(c.ID(), false); err != nil {
logrus.Errorf("error unmounting container %q: %v", c.ID(), err)
Expand Down Expand Up @@ -856,18 +856,18 @@ func (c *Container) init(ctx context.Context, retainRetries bool) error {
span.SetTag("struct", "container")
defer span.Finish()

// Generate the OCI spec
spec, err := c.generateSpec(ctx)
// Generate the OCI newSpec
newSpec, err := c.generateSpec(ctx)
if err != nil {
return err
}

// Save the OCI spec to disk
if err := c.saveSpec(spec); err != nil {
// Save the OCI newSpec to disk
if err := c.saveSpec(newSpec); err != nil {
return err
}

// With the spec complete, do an OCI create
// With the newSpec complete, do an OCI create
if err := c.ociRuntime.createContainer(c, c.config.CgroupParent, nil); err != nil {
return err
}
Expand Down Expand Up @@ -1167,8 +1167,8 @@ func (c *Container) cleanupStorage() error {
return nil
}

for _, mount := range c.config.Mounts {
if err := c.unmountSHM(mount); err != nil {
for _, containerMount := range c.config.Mounts {
if err := c.unmountSHM(containerMount); err != nil {
return err
}
}
Expand Down Expand Up @@ -1399,14 +1399,14 @@ func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (exten
}
return nil, err
}
hooks, err := manager.Hooks(config, c.Spec().Annotations, len(c.config.UserVolumes) > 0)
ociHooks, err := manager.Hooks(config, c.Spec().Annotations, len(c.config.UserVolumes) > 0)
if err != nil {
return nil, err
}
if len(hooks) > 0 || config.Hooks != nil {
logrus.Warnf("implicit hook directories are deprecated; set --hooks-dir=%q explicitly to continue to load hooks from this directory", hDir)
if len(ociHooks) > 0 || config.Hooks != nil {
logrus.Warnf("implicit hook directories are deprecated; set --ociHooks-dir=%q explicitly to continue to load ociHooks from this directory", hDir)
}
for i, hook := range hooks {
for i, hook := range ociHooks {
allHooks[i] = hook
}
}
Expand Down
66 changes: 41 additions & 25 deletions libpod/container_internal_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,13 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
// If network namespace was requested, add it now
if c.config.CreateNetNS {
if c.config.PostConfigureNetNS {
g.AddOrReplaceLinuxNamespace(spec.NetworkNamespace, "")
if err := g.AddOrReplaceLinuxNamespace(spec.NetworkNamespace, ""); err != nil {
return nil, err
}
} else {
g.AddOrReplaceLinuxNamespace(spec.NetworkNamespace, c.state.NetNS.Path())
if err := g.AddOrReplaceLinuxNamespace(spec.NetworkNamespace, c.state.NetNS.Path()); err != nil {
return nil, err
}
}
}

Expand Down Expand Up @@ -415,7 +419,9 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {

if rootPropagation != "" {
logrus.Debugf("set root propagation to %q", rootPropagation)
g.SetLinuxRootPropagation(rootPropagation)
if err := g.SetLinuxRootPropagation(rootPropagation); err != nil {
return nil, err
}
}

// Warning: precreate hooks may alter g.Config in place.
Expand Down Expand Up @@ -561,7 +567,9 @@ func (c *Container) checkpointRestoreLabelLog(fileName string) (err error) {
if err != nil {
return errors.Wrapf(err, "failed to create CRIU log file %q", dumpLog)
}
logFile.Close()
if err := logFile.Close(); err != nil {
logrus.Errorf("unable to close log file: %q", err)
}
if err = label.SetFileLabel(dumpLog, c.MountLabel()); err != nil {
return errors.Wrapf(err, "failed to label CRIU log file %q", dumpLog)
}
Expand Down Expand Up @@ -620,9 +628,11 @@ func (c *Container) checkpoint(ctx context.Context, options ContainerCheckpointO
"config.dump",
"spec.dump",
}
for _, delete := range cleanup {
file := filepath.Join(c.bundlePath(), delete)
os.Remove(file)
for _, del := range cleanup {
file := filepath.Join(c.bundlePath(), del)
if err := os.Remove(file); err != nil {
logrus.Debugf("unable to remove file %s", file)
}
}
}

Expand Down Expand Up @@ -702,7 +712,9 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti
if err != nil {
return err
}
json.Unmarshal(networkJSON, &networkStatus)
if err := json.Unmarshal(networkJSON, &networkStatus); err != nil {
return err
}
// Take the first IP address
var IP net.IP
if len(networkStatus) > 0 {
Expand Down Expand Up @@ -744,7 +756,9 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti

// We want to have the same network namespace as before.
if c.config.CreateNetNS {
g.AddOrReplaceLinuxNamespace(spec.NetworkNamespace, c.state.NetNS.Path())
if err := g.AddOrReplaceLinuxNamespace(spec.NetworkNamespace, c.state.NetNS.Path()); err != nil {
return err
}
}

if err := c.makeBindMounts(); err != nil {
Expand All @@ -769,7 +783,9 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti
}

// Cleanup for a working restore.
c.removeConmonFiles()
if err := c.removeConmonFiles(); err != nil {
return err
}

// Save the OCI spec to disk
if err := c.saveSpec(g.Spec()); err != nil {
Expand All @@ -793,8 +809,8 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti
logrus.Debugf("Non-fatal: removal of checkpoint directory (%s) failed: %v", c.CheckpointPath(), err)
}
cleanup := [...]string{"restore.log", "dump.log", "stats-dump", "stats-restore", "network.status"}
for _, delete := range cleanup {
file := filepath.Join(c.bundlePath(), delete)
for _, del := range cleanup {
file := filepath.Join(c.bundlePath(), del)
err = os.Remove(file)
if err != nil {
logrus.Debugf("Non-fatal: removal of checkpoint file (%s) failed: %v", file, err)
Expand Down Expand Up @@ -824,14 +840,14 @@ func (c *Container) makeBindMounts() error {
// will recreate. Only do this if we aren't sharing them with
// another container.
if c.config.NetNsCtr == "" {
if path, ok := c.state.BindMounts["/etc/resolv.conf"]; ok {
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
if resolvePath, ok := c.state.BindMounts["/etc/resolv.conf"]; ok {
if err := os.Remove(resolvePath); err != nil && !os.IsNotExist(err) {
return errors.Wrapf(err, "error removing container %s resolv.conf", c.ID())
}
delete(c.state.BindMounts, "/etc/resolv.conf")
}
if path, ok := c.state.BindMounts["/etc/hosts"]; ok {
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
if hostsPath, ok := c.state.BindMounts["/etc/hosts"]; ok {
if err := os.Remove(hostsPath); err != nil && !os.IsNotExist(err) {
return errors.Wrapf(err, "error removing container %s hosts", c.ID())
}
delete(c.state.BindMounts, "/etc/hosts")
Expand Down Expand Up @@ -968,10 +984,10 @@ func (c *Container) makeBindMounts() error {
// generateResolvConf generates a containers resolv.conf
func (c *Container) generateResolvConf() (string, error) {
resolvConf := "/etc/resolv.conf"
for _, ns := range c.config.Spec.Linux.Namespaces {
if ns.Type == spec.NetworkNamespace {
if ns.Path != "" && !strings.HasPrefix(ns.Path, "/proc/") {
definedPath := filepath.Join("/etc/netns", filepath.Base(ns.Path), "resolv.conf")
for _, namespace := range c.config.Spec.Linux.Namespaces {
if namespace.Type == spec.NetworkNamespace {
if namespace.Path != "" && !strings.HasPrefix(namespace.Path, "/proc/") {
definedPath := filepath.Join("/etc/netns", filepath.Base(namespace.Path), "resolv.conf")
_, err := os.Stat(definedPath)
if err == nil {
resolvConf = definedPath
Expand Down Expand Up @@ -1096,10 +1112,10 @@ func (c *Container) generatePasswd() (string, error) {
if c.config.User == "" {
return "", nil
}
spec := strings.SplitN(c.config.User, ":", 2)
userspec := spec[0]
if len(spec) > 1 {
groupspec = spec[1]
splitSpec := strings.SplitN(c.config.User, ":", 2)
userspec := splitSpec[0]
if len(splitSpec) > 1 {
groupspec = splitSpec[1]
}
// If a non numeric User, then don't generate passwd
uid, err := strconv.ParseUint(userspec, 10, 32)
Expand Down Expand Up @@ -1137,7 +1153,7 @@ func (c *Container) generatePasswd() (string, error) {
if err != nil {
return "", errors.Wrapf(err, "failed to create temporary passwd file")
}
if os.Chmod(passwdFile, 0644); err != nil {
if err := os.Chmod(passwdFile, 0644); err != nil {
return "", err
}
return passwdFile, nil
Expand Down
Loading