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

feature: implement the feature of reopen log about cri #2447

Merged
merged 1 commit into from
Dec 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
63 changes: 51 additions & 12 deletions cri/v1alpha2/cri.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (

// NOTE: "golang.org/x/net/context" is compatible with standard "context" in golang1.7+.
"github.com/cri-o/ocicni/pkg/ocicni"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -250,7 +251,8 @@ func (c *CriManager) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
return nil, err
}
sandboxMeta := &SandboxMeta{
ID: id,
ID: id,
ContainerLogMap: make(map[string]string),
}
if err := c.SandboxStore.Put(sandboxMeta); err != nil {
return nil, err
Expand Down Expand Up @@ -499,28 +501,26 @@ func (c *CriManager) RemovePodSandbox(ctx context.Context, r *runtime.RemovePodS

// Remove all containers in the sandbox.
for _, container := range containers {
err = c.ContainerMgr.Remove(ctx, container.ID, &apitypes.ContainerRemoveOptions{Volumes: true, Force: true})
if err != nil {
if err := c.ContainerMgr.Remove(ctx, container.ID, &apitypes.ContainerRemoveOptions{Volumes: true, Force: true}); err != nil {
return nil, fmt.Errorf("failed to remove container %q of sandbox %q: %v", container.ID, podSandboxID, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can use errors.Wrap here because the Remove might return the 404 or specific error.

We can handle this in next pr. it is not blocked issue.

}

logrus.Infof("success to remove container %q of sandbox %q", container.ID, podSandboxID)
}

// Remove the sandbox container.
err = c.ContainerMgr.Remove(ctx, podSandboxID, &apitypes.ContainerRemoveOptions{Volumes: true, Force: true})
if err != nil {
if err := c.ContainerMgr.Remove(ctx, podSandboxID, &apitypes.ContainerRemoveOptions{Volumes: true, Force: true}); err != nil {
return nil, fmt.Errorf("failed to remove sandbox %q: %v", podSandboxID, err)
}

// Cleanup the sandbox root directory.
sandboxRootDir := path.Join(c.SandboxBaseDir, podSandboxID)
err = os.RemoveAll(sandboxRootDir)
if err != nil {

if err := os.RemoveAll(sandboxRootDir); err != nil {
return nil, fmt.Errorf("failed to remove root directory %q: %v", sandboxRootDir, err)
}

err = c.SandboxStore.Remove(podSandboxID)
if err != nil {
if err := c.SandboxStore.Remove(podSandboxID); err != nil {
return nil, fmt.Errorf("failed to remove meta %q: %v", sandboxRootDir, err)
}

Expand Down Expand Up @@ -765,6 +765,11 @@ func (c *CriManager) CreateContainer(ctx context.Context, r *runtime.CreateConta
// Get container log.
if config.GetLogPath() != "" {
logPath := filepath.Join(sandboxConfig.GetLogDirectory(), config.GetLogPath())
sandboxMeta.ContainerLogMap[containerID] = logPath
Copy link
Contributor

Choose a reason for hiding this comment

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

@starnop containerLogMap has not been initialized

Copy link
Contributor Author

Choose a reason for hiding this comment

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

THX, I have updated it.

if err := c.SandboxStore.Put(sandboxMeta); err != nil {
return nil, err
}

if err := c.ContainerMgr.AttachCRILog(ctx, containerID, logPath); err != nil {
return nil, err
}
Expand Down Expand Up @@ -825,8 +830,7 @@ func (c *CriManager) RemoveContainer(ctx context.Context, r *runtime.RemoveConta

containerID := r.GetContainerId()

err := c.ContainerMgr.Remove(ctx, containerID, &apitypes.ContainerRemoveOptions{Volumes: true, Force: true})
if err != nil {
if err := c.ContainerMgr.Remove(ctx, containerID, &apitypes.ContainerRemoveOptions{Volumes: true, Force: true}); err != nil {
return nil, fmt.Errorf("failed to remove container %q: %v", containerID, err)
}

Expand Down Expand Up @@ -1128,7 +1132,42 @@ func (c *CriManager) UpdateContainerResources(ctx context.Context, r *runtime.Up
// to either create a new log file and return nil, or return an error.
// Once it returns error, new container log file MUST NOT be created.
func (c *CriManager) ReopenContainerLog(ctx context.Context, r *runtime.ReopenContainerLogRequest) (*runtime.ReopenContainerLogResponse, error) {
return nil, fmt.Errorf("ReopenContainerLog Not Implemented Yet")
containerID := r.GetContainerId()

container, err := c.ContainerMgr.Get(ctx, containerID)
if err != nil {
return nil, fmt.Errorf("failed to get container %q with error: %v", containerID, err)
}
if !container.IsRunning() {
return nil, errors.Wrap(errtypes.ErrPreCheckFailed, "container is not running")
}

// get the container's podSandbox id.
podSandboxID, ok := container.Config.Labels[sandboxIDLabelKey]
if !ok {
return nil, fmt.Errorf("failed to get the sandboxId of container %q", containerID)
}

// get logPath of container
res, err := c.SandboxStore.Get(podSandboxID)
if err != nil {
return nil, fmt.Errorf("failed to get metadata of %q from SandboxStore: %v", podSandboxID, err)
}
sandboxMeta, ok := res.(*SandboxMeta)
if !ok {
return nil, fmt.Errorf("failed to type asseration for sandboxMeta: %v", res)
}
logPath, ok := sandboxMeta.ContainerLogMap[containerID]
if !ok {
logrus.Warnf("log path of container: %q is empty", containerID)
return &runtime.ReopenContainerLogResponse{}, nil
}

if err := c.ContainerMgr.AttachCRILog(ctx, container.Name, logPath); err != nil {
return nil, err
}

return &runtime.ReopenContainerLogResponse{}, nil
}

// ExecSync executes a command in the container, and returns the stdout output.
Expand Down
3 changes: 3 additions & 0 deletions cri/v1alpha2/cri_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ type SandboxMeta struct {

// NetNS is the sandbox's network namespace
NetNS string

// ContainerLogMap store the mapping of container id and CRI logPath.
ContainerLogMap map[string]string
}

// Key returns sandbox's id.
Expand Down
3 changes: 3 additions & 0 deletions daemon/mgr/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,9 @@ func (mgr *ContainerManager) AttachContainerIO(ctx context.Context, name string,

// AttachCRILog adds cri log to a container.
func (mgr *ContainerManager) AttachCRILog(ctx context.Context, name string, logPath string) error {
if logPath == "" {
return errors.Wrap(errtypes.ErrInvalidParam, "logPath cannot be empty")
}
c, err := mgr.container(name)
if err != nil {
return err
Expand Down
1 change: 0 additions & 1 deletion hack/testing/run_daemon_cri_integration.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export PATH="${GOPATH}/bin:${PATH}"
# CRI_SKIP skips the test to skip.
DEFAULT_CRI_SKIP="seccomp localhost"
DEFAULT_CRI_SKIP="${DEFAULT_CRI_SKIP}|should error on create with wrong options"
DEFAULT_CRI_SKIP="${DEFAULT_CRI_SKIP}|runtime should support reopening container log"
CRI_SKIP="${CRI_SKIP:-"${DEFAULT_CRI_SKIP}"}"

# CRI_FOCUS focuses the test to run.
Expand Down
11 changes: 10 additions & 1 deletion pkg/errtypes/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ var (

// ErrNotModified represents that the resource is not modified
ErrNotModified = errorType{codeNotModified, "not modified"}

// ErrPreCheckFailed represents that failed to pre check.
ErrPreCheckFailed = errorType{codePreCheckFailed, "pre check failed"}
)

const (
Expand All @@ -47,6 +50,7 @@ const (
codeNotImplemented
codeInUse
codeNotModified
codePreCheckFailed

// volume error code
codeVolumeExisted
Expand Down Expand Up @@ -88,11 +92,16 @@ func IsInUse(err error) bool {
return checkError(err, codeInUse)
}

// IsNotModified checks the error is not modified erro or not.
// IsNotModified checks the error is not modified error or not.
func IsNotModified(err error) bool {
return checkError(err, codeNotModified)
}

// IsPreCheckFailed checks the error is failed to pre check or not.
func IsPreCheckFailed(err error) bool {
return checkError(err, codePreCheckFailed)
}

func checkError(err error, code int) bool {
err = causeError(err)

Expand Down