[lxc-devel] [crio-lxc/master] add lint target, clean up some trivial things

[mikemccracken on Github]

The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/crio-lxc/pull/20

This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.

=== Description (from pull-request) ===

From 282de8dec8e79cada960186e9cabba07b611a42f Mon Sep 17 00:00:00 2001
From: Michael McCracken 
Date: Mon, 10 Feb 2020 15:28:15 -0800
Subject: [PATCH 1/3] create: propagate errors from ensureShell function

Fixes #9

Signed-off-by: Michael McCracken 
---
 cmd/create.go | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/cmd/create.go b/cmd/create.go
index 6e6ed4b..0c52d23 100644
--- a/cmd/create.go
+++ b/cmd/create.go
@@ -54,32 +54,33 @@ var NamespaceMap = map[string]string{
"uts": "uts",
 }
 
-func ensureShell(rootfs string) {
+func ensureShell(rootfs string) error {
shPath := filepath.Join(rootfs, "bin/sh")
if exists, _ := pathExists(shPath); exists {
-   return
+   return nil
}
var err error
err = RunCommand("mkdir", filepath.Join(rootfs, "bin"))
if err != nil {
-   fmt.Printf("Failed doing mkdir: %v\n", err)
+   return errors.Wrapf(err, "Failed doing mkdir")
}
err = RunCommand("cp", "/bin/busybox", filepath.Join(rootfs, "bin/"))
if err != nil {
-   fmt.Printf("Failed copying busybox: %v\n", err)
+   return errors.Wrapf(err, "Failed copying busybox")
}
err = RunCommand("ln", filepath.Join(rootfs, "bin/busybox"), 
filepath.Join(rootfs, "bin/stat"))
if err != nil {
-   fmt.Printf("Failed linking stat: %v\n", err)
+   return errors.Wrapf(err, "Failed linking stat")
}
err = RunCommand("ln", filepath.Join(rootfs, "bin/busybox"), 
filepath.Join(rootfs, "bin/sh"))
if err != nil {
-   fmt.Printf("Failed linking sh: %v\n", err)
+   return errors.Wrapf(err, "Failed linking sh")
}
err = RunCommand("ln", filepath.Join(rootfs, "bin/busybox"), 
filepath.Join(rootfs, "bin/tee"))
if err != nil {
-   fmt.Printf("Failed linking tee : %v\n", err)
+   return errors.Wrapf(err, "Failed linking tee")
}
+   return nil
 }
 
 const (
@@ -243,7 +244,9 @@ func configureContainer(ctx *cli.Context, c *lxc.Container, 
spec *specs.Spec) er
return errors.Wrapf(err, "couldn't write wrapper init")
}
 
-   ensureShell(spec.Root.Path)
+   if err := ensureShell(spec.Root.Path); err != nil {
+   return errors.Wrap(err, "couldn't ensure a shell exists in 
container")
+   }
 
if err := c.SetConfigItem("lxc.init.cwd", spec.Process.Cwd); err != nil 
{
return errors.Wrap(err, "failed to set CWD")

From 3718a05f3eb22bd0d3ba9979d3aa284cc44e4cee Mon Sep 17 00:00:00 2001
From: Michael McCracken 
Date: Mon, 10 Feb 2020 15:31:02 -0800
Subject: [PATCH 2/3] add linter target to makefile

adds initial lint.yaml with some common annoying messages excluded.

assumes you have golangci-lint installed.

Signed-off-by: Michael McCracken 
---
 Makefile  |  3 +++
 lint.yaml | 17 +
 2 files changed, 20 insertions(+)
 create mode 100644 lint.yaml

diff --git a/Makefile b/Makefile
index e92cbd1..1d8f2b2 100644
--- a/Makefile
+++ b/Makefile
@@ -4,6 +4,9 @@ COMMIT=$(if $(shell git status --porcelain 
--untracked-files=no),$(COMMIT_HASH)-
 TEST?=$(patsubst test/%.bats,%,$(wildcard test/*.bats))
 PACKAGES_DIR?=~/packages
 
+lint:
+   golangci-lint run -c ./lint.yaml ./...
+
 crio-lxc: $(GO_SRC)
go build -ldflags "-X main.version=$(COMMIT)" -o crio-lxc ./cmd
 
diff --git a/lint.yaml b/lint.yaml
new file mode 100644
index 000..f58ce3b
--- /dev/null
+++ b/lint.yaml
@@ -0,0 +1,17 @@
+issues:
+  exclude:
+- 'Error return value of 
.((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*printf?|os\.(Un)?Setenv).
 is not checked'
+- 'error strings should not be capitalized'
+- 'error strings should not end with punctuation'
+- 'File is not `goimports`-ed'
+- 'has \d* occurrences, make it a constant'
+- 'line is \d* characters'
+- 'is a global variable'
+- 'ifElseChain: rewrite if-else to switch statement'
+- 'Error return value of `.*` is not checked'
+- 'cyclomatic complexity \d* of func'
+- 'G107: Potential HTTP request made with variable url'
+- 'should have name of the form ErrFoo'
+- 'naked return in func'
+- 'by other packages, and that stutters; consider calling this'
+- 'File is not `gofmt`-ed with `-s`'

From fa3e39e1e3873b530feb280184df64dec3024145 Mon Sep 17 00:00:00 2001
From: Michael McCracken 
Date: Mon, 10 Feb 2020 15:34:59 -0800
Subject: [PATCH 3/3] minor lint fixes

Signed-off-by: Michael McCracken 
---
 cmd/create.go | 2 +-
 cmd/state.go  | 2 +-
 

[lxc-devel] [crio-lxc/master] fix created status

[mikemccracken on Github]

The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/crio-lxc/pull/19

This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.

=== Description (from pull-request) ===
if the container has been created with a 'create' call, but not started yet, even if we have a running underlying container, we need to report 'created' instead of 'running' or 'stopped'.

detect this case by always getting the PID if the container's running, and then using /proc/$initpid/task/$initpid/children, check for a single child running our fifo-waiter.

If that's the case, we are 'created'.

Adds this to the tests.
From 09a48901b62ed46ffdc5ce15de343dfb828a7dbc Mon Sep 17 00:00:00 2001
From: Michael McCracken 
Date: Wed, 29 Jan 2020 14:07:40 -0800
Subject: [PATCH 1/2] state: support 'created' status for containers waiting on
 fifo

per the spec, a container that has been created but has not been
started by a separate call, should return status 'created', not
'running', despite its actual underlying container process being up
and running.

Do this by getting the child PID of the init task and reading its
cmdline. if the cmdline is our fifo-waiter, we're 'created'.

Signed-off-by: Michael McCracken 
---
 cmd/state.go | 44 ++--
 test/manual.bats |  8 
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/cmd/state.go b/cmd/state.go
index 6bea455..6494752 100644
--- a/cmd/state.go
+++ b/cmd/state.go
@@ -3,10 +3,13 @@ package main
 import (
"encoding/json"
"fmt"
+   "io/ioutil"
"os"
"path/filepath"
+   "strconv"
+   "strings"
 
-   //  "github.com/apex/log"
+   // "github.com/apex/log"
"github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
"github.com/urfave/cli"
@@ -51,14 +54,43 @@ func doState(ctx *cli.Context) error {
 
}
 
-   // TODO need to detect 'created' per
-   // 
https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc4/runtime.md#state
-   // it means "the container process has neither exited nor executed the 
user-specified program"
status := "stopped"
pid := 0
-   if c.Running() && checkHackyPreStart(c) == "started" {
-   status = "running"
+   if c.Running() {
+   if checkHackyPreStart(c) == "started" {
+   status = "running"
+   }
pid = c.InitPid()
+
+   // need to detect 'created' per
+   // 
https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc4/runtime.md#state
+   // it means "the container process has neither exited nor 
executed the user-specified program"
+
+   // if cmd name of the child of the init pid starts with 
"/bin/sh /fifo-wait" then we can say it's 'created'
+
+   procChildrenFilename := 
fmt.Sprintf("/proc/%d/task/%d/children", pid, pid)
+   childrenStr, err := ioutil.ReadFile(procChildrenFilename)
+   if err != nil {
+   return errors.Wrapf(err, "failed to read children from 
%s", procChildrenFilename)
+   }
+   children := 
strings.Split(strings.TrimSpace(string(childrenStr)), " ")
+
+   if len(children) == 1 {
+   childPid, err := strconv.Atoi(children[0])
+   if err != nil {
+   return errors.Wrapf(err, "failed to convert 
child pid")
+   }
+   procCmdlineFilename := fmt.Sprintf("/proc/%d/cmdline", 
childPid)
+   cmdline, err := ioutil.ReadFile(procCmdlineFilename)
+   if err != nil {
+   return errors.Wrapf(err, "failed to read 
cmdline from %s", procCmdlineFilename)
+   }
+
+   cmdArgv := strings.Split(string(cmdline), "\x00")
+   if len(cmdArgv) > 2 && cmdArgv[0] == "/bin/sh" && 
cmdArgv[1] == "/fifo-wait" {
+   status = "created"
+   }
+   }
}
// bundlePath is the enclosing directory of the rootfs:
// 
https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc4/bundle.md
diff --git a/test/manual.bats b/test/manual.bats
index 265e1fb..c4f9ae0 100644
--- a/test/manual.bats
+++ b/test/manual.bats
@@ -16,7 +16,15 @@ function teardown() {
 
 @test "manual invocation" {
 crio-lxc --debug --log-level trace --log-file "$TEMP_DIR/log" create 
--bundle "$TEMP_DIR/dest" --pid-file "$TEMP_DIR/pid" alpine
+
+status=$(crio-lxc --debug --log-level trace --log-file "$TEMP_DIR/log" 
state alpine | jq -r .status)
+[ $status = "created" ]
+
 crio-lxc --debug --log-level trace --log-file "$TEMP_DIR/log" start alpine
+
+

[lxc-devel] [crio-lxc/master] handle namespaces

[mikemccracken on Github]

The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/crio-lxc/pull/14

This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.

=== Description (from pull-request) ===
- passes through namespace config as specified in https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc4/config-linux.md#namespaces

- adds a test for namespace sharing

- fixes tests to clean up created containers
From 71f195b3d423e1abf8b82d4357cfa1122511090d Mon Sep 17 00:00:00 2001
From: Michael McCracken 
Date: Wed, 1 May 2019 18:27:57 -0700
Subject: [PATCH 1/5] create: handle namespaces in spec

Signed-off-by: Michael McCracken 
---
 cmd/create.go | 52 +++
 1 file changed, 52 insertions(+)

diff --git a/cmd/create.go b/cmd/create.go
index 2d53b6c..c3edad9 100644
--- a/cmd/create.go
+++ b/cmd/create.go
@@ -9,6 +9,7 @@ import (
"os/exec"
"path"
"path/filepath"
+   "regexp"
"strings"
"time"
 
@@ -42,6 +43,17 @@ var createCmd = cli.Command{
},
 }
 
+// maps from CRIO namespace names to LXC names
+var NamespaceMap = map[string]string{
+   "cgroup":  "cgroup",
+   "ipc": "ipc",
+   "mount":   "mnt",
+   "network": "net",
+   "pid": "pid",
+   "user":"user",
+   "uts": "uts",
+}
+
 func ensureShell(rootfs string) {
shPath := filepath.Join(rootfs, "bin/sh")
if exists, _ := pathExists(shPath); exists {
@@ -202,6 +214,46 @@ func configureContainer(ctx *cli.Context, c 
*lxc.Container, spec *specs.Spec) er
return errors.Wrap(err, "failed to set hook version")
}
 
+   procPidPathRE := regexp.MustCompile(`/proc/(\d+)/ns`)
+
+   var nsToClone []string
+   var configVal string
+   seenNamespaceTypes := map[specs.LinuxNamespaceType]bool{}
+   for _, ns := range spec.Linux.Namespaces {
+   if _, ok := seenNamespaceTypes[ns.Type]; ok == true {
+   return fmt.Errorf("duplicate namespace type %s", 
ns.Type)
+   }
+   seenNamespaceTypes[ns.Type] = true
+   if ns.Path == "" {
+   nsToClone = append(nsToClone, 
NamespaceMap[string(ns.Type)])
+   } else {
+   configKey := fmt.Sprintf("lxc.namespace.share.%s", 
NamespaceMap[string(ns.Type)])
+
+   matches := procPidPathRE.FindStringSubmatch(ns.Path)
+   switch len(matches) {
+   case 0:
+   configVal = ns.Path
+   case 1:
+   return fmt.Errorf("error parsing namespace 
path. expected /proc/(\\d+)/ns/*, got '%s'", ns.Path)
+   case 2:
+   configVal = matches[1]
+   default:
+   return fmt.Errorf("error parsing namespace 
path. expected /proc/(\\d+)/ns/*, got '%s'", ns.Path)
+   }
+
+   if err := c.SetConfigItem(configKey, configVal); err != 
nil {
+   return errors.Wrapf(err, "failed to set 
namespace config: '%s'='%s'", configKey, configVal)
+   }
+   }
+   }
+
+   if len(nsToClone) > 0 {
+   configVal = strings.Join(nsToClone, " ")
+   if err := c.SetConfigItem("lxc.namespace.clone", configVal); 
err != nil {
+   return errors.Wrapf(err, "failed to set 
lxc.namespace.clone=%s", configVal)
+   }
+   }
+
// capabilities?
 
// if !spec.Process.Terminal {

From 8fbba421bedf68439fdc0b72d38d2a6cd4335411 Mon Sep 17 00:00:00 2001
From: Michael McCracken 
Date: Thu, 2 May 2019 12:10:24 -0700
Subject: [PATCH 2/5] helpers: fix var reference in crictl func

want to substitute, not run CRICTLDEBUG

Signed-off-by: Michael McCracken 
---
 test/helpers.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/helpers.bash b/test/helpers.bash
index 9b906db..1741ea1 100644
--- a/test/helpers.bash
+++ b/test/helpers.bash
@@ -54,7 +54,7 @@ function crictl {
 # watch out for: https://github.com/kubernetes-sigs/cri-tools/issues/460
 # If you need more debug output, set CRICTLDEBUG to -D
 CRICTLDEBUG=""
-$(which crictl) $(CRICTLDEBUG) --runtime-endpoint "$TEMP_DIR/crio.sock" $@
+$(which crictl) ${CRICTLDEBUG} --runtime-endpoint "$TEMP_DIR/crio.sock" $@
 echo "$output"
 }
 

From d7ed2812ea42801e86d999a6e9e13cfcece4a86c Mon Sep 17 00:00:00 2001
From: Michael McCracken 
Date: Thu, 2 May 2019 16:38:08 -0700
Subject: [PATCH 3/5] test: clean up created containers

Signed-off-by: Michael McCracken 
---
 test/basic.bats  | 2 ++
 test/manual.bats | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/test/basic.bats 

[lxc-devel] [crio-lxc/master] readme: fix typo in crictl download link

[mikemccracken on Github]

The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/crio-lxc/pull/12

This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.

=== Description (from pull-request) ===
Signed-off-by: Michael McCracken 
From 230fb5e65f9c5bb270e9e2407194ea20e86e3e04 Mon Sep 17 00:00:00 2001
From: Michael McCracken 
Date: Wed, 24 Apr 2019 13:21:09 -0700
Subject: [PATCH] readme: fix typo in crictl download link

Signed-off-by: Michael McCracken 
---
 README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 4e4e7d6..5d01c66 100644
--- a/README.md
+++ b/README.md
@@ -41,7 +41,7 @@ You'll also need crictl.  Download the tarball, extract it, 
and
 copy crictl to /usr/bin:
 
 ```
-wget 
https://github.com/kubernetes-sigs/cri-tools/releases/download/v1.13.0/crictl-v1.14.0-linux-amd64.tar.gz
+wget 
https://github.com/kubernetes-sigs/cri-tools/releases/download/v1.14.0/crictl-v1.14.0-linux-amd64.tar.gz
 tar zxf crictl-v1.14.0-linux-amd64.tar.gz
 sudo cp crictl /usr/bin
 ```
___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

[lxc-devel] [lxc/master] storage: treat return value from ops->destroy as int

[mikemccracken on Github]

The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/2097

This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.

=== Description (from pull-request) ===
r->ops->destroy() returns an int, -1 on error.
When assigned to a bool, this becomes true and hides errors.

ret will always be true - on error, it's -1 - a true bool - or it is zero and thus set to true.

Signed-off-by: Michael McCracken 
From ed05aac829498161289d4f2da4002b42fb54bc32 Mon Sep 17 00:00:00 2001
From: Michael McCracken 
Date: Fri, 19 Jan 2018 08:38:36 -0800
Subject: [PATCH] storage: treat return value from ops->destroy as int

r->ops->destroy() returns an int, -1 on error.
When assigned to a bool, this becomes true and hides errors.

Signed-off-by: Michael McCracken 
---
 src/lxc/storage/storage.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/lxc/storage/storage.c b/src/lxc/storage/storage.c
index 98aa031b7..e080ad87a 100644
--- a/src/lxc/storage/storage.c
+++ b/src/lxc/storage/storage.c
@@ -603,13 +603,14 @@ bool storage_destroy(struct lxc_conf *conf)
 {
struct lxc_storage *r;
bool ret = false;
+   int destroy_rv = 0;
 
r = storage_init(conf);
if (!r)
return ret;
 
-   ret = r->ops->destroy(r);
-   if (ret == 0)
+   destroy_rv = r->ops->destroy(r);
+   if (destroy_rv == 0)
ret = true;
 
storage_put(r);
___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

[lxc-devel] [lxc/master] manpage: correct lxc.log.file conf option

[mikemccracken on Github]

The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/2096

This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.

=== Description (from pull-request) ===
lxc.log.file instead of just lxc.log
From d5d300094dd549b797a12de5a11b9db0193f7bb8 Mon Sep 17 00:00:00 2001
From: Michael McCracken 
Date: Fri, 19 Jan 2018 11:05:26 -0800
Subject: [PATCH] manpage: correct lxc.log.file conf option

lxc.log.file instead of just lxc.log
---
 doc/lxc.container.conf.sgml.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/lxc.container.conf.sgml.in b/doc/lxc.container.conf.sgml.in
index d106c9635..3ae4bfd18 100644
--- a/doc/lxc.container.conf.sgml.in
+++ b/doc/lxc.container.conf.sgml.in
@@ -2136,7 +2136,7 @@ dev/null proc/kcore none bind,relative 0 0
 
 
   
-lxc.log
+lxc.log.file
   
   
 
___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel