Feat: support container root filesystem log collection #209
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ethfoo
changed the title
ethfoo
force-pushed
the
feat-proc-root
branch
4 times, most recently
from
497af4e
to
7fa7e1d
Compare
ethfoo
changed the title
iamyeka
reviewed
May 29, 2022
| @@ -386,33 +388,38 @@ func updatePaths(config *Config, s *fileSource, pod *corev1.Pod, containerName, | |||
| return nil | |||
| } | |||
|
|
|||
| func getPathsInNode(config *Config, containerPaths []string, pod *corev1.Pod, containerName string, containerId string) ([]string, error) { | |||
| func (c *Controller) getPathsInNode(config *Config, containerPaths []string, pod *corev1.Pod, containerName string, containerId string) ([]string, error) { | |||
| if len(containerPaths) == 0 { | |||
| return nil, errors.New("path is empty") | |||
| } | |||
|
|
|||
| var paths []string | |||
| for _, p := range containerPaths { | |||
There was a problem hiding this comment.
为什么不把这段逻辑放在下面helper.PathsInNode方法的实现里，我看PathsInNode里面还特意忽略掉了path为stdout的情况，而且GenContainerStdoutLog这个方法和PathsInNode也是在一个包里面，直接调用应该也比较方便
| continue | ||
| } | ||
| } | ||
|
|
||
| p, err := helper.PathsInNode(config.KubeletRootDir, containerPaths, pod, containerName) |
There was a problem hiding this comment.
这里是如果containerPaths中通过volume方式找不到路径就失败了，然后全部检测是否是rootfs，有没有可能用户配了volume格式的path，也配了rootfs，两种path都有？
There was a problem hiding this comment.
如果找不到volume，会去找rootfs，不冲突
| @@ -71,6 +83,8 @@ func (d *Discovery) Start(stopCh <-chan struct{}) { | |||
| logConfInformerFactory.Loggie().V1beta1().LogConfigs(), logConfInformerFactory.Loggie().V1beta1().ClusterLogConfigs(), logConfInformerFactory.Loggie().V1beta1().Sinks(), | |||
| logConfInformerFactory.Loggie().V1beta1().Interceptors(), nodeInformerFactory.Core().V1().Nodes()) | |||
|
|
|||
| ctrl.Runtime = d.runtime | |||
There was a problem hiding this comment.
controller其他参数都通过NewController方法传递，为什么Runtime要通过赋值的方式来传递？
| func Init(endpoints []string, defaultRuntime string) Runtime { | ||
| runtimeName := defaultRuntime | ||
| if defaultRuntime == "" { | ||
| runtimeName = getRuntimeName(endpoints, defaultRuntime) |
There was a problem hiding this comment.
这里defaultRuntime已经是空了，还需要作为参数往下传吗？
| continue | ||
| } | ||
|
|
||
| rootfsPaths = append(rootfsPaths, rootfsPrePath+p) |
There was a problem hiding this comment.
p这个path是/开头吗，我看rootfsPrePath以merged结尾的，会不会缺/的情况？
| if p == logconfigv1beta1.PathStdout { | ||
| continue | ||
| } | ||
| rootfsPaths = append(rootfsPaths, prefix+p) |
ethfoo
force-pushed
the
feat-proc-root
branch
2 times, most recently
from
a854ce4
to
9d3954c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed Changes:
enable this feature by
rootFsCollectionEnabledsee below:mergedrootfs by inspecting the docker status/proc/{pid}/rootWhich issue(s) this PR fixes:
Fixes #208
Additional documentation: